Opened 2 years ago

Closed 23 months ago

Last modified 23 months ago

#34283 closed Bug (fixed)

Missing parameter escaping in admin filters.js

Reported by: Stanislav Owned by: Stanislav
Component: contrib.admin Version: 4.2
Severity: Release blocker Keywords:
Cc: Stanislav, Marcelo Galigniana Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I am using ukrainian language in django admin. This language has an apostrophe that users often write as '.

My code:

class HasMasterFilter(admin.SimpleListFilter):
    title = "Пов'язаний?"

leads to error in /django/contrib/admin/static/admin/js/filters.js at the line:

const detailElement = document.querySelector(`[data-filter-title='${key}']`);

it's needed to escape the value in key

Change History (11)

comment:1 by Stanislav, 2 years ago

Cc: Stanislav added
Has patch: set

Patch:

14c14
<         const detailElement = document.querySelector(`[data-filter-title='${key}']`);
---
>         const detailElement = document.querySelector(`[data-filter-title='${CSS.escape(key)}']`);
30a31
>

comment:2 by Mariusz Felisiak, 2 years ago

Cc: Marcelo Galigniana added
Has patch: unset
Severity: Normal → Release blocker
Triage Stage: Unreviewed → Accepted
Version: 4.1 → 4.2

Thanks for the report and testing against the alpha release!

Regression in 27aa7035f57f0db30b6632e4274e18b430906799.

comment:3 by Stanislav, 2 years ago

Owner: changed from nobody to Stanislav
Status: new → assigned

comment:4 by Mariusz Felisiak, 2 years ago

Stanislav, Can you send PR via GitHub? (a regression test is required.)

in reply to:  4 ; comment:5 by Stanislav, 23 months ago

Replying to Mariusz Felisiak:

Stanislav, Can you send PR via GitHub? (a regression test is required.)

I hope i did it

in reply to:  5 ; comment:6 by Marcelo Galigniana, 23 months ago

Replying to Stanislav:

Replying to Mariusz Felisiak:

Stanislav, Can you send PR via GitHub? (a regression test is required.)

I hope i did it

Hi Stanislav! I just saw your PR but it doesn’t have a new test. Marius means add a test like this: ​https://github.com/django/django/commit/27aa7035f57f0db30b6632e4274e18b430906799#diff-640602cc17dc3e9a26db47f58b40a1289d5c0f1b5b0fe547810b1d962f6cde8b. You could use it as a reference and add the escape case! Don’t forget update the issue status here too!

Let me know if you need help with something!

comment:7 by Mariusz Felisiak, 23 months ago

Has patch: set
Needs tests: set

​PR

in reply to:  6 comment:8 by Stanislav, 23 months ago

Replying to Marcelo Galigniana:

Replying to Stanislav:

Replying to Mariusz Felisiak:

Stanislav, Can you send PR via GitHub? (a regression test is required.)

I hope i did it

Hi Stanislav! I just saw your PR but it doesn’t have a new test. Marius means add a test like this: ​https://github.com/django/django/commit/27aa7035f57f0db30b6632e4274e18b430906799#diff-640602cc17dc3e9a26db47f58b40a1289d5c0f1b5b0fe547810b1d962f6cde8b. You could use it as a reference and add the escape case! Don’t forget update the issue status here too!

Let me know if you need help with something!

​https://github.com/django/django/pull/16499
the error is related to js, ​​so I process the browser logs in selenium

comment:9 by Mariusz Felisiak, 23 months ago

Needs tests: unset
Triage Stage: Accepted → Ready for checkin

comment:10 by Mariusz Felisiak <felisiak.mariusz@…>, 23 months ago

Resolution: → fixed
Status: assigned → closed

In 7217c11e:

[4.2.x] Fixed #34283 -- Escaped title in admin's changelist filters.

Regression in 27aa7035f57f0db30b6632e4274e18b430906799.

Backport of 20a0850099340fb4cb8df0e4441e5019b2cbd1ea from main

comment:11 by Mariusz Felisiak <felisiak.mariusz@…>, 23 months ago

In 20a0850:

Fixed #34283 -- Escaped title in admin's changelist filters.

Regression in 27aa7035f57f0db30b6632e4274e18b430906799.

Note: See TracTickets for help on using tickets.
Back to Top