[Django] #36829: Breaking change in ClearableFileInput

[Django]

#36829: Breaking change in ClearableFileInput
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Type: Bug
Status: new | Component: Forms
Version: 6.0 | Severity: Release
| blocker
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Howdy y'all,

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

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
--
Ticket URL: <https://code.djangoproject.com/ticket/36829>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36829: Breaking change in ClearableFileInput

---------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: (none)

Type: Bug | Status: new
Component: Forms | Version: 6.0

Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1

---------------------------------+--------------------------------------
Description changed by Johannes Maron:

Old description:

> Howdy y'all,
>
> #35892 flipped `django.forms.widgets.ClearableFileInput.use_fieldset`
> from false to true.
>
> 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

New description:

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

--

--
Ticket URL: <https://code.djangoproject.com/ticket/36829#comment:1>

[Django]

#36829: Breaking change in ClearableFileInput
---------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
---------------------------------+--------------------------------------

Comment (by Tim Graham):

It might be that only `AdminFileWidget` (the `ClearableFileInput` used in
admin forms) should have that attribute.
--
Ticket URL: <https://code.djangoproject.com/ticket/36829#comment:2>

[Django]

#36829: Breaking change in ClearableFileInput
---------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
---------------------------------+--------------------------------------

Comment (by 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/36829#comment:3>

[Django]

#36829: Breaking change in ClearableFileInput
---------------------------------+--------------------------------------
Reporter: Johannes Maron | Owner: (none)
Type: Bug | Status: new
Component: Forms | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
---------------------------------+--------------------------------------

Changes (by Antoliny):

* cc: Antoliny (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/36829#comment:4>

[Django]

#36829: Breaking change in ClearableFileInput

-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned

Component: Forms | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1

-------------------------------------+-------------------------------------
Changes (by Johannes Maron):

* owner: (none) => Johannes Maron
* status: new => assigned

Comment:

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

I'll attach a potential patch.
--
Ticket URL: <https://code.djangoproject.com/ticket/36829#comment:5>

[Django]

#36829: Breaking change in ClearableFileInput
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: Forms | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Johannes Maron):

* Attachment "ticket-36829.patch" added.

--
Ticket URL: <https://code.djangoproject.com/ticket/36829>

[Django]

#36829: Breaking change in ClearableFileInput
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: Forms | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------
Changes (by Johannes Maron):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/36829#comment:6>

[Django]

#36829: Breaking change in ClearableFileInput
-------------------------------------+-------------------------------------
Reporter: Johannes Maron | Owner: Johannes
| Maron
Type: Bug | Status: assigned
Component: Forms | Version: 6.0
Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 1
-------------------------------------+-------------------------------------

Changes (by Jacob Walls):

* cc: David Smith (added)

Comment:

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 [https://docs.djangoproject.com/en/6.0/misc/api-stability/ 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:

[https://github.com/django/django/pull/15317#issuecomment-1059091155
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.

[https://github.com/django/django/pull/15317#pullrequestreview-922494843
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/36829#comment:7>