[Django] #34705: BoundField.as_widget() ignores aria-describedby in attrs argument
Django
-----------------------------------------+-------------------------------
Reporter: Sage Abdullah | Owner: Sage Abdullah
Type: Bug | Status: assigned
Component: Forms | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------+-------------------------------
`BoundField.as_widget()` ignores `aria-describedby` that is passed in the
`attrs` argument.
Use case:
In Wagtail, we have our own mechanism for rendering form fields (called
"Panels") that is built on top of Django's Forms API. We use
`BoundField.as_widget()` in our rendering process to render only the form
field's widget, while its help text element is rendered separately via our
Panels API with a custom markup (including HTML `id`) that conforms to our
design system. We've been passing additional attributes to
`BoundField.as_widget()` to improve accessibility, including `aria-
describedby`, to establish the relationship between the field widget
rendered by Django and our help text element.
As of 966ecdd482167f3f6b08b00f484936c837751cb9, Django automatically adds
`aria-describedby` to form fields with a `help_text`. The logic in
`build_widget_attrs()` (used by `as_widget()`) checks for an existing
`aria-describedby` in the field's widget's `attrs` before automatically
generating one. However, it does not take into account the possibility of
an existing `aria-describedby` in the `attrs` argument.
A workaround on Wagtail's side could be to modify the widget's attrs
before using `as_widget()`, but this is not ideal as the widget is
customisable by the developer and we would have to copy the widget
instance to avoid modifying the existing widget instance (which might be
shared across form fields).
I believe Django should check for `aria-describedby` in `attrs` first,
before falling back to `widget.attrs` and the auto-generated value.
Test case:
{{{
class BoundFieldTests(SimpleTestCase):
def test_as_widget_with_custom_aria_describedby(self):
class TestForm(Form):
data = CharField(help_text="Some help text")
form = TestForm({"data": "some value"})
self.assertHTMLEqual(
form["data"].as_widget(attrs={"aria-describedby":
"custom_help_text_id"}),
"""
<input type="text" name="data" value="some value"
aria-describedby="custom_help_text_id" required id="id_data">
""",
)
}}}
Patch:
{{{
diff --git a/django/forms/boundfield.py b/django/forms/boundfield.py
index deba739329..b9f29e3da5 100644
--- a/django/forms/boundfield.py
+++ b/django/forms/boundfield.py
@@ -290,7 +290,9 @@ class BoundField(RenderableFieldMixin):
# If a custom aria-describedby attribute is given and help_text
is
# used, the custom aria-described by is preserved so user can set
the
# desired order.
- if custom_aria_described_by_id := widget.attrs.get("aria-
describedby"):
+ if custom_aria_described_by_id := attrs.get(
+ "aria-describedby"
+ ) or widget.attrs.get("aria-describedby"):
attrs["aria-describedby"] = custom_aria_described_by_id
elif self.field.help_text and self.id_for_label:
attrs["aria-describedby"] = f"{self.id_for_label}_helptext"
}}}
Happy to submit a PR if this is accepted.
--
Ticket URL: <https://code.djangoproject.com/ticket/34705>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Sage Abdullah | Owner: Sage Abdullah
Type: Bug | Status: assigned
Component: Forms | Version: dev
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Sage Abdullah):
* Attachment "boundfield-as_widget-aria_describedby.patch" added.
Patch to fix the issue
Django
-------------------------------+-----------------------------------------
Reporter: Sage Abdullah | Owner: Sage Abdullah
Type: Bug | Status: assigned
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Old description:
New description:
Test case:
Patch:
{{{diff --git a/django/forms/boundfield.py b/django/forms/boundfield.py
index deba739329..e4261d5e50 100644
--- a/django/forms/boundfield.py
+++ b/django/forms/boundfield.py
@@ -290,10 +290,11 @@ class BoundField(RenderableFieldMixin):
# If a custom aria-describedby attribute is given and help_text
is
# used, the custom aria-described by is preserved so user can set
the
# desired order.
- if custom_aria_described_by_id := widget.attrs.get("aria-
describedby"):
- attrs["aria-describedby"] = custom_aria_described_by_id
- elif self.field.help_text and self.id_for_label:
- attrs["aria-describedby"] = f"{self.id_for_label}_helptext"
+ if not attrs.get("aria-describedby"):
+ if custom_aria_described_by_id := widget.attrs.get("aria-
describedby"):
+ attrs["aria-describedby"] = custom_aria_described_by_id
+ elif self.field.help_text and self.id_for_label:
+ attrs["aria-describedby"] =
f"{self.id_for_label}_helptext"
return attrs
@property
}}}
Happy to submit a PR if this is accepted.
--
Comment (by Sage Abdullah):
Updated the patch in the description for better readability.
--
Ticket URL: <https://code.djangoproject.com/ticket/34705#comment:1>
Django
-------------------------------+-----------------------------------------
Reporter: Sage Abdullah | Owner: Sage Abdullah
Type: Bug | Status: assigned
Component: Forms | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+-----------------------------------------
Old description:
New description:
Test case:
Patch:
@property
}}}
--
--
Ticket URL: <https://code.djangoproject.com/ticket/34705#comment:2>
Django
Reporter: Sage Abdullah | Owner: Sage Abdullah
Type: Bug | Status: assigned
Component: Forms | Version: dev
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 Mariusz Felisiak):
* cc: Gregor Jerše, David Smith, Thibaud Colas (added)
* has_patch: 1 => 0
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
Comment:
Thanks for the report, good catch! Bug in
966ecdd482167f3f6b08b00f484936c837751cb9.
--
Ticket URL: <https://code.djangoproject.com/ticket/34705#comment:3>
Django
---------------------------------+-----------------------------------------
Reporter: Sage Abdullah | Owner: Sage Abdullah
Type: Bug | Status: assigned
Component: Forms | Version: dev
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/17065 PR].
Slightly adjusted the logic to make the logic closer to the old behaviour.
--
Ticket URL: <https://code.djangoproject.com/ticket/34705#comment:4>
Django
Reporter: Sage Abdullah | Owner: Sage
| Abdullah
Type: Bug | Status: assigned
Component: Forms | Version: dev
Severity: Release blocker | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34705#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Sage Abdullah | Owner: Sage
| Abdullah
Component: Forms | Version: dev
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: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"3f73df44f2726319f6af44c5613e5e531e9370b6" 3f73df44]:
{{{
#!CommitTicketReference repository=""
revision="3f73df44f2726319f6af44c5613e5e531e9370b6"
Fixed #34705 -- Reallowed BoundField.as_widget()'s attrs argument to set
aria-describedby.
Regression in 966ecdd482167f3f6b08b00f484936c837751cb9.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34705#comment:6>