Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#47141 closed defect (bug) (fixed)

Radio and checkbox labels rely on implicit association

Reported by: anevins's profile anevins Owned by: afercia's profile afercia
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Media Keywords: has-screenshots wpcampus-report form-controls has-patch
Focuses: accessibility Cc:

Description

Moved from the WPCampus accessibility report issues on GitHub, see https://github.com/WordPress/gutenberg/issues/15300

  • Severity:
    • Low
  • Affected Populations:
    • Motor Impaired
    • Cognitively Impaired
  • Platform(s):
    • Windows - Dragon
  • Components affected:
    • Edit Media
    • Media Dialog

Issue description
In both the "Featured Image" dialog and the Edit Media page, form
inputs are wrapped in label elements without explicit association.

Some combinations of browser and assistive technologies do not reliably
support implicit association (for example, browsers used with Dragon
Naturally Speaking), and so for affected users, the label text may not
be announced with the field it relates to.

Issue Code

    <!-- Edit Media -->


    <div id="imgedit-save-target-159" class="imgedit-save-target">


        <fieldset>


            <legend><strong>Apply changes to:</strong></legend>


            <label class="imgedit-label">


            <input type="radio" name="imgedit-target-159" value="all" checked="checked"> All image sizes</label>


            ...


        </fieldset>


    </div>





    <!-- Media Dialog -->


    <div tabindex="0" data-id="159" class="attachment-details save-ready needs-refresh">


        <h2>Attachment Details ... Saved ...</h2>


        <div class="attachment-info">...</div>


        <label class="setting" data-setting="url">


            <span class="name">URL</span>


            <input type="text" value="http://142.93.114.99/wp-content/uploads/2019/01/this-is-fine.jpg" readonly="">


        </label>


         ...


    </div>

Remediation Guidance
Use for and id attributes to explicitly associate each <label> with its field.

Recommended Code

    <!-- Edit Media -->


    <div id="imgedit-save-target-159" class="imgedit-save-target">


        <fieldset>


            <legend><strong>Apply changes to:</strong></legend>


            <label for="imgedit-radio-1"  class="imgedit-label">


                <input id="imgedit-radio-1" type="radio" name="imgedit-target-159" value="all" checked="checked">


                All image sizes


            </label>


            ...


        </fieldset>


    </div>





    <!-- Media Dialog -->


    <div tabindex="0" data-id="159" class="attachment-details save-ready needs-refresh">


        <h2>Attachment Details ... Saved ...</h2>


        <div class="attachment-info">...</div>


        <label for="imgedit-url" class="setting" data-setting="url">


            <span class="name">URL</span>


            <input id="imgedit-url" type="text" value="http://142.93.114.99/wp-content/uploads/2019/01/this-is-fine.jpg" readonly="">


        </label>


         ...


    </div>

Note: This issue may be a duplicate with other existing accessibility-related bugs in this project. This issue comes from the Gutenberg accessibility audit, performed by Tenon and funded by WP Campus. This issue is GUT-74 in Tenon's report

Attachments (2)

57185850-7770ce80-6e88-11e9-806f-7c9b1feb39f1.png (165.2 KB) - added by anevins 5 years ago.
47141.diff (78.0 KB) - added by afercia 5 years ago.

Download all attachments as: .zip

Change History (19)

#1 @afercia
5 years ago

Note about the recommended code above:

The current WordPress accessibility codings standards recommend explicit labelling with for/id attributes and warn to not use both implicit and explicit labelling: https://make.wordpress.org/core/handbook/best-practices/coding-standards/accessibility-coding-standards/#labeling

Testing and data (note: tests were made on 12/01/2012):

https://developer.paciellogroup.com/blog/2011/07/html5-accessibility-chops-form-control-labeling/

Until browser implementers fix their accessibility support, if you want controls to be understandable to AT users label controls using for and id. Do not use the control inside the label method. Do not use a combination of the 2 methods.

Until there are new data, I'd recommend to stick to the WordPress accessibility codings standards.

#2 @afercia
5 years ago

  • Keywords form-controls added

#3 @afercia
5 years ago

  • Milestone changed from Awaiting Review to 5.3

#4 @afercia
5 years ago

  • Owner set to afercia
  • Status changed from new to assigned

This ticket was mentioned in Slack in #accessibility by afercia. View the logs.


5 years ago

#6 @afercia
5 years ago

Related: #47122.

@afercia
5 years ago

#7 @afercia
5 years ago

  • Keywords has-patch commit added; needs-patch removed

47141.diff changes all the media views labels (at least the ones I could find) from implicitly associated (wrapping) labels to implicitly associated (with for/id attributes).

