Opened 3 years ago

Closed 17 months ago

#32338 closed Bug (fixed)

Accessibility issues with Django forms RadioSelect and CheckboxSelectMultiple

Reported by: Thibaud Colas Owned by: David Smith
Component: Forms Version: dev
Severity: Normal Keywords: accessibility, forms, wcag
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: yes

Description (last modified by Mariusz Felisiak)

In the Forms API, there are issues with the default rendering of fields that rely on widgets based on multiple radio or checkbox inputs: RadioSelect and CheckboxSelectMultiple. This is with the default rendering of those widgets, and with the custom rendering examples in the official documentation. Here is a form I set up for my testing of the default rendering:

RadioSelect issues

  1. The fields aren’t semantically grouped, only visually, so the grouping isn’t apparent to assistive technlogies. It’s important that related fields are grouped, and that the group has a label attached to it. If I was auditing this professionally I would classify this as a fail for SC 1.3.1 - Info and Relationships of WCAG 2.1, as the grouping and label are somewhat there, just not done with the appropriate semantics.
  2. (fixed in b9e872b59329393f615c440c54f632a49ab05b78) Speaking of label – currently the group of fields is labeled by preceding it with <label for="id_radio_choice_required_0">Radio choice required:</label>. This overrides the label of the first field within the group (and only labels the first field, not the whole group). This is a fail for 3.3.2: Labels or Instructions and/or SC 1.3.1.
  3. (fixed in 5942ab5eb165ee2e759174e297148a40dd855920) Wrapping the fields each in their own list item makes the navigation more tedious, as screen readers will first announce list items numbering, and only then their content.

Here is a screenshot demonstrating the label issue with macOS VoiceOver’s rotor. Note how the first field has the wrong label due to the markup.

https://code.djangoproject.com/raw-attachment/ticket/32338/radio-select-first-input-label.png

CheckboxSelectMultiple issues

Almost identical to the above,

  • Lack of a fieldset means no semantic grouping.
  • The label isn’t associated with anything for this widget, so is less problematic but no more correct.
  • (fixed in 5942ab5eb165ee2e759174e297148a40dd855920) Wrapping the fields in list items also makes the form more verbose than it should be (and the semantics issues are the same).

Documentation issues (fixed in d8c17aa10c7f41e692fb6f5d0bf2fab7a90b9374)

Additionally to the above, there are a few occurences of custom radio/checkbox markup in the documentation that will lead to the same issues:

Proposed solution

Essentially following technique H71 of WCAG. The ideal solution is identical for both widgets, and also for documentation code snippets:

  1. Wrap the group of fields in a fieldset, including the group’s label, the inputs, the help text, and any errors.
  2. Use a legend as the first item in the fieldset for the group’s label, rather than a label.
  3. (fixed in 5942ab5eb165ee2e759174e297148a40dd855920) Replace the list markup with un-semantic div, or remove altogether. If the list markup is needed for backwards compatibility, it could also use role="presentation" so assistive technologies ignore the list & listitem semantics, but I would only recommend that as a temporary workaround.

Here is sample markup to implement the above. The div aren’t needed, I’ve only added them to preserve the vertical layout of the current implementation:

<fieldset>
    <legend>Radio choice required:</legend>
    <div><label for="id_radio_choice_required_0"><input type="radio" name="radio_choice_required" value="one" required="" id="id_radio_choice_required_0">
 One</label></div>
    <div><label for="id_radio_choice_required_1"><input type="radio" name="radio_choice_required" value="two" required="" id="id_radio_choice_required_1">
 Two</label></div>
    <div><label for="id_radio_choice_required_2"><input type="radio" name="radio_choice_required" value="three" required="" id="id_radio_choice_required_2">
 Three</label></div>
    <div><label for="id_radio_choice_required_3"><input type="radio" name="radio_choice_required" value="four" required="" id="id_radio_choice_required_3">
 Four</label></div>
    <span class="helptext">Help</span>
</fieldset>

Attachments (1)

radio-select-first-input-label.png (143.3 KB) - added by Thibaud Colas 3 years ago.
Screenshot of a RadioSelect widget, with DevTools open to see the markup, and VoiceOver rotor menu showing the Form Controls’ labels

Download all attachments as: .zip

Change History (28)

Changed 3 years ago by Thibaud Colas

Screenshot of a RadioSelect widget, with DevTools open to see the markup, and VoiceOver rotor menu showing the Form Controls’ labels

