[Django] #35273: AdminFileWidget renders two elements with the same ID

22 Aufrufe
Direkt zur ersten ungelesenen Nachricht

Django

ungelesen,
05.03.2024, 11:19:465. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
------------------------------------------+------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
The `AdminFileWidget` renders both the checkbox and file input with the
same attributes, including the ID.
The template actually renders the ID attribute twice.

Both, two identical attributes on an element or two elements with the same
ID are in violation with version of HTML.

See
https://github.com/django/django/blob/eff21d8e7a1cb297aedf1c702668b590a1b618f3/django/contrib/admin/templates/admin/widgets/clearable_file_input.html#L3

This first appeared in Django 5.0.

My suggestion would be to avoid rendering `attrs` into the checkbox.
--
Ticket URL: <https://code.djangoproject.com/ticket/35273>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

ungelesen,
05.03.2024, 13:28:235. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
--------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
Changes (by Natalia Bidart):

* resolution: => needsinfo
* status: new => closed

Comment:

Hello Johanes, thank you for your interest in making Django better.

Could you please include a small example (or test case!) showcasing the
described issue? Looking at the code and
[https://github.com/django/django/blob/eff21d8e7a1cb297aedf1c702668b590a1b618f3/tests/admin_widgets/tests.py#L574
relevant test] I do not see the duplicated `id` attribute, so I'm assuming
there is some specifics on the code you are using that is triggering this
behavior.

Thank you! Feel free to re-open when you can provide more details.
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:1>

Django

ungelesen,
06.03.2024, 07:11:386. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
--------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
Changes (by Johannes Maron):

* resolution: needsinfo =>
* status: closed => new

Comment:

Hi Natalia,

Thanks, I appreciate you taking the time looking into this and welcoming
me. Hehe, but I am not really new ;) I happened to contribute, among other
things, the template-based form rendering and autocomplete fields. Thus, I
would consider myself somewhat familiar with this part of Django.

The test you are pointing to is insufficient, since it doesn't include all
arguments what would be passed to a widget by a form.
You can see that the output HTML doesn't include an ID for the
`input[type=file]`. Which it would obviously have in an actual form.

Funnily enough, the test actually outputs invalid HTML too. A `label.for`
attribute must point to an existing ID, which it doesn't.

I am happy to provide a test that will fail. The regression was introduced
in Django 5 and only applies to the admin file input template.

Cheers,
Joe
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:2>

Django

ungelesen,
06.03.2024, 07:14:586. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
--------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
Changes (by Mariusz Felisiak):

* cc: Marcelo Galigniana, David Smith (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:3>

Django

ungelesen,
06.03.2024, 07:22:466. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
--------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
Comment (by herrbenesch):

I can second the report and will put the triage to accepted.
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:4>

Django

ungelesen,
06.03.2024, 07:23:006. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
--------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+------------------------------------
Changes (by herrbenesch):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:5>

Django

ungelesen,
06.03.2024, 09:03:376. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
--------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+-------------------------------------
Changes (by Natalia Bidart):

* resolution: => needsinfo
* status: new => closed

Comment:

Thank you Johanness and Jörg!

Before accepting, we'd still need a sample project or test case showcasing
the issue, as I can't reproduce the duplicated ID symptom (I'm using
latest `main` though). I created a project of my own and defined a model
with a FileField, then configured its admin so the `AdminFileWidget` is
used. This is the resulting HTML which looks correct to me (at least it
does not have duplicated `id`s):