It's a big patch and I'd like to commit it soon in this release cycle, as I think that's the most efficient way to test it. I tested it as best as I can, also on Windows and Internet Explorer 11.

To limit the CSS changes as much as possible I took this approach:

  • a <span class="setting"> now wraps most of the inputs + labels
  • though it's an additional element, in most of the cases this replaces the previous wrapping <label class="setting">
  • the new span element is the only way to address this issue programmatically in a consistent way and allows to keep the CSS changes pretty contained
  • I did my best to provide some backwards compatibility for plugins that may use custom views and get the styles from core

I do expect some minor CSS glitches and I'd tend to think the best way to discover and fix them is to commit this patch so the testers base gets larger. Following fixes can go in additional patches attached to this ticket. Testing and reports greatly appreciated, especially for the responsive views with breakpoints at 900 and 600 pixels.

Trying to recap the most relevant changes:

  • changes the media views form controls to have explicitly associated labels with for/id attributes
  • adds a few missing labels / aria-labels
  • improves a few existing labels / aria-labels
  • improves semantics in a few places, by adding visually hidden headings, fieldset + legend elements, aria-describedby attributes
  • improves the image custom size input fields and their labelling
  • adds role="status" to the "saved" indicator
  • swaps the columns order in the image details template, to make visual and DOM order match
  • swaps the "Replace" and "Back" buttons order in the Replace Image view, to make visual and DOM order match
  • gallery settings: move checkbox label to the right: checkboxes are supposed to have labels on the right
  • merge similar strings, unified to "Drop files to upload" (removed "Drop files here", and "Drop files anywhere to upload")
  • also makes the "upload-ui" consistent across the media views
  • hides the IE 11 "X" -ms-clear button in the Insert from URL field, as it conflicts with the spinner
  • adds comments to all the media templates to clarify their usage
  • increases vertical spacing between form fields in the media sidebar
  • removes some CSS selectors introduced as back-compat for WordPress pre-4.4
  • removes some CSS still targeting Internet Explorer 7 and 8

Todo: update the QUnit index file.

#8 @afercia
5 years ago

A few numbers, for history:

  • total number of templates in the media views: 25
  • total number of labels associated with for/id attributes: 58

This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.


5 years ago

#11 @afercia
5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 45499:

Accessibility: Improve accessibility of all the media views form controls.

  • changes the media views form controls to have explicitly associated labels with for/id attributes
  • adds a few missing labels / aria-labels
  • improves a few existing labels / aria-labels
  • improves semantics in a few places, by adding visually hidden headings, fieldset + legend elements, aria-describedby attributes
  • improves the image custom size input fields and their labelling
  • adds role="status" to the "saved" indicator so that status messages are announced to assistive technologies
  • swaps the columns source order in the image details template, to make visual and DOM order match
  • swaps the "Replace" and "Back" buttons source order in the Replace Image view, to make visual and DOM order match
  • gallery settings: move checkbox label to the right: checkboxes are supposed to have labels on the right
  • merge similar strings, unified to "Drop files to upload" (removed "Drop files here", and "Drop files anywhere to upload")
  • makes the "upload-ui" consistent across the media views
  • hides the IE 11 "X" ::-ms-clear button in the Insert from URL field, as it conflicts with the uploading spinner
  • adds comments to all the media templates to clarify their usage
  • slightly increases vertical spacing between form fields in the media sidebar
  • removes some CSS selectors introduced as backwards compatibility for WordPress pre-4.4
  • removes some CSS still targeting Internet Explorer 7 and 8

Fixes #47141.
Fixes #47122.

#12 @afercia
5 years ago

  • Keywords commit removed

I'll update the QUnit index file in a following commit (even if it's not strictly necessary, as the tests pass).

As mentioned in a previous comment, please report any glitches you may notice so they can be fixed in following commits.

This ticket was mentioned in Slack in #core-media by anevins. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-media by anevins. View the logs.


5 years ago

#15 @JeffPaul
5 years ago

  • Keywords needs-testing fixed-major added
  • Milestone changed from 5.3 to 5.2.3
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening this so it can be back-ported to the 5.2 branch, also needs testing to validate if this ticket is good to land in 5.2.3.

#16 @SergeyBiryukov
5 years ago

  • Keywords needs-testing fixed-major removed
  • Milestone changed from 5.2.3 to 5.3
  • Resolution set to fixed
  • Status changed from reopened to closed

[45499] applies to both #47122 and this ticket.

Since #47122 was moved back to 5.3, moving this one as well, for the same reasons as in comment:7:ticket:47122.

This ticket was mentioned in Slack in #core by marybaum. View the logs.


5 years ago

Note: See TracTickets for help on using tickets.