[wp-trac] [WordPress Trac] #42054: Customize Themes: invalid HTML and aria-describedby values

[WordPress Trac]

#42054: Customize Themes: invalid HTML and aria-describedby values
--------------------------+----------------------------
 Reporter:  afercia       |       Owner:  afercia
     Type:  defect (bug)  |      Status:  reviewing
 Priority:  normal        |   Milestone:  4.9
Component:  Customize     |     Version:  trunk
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:  accessibility
--------------------------+----------------------------
Changes (by celloexpressions):

 * keywords:  needs-patch => has-patch
 * owner:  celloexpressions => afercia
 * status:  assigned => reviewing


Old description:

> Results after a quick check with the W3C HTML validator.
>
> To reproduce, copy from your browser inspector the HTML of the element
> with id `sub-accordion-section-themes`, paste it in the validator inside
> the body element:
>
> Error: Element div not allowed as child of element ul in this context.
> (Suppressing further errors from this subtree.)
> `</li>↩                 <div class="customize-themes-section themes-
> section-installed_themes control-section-content themes-php">`
>

> Error: Element div not allowed as child of element ul in this context.
> (Suppressing further errors from this subtree.)
> `<div class="customize-themes-section themes-section-wporg_themes
> control-section-content themes-php current-section">`
>
> These two errors relate to 2 `<div>` elements output inside a `<ul>`.
>
> Error: Duplicate ID live-search-desc.
> From line 297, column 5; to line 297, column 59
> `<span id="live-search-desc" class="screen-reader-text">`
> Warning: The first occurrence of ID live-search-desc was here.
> From line 45, column 6; to line 45, column 60
>
> The installed themes and wp.org themes search have the same visually
> hidden text with the same id.
>
> Error: The aria-describedby attribute must point to an element in the
> same document.
> <div class="theme active" tabindex="0" aria-describedby
> ="installed_themes-twentysixteen-action twentysixteen-name">
>
> Error: The aria-describedby attribute must point to an element in the
> same document.
> <div class="theme" tabindex="0" aria-describedby="installed_themes-
> intergalactic-action intergalactic-name">
>
> etc, lots of them.
>
> Re: the aria-described value, it references two elements: for example:
> - for the installed themes: `aria-describedby="installed_themes-
> twentysixteen-action twentysixteen-name"`
> - for the wp.org themes: `aria-describedby="wporg_themes-mytheme-action
> mytheme-name"`
>
> however, there are no elements with ID `twentysixteen-name` or `mytheme-
> name`
> Instead, the elements have a prefix:
> `installed_themes-twentysixteen-name`
> and
> `wporg_themes-mytheme-name`
>
> However, I'm not sure the aria-describedby attribute should target both
> elements, it's a bit pointless to repeat the theme name, unless I'm
> missing something the result will be screen readers announcing, for
> example:
>
> `Details for theme: Twenty Sixteen Previewing: Twenty Sixteen`
>
> or
>
> `Details for theme: Twenty Eleven Twenty Eleven`
>
> Worth reminding the aria-label set on the first element will override its
> text, so the actual text used by aria-describedby will be the aria-label
> value e.g. "Details for theme: Twenty Sixteen"
>

> Correct association of labels:
> as per the WordPress Accessibility standards, labels should be associated
> to their form controls using for/id attributes (not wrapping the label
> around the control). Doing both things can really confuse assistive
> technologies:
>
> {{{
> <label for="themes-filter">
>                                         <span class="screen-reader-
> text">Search themes…</span>
>                                         <input type="search" id="themes-
> filter" placeholder="Search themes…" aria-describedby="live-search-desc"
> class="wp-filter-search wp-filter-search-themes">
>                                         <span id="live-search-desc" class
> ="screen-reader-text">The search results will be updated as you
> type.</span>
>                                 </label>
> }}}
>
> Please review all the new labels and make sure they do not wrap the form
> control and use for/id attributes instead.

New description:

 Results after a quick check with the W3C HTML validator.

 To reproduce, copy from your browser inspector the HTML of the element
 with id `sub-accordion-section-themes`, paste it in the validator inside
 the body element:

 Error: Element div not allowed as child of element ul in this context.
 (Suppressing further errors from this subtree.)
 `</li>                  <div class="customize-themes-section themes-
 section-installed_themes control-section-content themes-php">`


 Error: Element div not allowed as child of element ul in this context.
 (Suppressing further errors from this subtree.)
 `<div class="customize-themes-section themes-section-wporg_themes control-
 section-content themes-php current-section">`

 These two errors relate to 2 `<div>` elements output inside a `<ul>`.

 Error: Duplicate ID live-search-desc.
 From line 297, column 5; to line 297, column 59
 `<span id="live-search-desc" class="screen-reader-text">`
 Warning: The first occurrence of ID live-search-desc was here.
 From line 45, column 6; to line 45, column 60

 The installed themes and wp.org themes search have the same visually
 hidden text with the same id.

 Error: The aria-describedby attribute must point to an element in the same
 document.
 <div class="theme active" tabindex="0" aria-describedby="installed_themes-
 twentysixteen-action twentysixteen-name">

 Error: The aria-describedby attribute must point to an element in the same
 document.
 <div class="theme" tabindex="0" aria-describedby="installed_themes-
 intergalactic-action intergalactic-name">

 etc, lots of them.

 Re: the aria-described value, it references two elements: for example:
 - for the installed themes: `aria-describedby="installed_themes-
 twentysixteen-action twentysixteen-name"`
 - for the wp.org themes: `aria-describedby="wporg_themes-mytheme-action
 mytheme-name"`

 however, there are no elements with ID `twentysixteen-name` or `mytheme-
 name`
 Instead, the elements have a prefix:
 `installed_themes-twentysixteen-name`
 and
 `wporg_themes-mytheme-name`

 However, I'm not sure the aria-describedby attribute should target both
 elements, it's a bit pointless to repeat the theme name, unless I'm
 missing something the result will be screen readers announcing, for
 example:

 `Details for theme: Twenty Sixteen Previewing: Twenty Sixteen`

 or

 `Details for theme: Twenty Eleven Twenty Eleven`

 Worth reminding the aria-label set on the first element will override its
 text, so the actual text used by aria-describedby will be the aria-label
 value e.g. "Details for theme: Twenty Sixteen"


 Correct association of labels:
 as per the WordPress Accessibility standards, labels should be associated
 to their form controls using for/id attributes (not wrapping the label
 around the control). Doing both things can really confuse assistive
 technologies:

 {{{
 <label for="themes-filter">
                                         <span class="screen-reader-
 text">Search themes…</span>
                                         <input type="search" id="themes-
 filter" placeholder="Search themes…" aria-describedby="live-search-desc"
 class="wp-filter-search wp-filter-search-themes">
                                         <span id="live-search-desc" class
 ="screen-reader-text">The search results will be updated as you
 type.</span>
                                 </label>
 }}}

 Please review all the new labels and make sure they do not wrap the form
 control and use for/id attributes instead.

--

Comment:

 [attachment:42054.diff]:
 - Fix div as child of ul by not using ul as a child of li where it doesn't
 represent a list
 - Fix duplicate filtering IDs
 - Remove broken reference to theme name in aria-described by
 - Remove `for` attribute from themes filter label (multiple elements
 comprise the label, se for wouldn't work)

--
Ticket URL: <https://core.trac.wordpress.org/ticket/42054#comment:3>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform