AdminFileWidget renders two elements with the same ID

[Johannes Maron]

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.

[Natalia Bidart]

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 ​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.

[Johannes Maron]

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

[Jörg Benesch]

I can second the report and will put the triage to accepted.

[Natalia Bidart]

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 ids):

<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>

[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:
​https://github.com/django/django/actions/runs/8178301006/job/22361926712#step:6:25331

- <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">

And yes, a bound field will pass the ID as an attribute to a widget here:
​https://github.com/django/django/blob/c4df2a77761a1ae392eb5c4803b5712803d5239f/django/forms/boundfield.py#L96-L99

Hehe, and yes, good to be cautious. My last ticket was a dumpster fire. I read the docs this time, promise :P
I sincerely appreciate the swift support here <3

[Tim Graham]

Bisected to 8a6c0203c4e92908c2b26ba54feba4ce7e76d081.

[Johannes Maron]

I will write a full patch tomorrow and check if we have other widgets that include the same problem.

[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.

[Tim Graham]

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.)

[Johannes Maron]

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.

[Mariusz Felisiak <felisiak.mariusz@…>]

In e6901955:

Fixed #35273 -- Fixed rendering AdminFileWidget's attributes.

Regression in 8a6c0203c4e92908c2b26ba54feba4ce7e76d081.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 8fd953f2:

[5.0.x] Fixed #35273 -- Fixed rendering AdminFileWidget's attributes.

Regression in 8a6c0203c4e92908c2b26ba54feba4ce7e76d081.

Backport of e69019555d683fd6a831f87cb09e3deb86e4e7c7 from main