{{{
<p class="file-upload">Currently: <a
href="/images/test_X66RiS7.py">images/test_X66RiS7.py</a>
<span class="clearable-file-input">
<input type="checkbox" name="avatar-clear" id="avatar-clear_id">
<label for="avatar-clear_id">Clear</label></span><br>
Change:
<input type="file" name="avatar" id="id_avatar"></p>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:6>

Django

ungelesen,
06.03.2024, 11:21:576. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
--------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Unreviewed

--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:7>

Django

ungelesen,
06.03.2024, 15:32:456. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
--------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: closed
Component: contrib.admin | Version: 5.0
Severity: Normal | Resolution: needsinfo
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------+--------------------------------------
Comment (by Johannes Maron):

Absolutely, here is a draft with a commit that exploits the error:
https://github.com/django/django/pull/17946

And that's the error:


{{{
- <input id="test-clear_id" id="test_id" name="test-clear"
type="checkbox"><label for="test-clear_id">
? -------------

+ <input id="test-clear_id" name="test-clear" type="checkbox"><label for
="test-clear_id">
}}}



If and yes, a bound field will pass the ID as an attribute here:
https://github.com/django/django/blob/c4df2a77761a1ae392eb5c4803b5712803d5239f/django/forms/boundfield.py#L96-L99
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:8>

Django

ungelesen,
06.03.2024, 17:58:366. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
---------------------------------+------------------------------------
Reporter: Johannes Maron | Owner: nobody
Type: Bug | Status: new
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+------------------------------------
Changes (by Tim Graham):

* resolution: needsinfo =>
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
* status: closed => new

Comment:

Bisected to 8a6c0203c4e92908c2b26ba54feba4ce7e76d081.
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:9>

Django

ungelesen,
07.03.2024, 08:09:217. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Johannes Maron):

* owner: nobody => Johannes Maron
* status: new => assigned

Comment:

I write a full patch tomorrow and check if we have other widgets that
include the same problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:10>

Django

ungelesen,
08.03.2024, 14:46:198. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Johannes Maron):

* easy: 0 => 1
* has_patch: 0 => 1
* ui_ux: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:11>

Django

ungelesen,
08.03.2024, 14:51:318. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Johannes Maron):

OK, the patch is ready for review. BTW, the problem was only in the
Django, not the Jinja2 template. I checked all the other widgets, none-
other seemed to have the same issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:12>

Django

ungelesen,
08.03.2024, 21:43:088. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Tim Graham):

* easy: 1 => 0
* needs_better_patch: 0 => 1

Comment:

The patch merely reverts part of 8a6c0203c4e92908c2b26ba54feba4ce7e76d081.
and causes
`admin_widgets.tests.ImageFieldWidgetsSeleniumTests.test_clearablefileinput_widget_preserve_clear_checkbox`
to fail.

(Incidentally, there is no need to set "easy pickings" to put this ticket
on the radar for new contributors.)
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:13>

Django

ungelesen,
09.03.2024, 07:23:429. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Johannes Maron):

* needs_better_patch: 1 => 0

Comment:

Thanks, Tim, for your diligence. I did read up on the other ticket and
applied its behavior to all clearable file templates and added tests and
another line to the release docs.
Please tell me, if there is anything else that needs to be addressed.
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:14>

Django

ungelesen,
14.03.2024, 08:50:0914. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:15>

Django

ungelesen,
14.03.2024, 15:37:4314. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: closed
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"e69019555d683fd6a831f87cb09e3deb86e4e7c7" e6901955]:
{{{#!CommitTicketReference repository=""
revision="e69019555d683fd6a831f87cb09e3deb86e4e7c7"
Fixed #35273 -- Fixed rendering AdminFileWidget's attributes.

Regression in 8a6c0203c4e92908c2b26ba54feba4ce7e76d081.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:16>

Django

ungelesen,
14.03.2024, 15:38:1314. März
an django-...@googlegroups.com
#35273: AdminFileWidget renders two elements with the same ID
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: closed
Component: contrib.admin | Version: 5.0
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):

In [changeset:"8fd953f28abc95aee2e2f59c94d8c58af0d792d7" 8fd953f2]:
{{{#!CommitTicketReference repository=""
revision="8fd953f28abc95aee2e2f59c94d8c58af0d792d7"
[5.0.x] Fixed #35273 -- Fixed rendering AdminFileWidget's attributes.

Regression in 8a6c0203c4e92908c2b26ba54feba4ce7e76d081.

Backport of e69019555d683fd6a831f87cb09e3deb86e4e7c7 from main
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35273#comment:17>
Allen antworten
Antwort an Autor
Weiterleiten
0 neue Nachrichten