Opened 8 months ago

Closed 8 months ago

#34705 closed Bug (fixed)

BoundField.as_widget() ignores aria-describedby in attrs argument

Reported by: Sage Abdullah Owned by: Sage Abdullah
Component: Forms Version: dev
Severity: Release blocker Keywords:
Cc: Gregor Jerše, David Smith, Thibaud Colas Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Sage Abdullah)

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

Attachments (1)

boundfield-as_widget-aria_describedby.patch (831 bytes ) - added by Sage Abdullah 8 months ago.
Patch to fix the issue

Download all attachments as: .zip

Change History (7)

by Sage Abdullah, 8 months ago

Patch to fix the issue

comment:1 by Sage Abdullah, 8 months ago

Description: modified (diff)

Updated the patch in the description for better readability.

comment:2 by Sage Abdullah, 8 months ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 8 months ago

Cc: Gregor Jerše David Smith Thibaud Colas added
Has patch: unset
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thanks for the report, good catch! Bug in 966ecdd482167f3f6b08b00f484936c837751cb9. I'd prefer a solution from the ticket description, it's more readable.

Last edited 8 months ago by Mariusz Felisiak (previous) (diff)

comment:4 by Sage Abdullah, 8 months ago

Has patch: set

PR.

Slightly adjusted the logic to make the logic closer to the old behaviour.

comment:5 by Mariusz Felisiak, 8 months ago

Triage Stage: AcceptedReady for checkin

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 8 months ago

Resolution: fixed
Status: assignedclosed

In 3f73df44:

Fixed #34705 -- Reallowed BoundField.as_widget()'s attrs argument to set aria-describedby.

Regression in 966ecdd482167f3f6b08b00f484936c837751cb9.

Note: See TracTickets for help on using tickets.
Back to Top