ClearableFileInput.use_fieldset became True in Django 6.0

[Johannes Maron]

Howdy y'all,

#35892 flipped django.forms.widgets.ClearableFileInput.use_fieldset from false to true.

The change can be found here: ​https://github.com/django/django/blame/4187da258fe212d494cb578a0bc2b52c4979ab95/django/forms/widgets.py#L533

This changes the form rendering outside the admin too. Probably just a small oversight and not an intentional change, since changes weren't accompanied by release notes.

Anyhow, both the input and the use_fieldset are part of the public documentation, and changes must undergo deprecation.

The problem gets amplified with #36828, since the current fieldset implementation violates HTML and WCAG 2.1

The only affected version is 6.0. I would recommend providing a regression patch in the next bugfix release.

Cheers!
Joe

[Tim Graham]

It might be that only AdminFileWidget (the ClearableFileInput used in admin forms) should have that attribute.

[Antoliny]

Thank you for reporting this issue, Joe :)

In my view, using a fieldset for ClearableFileInput itself makes sense.
However, I also agree that it’s a problem that a change related to the admin affected form rendering outside of the admin as well (and that this wasn’t communicated).

As Tim suggested, should this change be limited to the admin only?

[Johannes Maron]

Agreed, let's limit the change to the admin, since its widgets aren't publicly documented.

I'll attach a potential patch.

[Jacob Walls]

Thanks for the report. I'm assuming that the change was intentional and regarded as a bugfix, which we don't document. I'm not certain whether we shouldn't just resolve this with a release note and versionchanged annotations.

Docs say:

when the widget contains multiple <input> tags such as ...

By that criterion, use_fieldset should be True here, modulo the configurability of required=True or no initial value causing the checkbox to not render.

The ​API stability policy mentions that minor changes may be necessary on upgrade. Johannes, can I ask what inconvenience this amounted to: have you authored a library that now has to fork on this behavior change?

---

David considered setting use_fieldset = True on ClearableFileInput from the get-go, but hesitated given the checkbox not being present if required=True or no initial value:

ClearableFiles render in different ways. It is possible to get a checkbox and a file field input with this one, and I wonder if we should be adding a fieldset in that scenario

and received mixed feedback:

​against:

I'm not sure fieldset is needed in that case, aria attributes here will be the aria-controlsand aria-label to know that there is a button action and the title of the action.

​in favor:

Based on a quick review, yes, I think it would make sense to have a fieldset for this, although it looks like the template would need further work (missing a label as well for the input?) so it’d warrant a more thorough review (and its own ticket).

... with the result that it was just tabled.

David, do you have any opinion about whether to retain this change as a bugfix?

[David Smith]

I'll comment on this ticket and #36828 as a pair. In summary, my view is to:

• Keep this ticket to revert the change to ClearableFileInput in Django 6.0 but keep it in the admin AdminFileWidget (i.e. implement Tim's comment).
• Keep #36828 as a new feature to change the ClearableFileInput template to add <fieldset> and <legend>. This should be a new feature that can target 6.1 or later.

My reason for this suggestion is that #35892, which introduced this change, was specifically aimed at targeting the admin widget. The change to the default widget, as far as I can tell, was an unintended consequence of that change and needs further changes to the default template to make it work. Reverting the change to the default template, but keeping it in the admin, seems (but I've not specifically tried) like it would be a straight forward enough change to make.

This will allow us to take more time and to get feedback from the accessibility team before making changes to the default ClearableFileInput template. Especially as the accessibility team previously had mixed reviews on the change that would be required -- albeit that was some time ago.

[Jacob Walls]

Thanks, David. I think that's prudent. I'll review Johannes's PR implementing Tim's suggestion.

[Jacob Walls]

I dug around and found some prior ideation around providing a deprecation path for widget rendering changes. See ticket:32339#comment:2:

One issue we have is breaking peoples existing pages if we change the rendered HTML (so that CSS/JS selectors change for instance).
Can I point you to #31026, ... If we can get that in, then we'd be able to document a Use this template ... to restore the previous HTML, which then would give us room to introduce a more significant change.

Thanks Johannes for implementing #31026 years ago! :-)

Assuming something like that is possible here, I now agree this change would need to go through a deprecation path.

[Jacob Walls <jacobtylerwalls@…>]

In 79ab0993:

Fixed #36829 -- Reverted value of ClearableFileInput.use_fieldset to True.

There was unresolved discussion regarding whether to set
ClearableFileInput.use_fieldset to True or False when use_fieldset was
introduced in Django 4.1, since the clear checkbox appears only
sometimes. Although using <fieldset> is likely desirable, since the
primary motivation in #35892 was just to improve markup in the admin,
and a deprecation path was not provided for general form usage, future
work is deferred to #36828.

Regression in 4187da258fe212d494cb578a0bc2b52c4979ab95.

Thanks Tim Graham, Antoliny, and David Smith for triage.

[Jacob Walls <jacobtylerwalls@…>]

In b7b5465:

[6.0.x] Fixed #36829 -- Reverted value of ClearableFileInput.use_fieldset to True.

There was unresolved discussion regarding whether to set
ClearableFileInput.use_fieldset to True or False when use_fieldset was
introduced in Django 4.1, since the clear checkbox appears only
sometimes. Although using <fieldset> is likely desirable, since the
primary motivation in #35892 was just to improve markup in the admin,
and a deprecation path was not provided for general form usage, future
work is deferred to #36828.

Regression in 4187da258fe212d494cb578a0bc2b52c4979ab95.

Thanks Tim Graham, Antoliny, and David Smith for triage.

[cessor]

Incidentally, this change also fixed #36772 which was originally closed as invalid.