comment:1 Changed 3 years ago by Thibaud Colas

Description: modified (diff)

comment:2 Changed 3 years ago by Thibaud Colas

Description: modified (diff)

comment:3 Changed 3 years ago by Thibaud Colas

Description: modified (diff)

comment:4 Changed 3 years ago by Thibaud Colas

Description: modified (diff)

comment:5 Changed 3 years ago by Mariusz Felisiak

Triage Stage: UnreviewedAccepted

Thanks for detailed report.

comment:6 Changed 3 years ago by jcoombes

Owner: changed from nobody to jcoombes
Status: newassigned

comment:7 Changed 3 years ago by jcoombes

21/01/21

Okay, so I can reproduce this bug.
I found the relevant template which fills the content of a relevant widget at:

django/django/forms/templates/django/forms/widgets/multiple_input.html
[The optional version of this template is at .../django/forms/input_option.html]

This template is used to render the html form using:

django/django/forms/forms.py

BaseForm.as_p,
BaseForm.as_ul,
BaseForm.as_table.

I got stuck today - as_p, as_ul and as_table call _html_output() with different parameters.
Changing _html_output() is risky as it would be easy to break single-choice form fields. The code is already quite abstracted.

Easier approach:
Create and document a new method as_fieldset() for this new use case. Avoid _html_output() entirely.

Harder approach:
Rewrite _html_output() to include the html tags necessary to render RadioSelect and CheckboxSelectMultiple accessibly.
Avoid breaking existing code.

I think I will take the easier approach, but I'm open to feedback on how to do it the harder way without introducing regressions.

comment:8 Changed 3 years ago by Claude Paroz

Note you should be aware of current work in #31026 (template-based rendering).

comment:9 Changed 3 years ago by jcoombes

Owner: jcoombes deleted
Status: assignednew

comment:10 Changed 2 years ago by David Smith

Owner: set to David Smith
Status: newassigned

comment:11 Changed 2 years ago by David Smith

Has patch: set

comment:12 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 32b366a8:

[3.2.x] Refs #32338 -- Improved accessibility of RadioSelect examples in docs.

Co-authored-by: Thibaud Colas <thibaudcolas@…>

Backport of d8c17aa10c7f41e692fb6f5d0bf2fab7a90b9374 from main

comment:13 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In d8c17aa:

Refs #32338 -- Improved accessibility of RadioSelect examples in docs.

Co-authored-by: Thibaud Colas <thibaudcolas@…>

comment:14 Changed 2 years ago by Mariusz Felisiak

Description: modified (diff)

comment:15 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In b9e872b5:

Refs #32338 -- Removed 'for ="..."' from RadioSelect's <label>.

This improves accessibility for screen reader users.

Co-authored-by: Thibaud Colas <thibaudcolas@…>

comment:16 Changed 2 years ago by Mariusz Felisiak

Description: modified (diff)

comment:17 Changed 2 years ago by Mariusz Felisiak

Triage Stage: AcceptedReady for checkin

comment:18 Changed 2 years ago by Mariusz Felisiak <felisiak.mariusz@…>

In 5942ab5:

Refs #32338 -- Made RadioSelect/CheckboxSelectMultiple render in <div> tags.

This improves accessibility for screen reader users.

comment:19 Changed 2 years ago by Mariusz Felisiak

Description: modified (diff)
Has patch: unset
Triage Stage: Ready for checkinAccepted

comment:20 Changed 2 years ago by David Smith

Has patch: set

comment:21 Changed 23 months ago by Mariusz Felisiak

Patch needs improvement: set

comment:22 Changed 22 months ago by Mariusz Felisiak

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:23 Changed 22 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In eba9a9b:

Refs #32338 -- Added Boundfield.legend_tag().

comment:24 Changed 22 months ago by Mariusz Felisiak

Has patch: unset
Triage Stage: Ready for checkinAccepted

comment:25 Changed 21 months ago by David Smith

Has patch: set
Last edited 21 months ago by Mariusz Felisiak (previous) (diff)

comment:26 Changed 21 months ago by Mariusz Felisiak

Patch needs improvement: set

comment:27 Changed 17 months ago by Carlton Gibson

Resolution: fixed
Status: assignedclosed

Fixed by ec5659382a5f5fc2daf0c87ccc89d0fb07534874 for #32339.

It may be possible to followup improving the p, table, and ul templates but see https://code.djangoproject.com/ticket/32339#comment:3.

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