Make WordPress Core

Opened 21 months ago

Closed 19 months ago

Last modified 17 months ago

#54079 closed enhancement (fixed)

Remove "role" attribute on HTML5 elements with a default landmark role.

Reported by: costdev's profile costdev Owned by: hellofromtonya's profile hellofromTonya
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: has-patch has-dev-note
Focuses: accessibility, css, coding-standards Cc:

Description

With IE11 no longer supported, the primary problem with assistive technology support for native HTML5 elements no longer applies.

This ticket proposes to remove the role attribute from the following HTML5 elements with default landmark roles, per formerly required role attributes and W3C.

A search through the codebase revealed the following:

Notes:

  1. form defaults to role="form" when it has an accessible name using aria-labelledby, aria-label or title attributes. In the codebase, all instances of form that have a role use role="search". As this isn't the default landmark, I left form out of this search.
  2. section doesn't exist in the codebase, so I excluded it from this search.
# Query: (<)(aside|footer|header|main|nav)(<?php)?(.*)?(\\?>)?([^>]+)(role=)(.*)(>)
# Flags: RegExp
# ContextLines: 1

50 results - 50 files

src/wp-content/themes/twentyeleven/footer.php:
- 15: <footer id="colophon" role="contentinfo">

src/wp-content/themes/twentyeleven/header.php:
- 79: <header id="branding" role="banner">

src/wp-content/themes/twentyfifteen/404.php:
- 13: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyfifteen/archive.php:
- 22: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyfifteen/footer.php:
- 15: <footer id="colophon" class="site-footer" role="contentinfo">

src/wp-content/themes/twentyfifteen/header.php:
- 30: <header id="masthead" class="site-header" role="banner">

src/wp-content/themes/twentyfifteen/image.php:
- 13: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyfifteen/index.php:
- 20: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyfifteen/page.php:
- 17: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyfifteen/search.php:
- 13: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyfifteen/single.php:
- 13: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyfourteen/footer.php:
- 15: <footer id="colophon" class="site-footer" role="contentinfo">

src/wp-content/themes/twentyfourteen/header.php:
- 44: <header id="masthead" class="site-header" role="banner">

src/wp-content/themes/twentynineteen/template-parts/footer/footer-widgets.php:
- 12: <aside class="widget-area" role="complementary" aria-label="<?php esc_attr_e( 'Footer', 'twentynineteen' ); ?>">

src/wp-content/themes/twentyseventeen/404.php:
- 17: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyseventeen/archive.php:
- 27: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyseventeen/footer.php:
- 19: <footer id="colophon" class="site-footer" role="contentinfo">

src/wp-content/themes/twentyseventeen/front-page.php:
- 18: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyseventeen/header.php:
- 30: <header id="masthead" class="site-header" role="banner">

src/wp-content/themes/twentyseventeen/index.php:
- 32: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyseventeen/page.php:
- 22: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyseventeen/search.php:
- 31: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyseventeen/sidebar.php:
- 18: <aside id="secondary" class="widget-area" role="complementary" aria-label="<?php esc_attr_e( 'Blog Sidebar', 'twentyseventeen' ); ?>">

src/wp-content/themes/twentyseventeen/single.php:
- 17: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentyseventeen/template-parts/footer/footer-widgets.php:
- 18: <aside class="widget-area" role="complementary" aria-label="<?php esc_attr_e( 'Footer', 'twentyseventeen' ); ?>">

src/wp-content/themes/twentysixteen/404.php:
- 13: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentysixteen/archive.php:
- 22: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentysixteen/footer.php:
- 15: <footer id="colophon" class="site-footer" role="contentinfo">

src/wp-content/themes/twentysixteen/header.php:
- 30: <header id="masthead" class="site-header" role="banner">

src/wp-content/themes/twentysixteen/image.php:
- 13: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentysixteen/index.php:
- 20: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentysixteen/page.php:
- 17: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentysixteen/search.php:
- 13: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentysixteen/sidebar-content-bottom.php:
- 16: <aside id="content-bottom-widgets" class="content-bottom-widgets" role="complementary">

src/wp-content/themes/twentysixteen/sidebar.php:
- 12: <aside id="secondary" class="sidebar widget-area" role="complementary">

src/wp-content/themes/twentysixteen/single.php:
- 13: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentythirteen/footer.php:
- 14: <footer id="colophon" class="site-footer" role="contentinfo">

src/wp-content/themes/twentythirteen/header.php:
- 36: <header id="masthead" class="site-header" role="banner">

src/wp-content/themes/twentytwelve/footer.php:
- 13: <footer id="colophon" role="contentinfo">

src/wp-content/themes/twentytwelve/header.php:
- 37: <header id="masthead" class="site-header" role="banner">

src/wp-content/themes/twentytwenty/404.php:
- 13: <main id="site-content" role="main">

src/wp-content/themes/twentytwenty/footer.php:
- 15: <footer id="site-footer" role="contentinfo" class="header-footer-group">

src/wp-content/themes/twentytwenty/header.php:
- 33: <header id="site-header" class="header-footer-group" role="banner">

src/wp-content/themes/twentytwenty/index.php:
- 20: <main id="site-content" role="main">

src/wp-content/themes/twentytwenty/singular.php:
- 15: <main id="site-content" role="main">

src/wp-content/themes/twentytwenty/template-parts/footer-menus-widgets.php:
- 89: <aside class="footer-widgets-outer-wrapper" role="complementary">

src/wp-content/themes/twentytwenty/templates/template-cover.php:
- 14: <main id="site-content" role="main">

src/wp-content/themes/twentytwentyone/footer.php:
- 21: <footer id="colophon" class="site-footer" role="contentinfo">

src/wp-content/themes/twentytwentyone/header.php:
- 32: <main id="main" class="site-main" role="main">

src/wp-content/themes/twentytwentyone/template-parts/header/site-header.php:
- 16: <header id="masthead" class="<?php echo esc_attr( $wrapper_classes ); ?>" role="banner">

Related tickets: #48920, #54054.

Attachments (3)

54079.diff (25.8 KB) - added by costdev 21 months ago.
54079.1.diff (25.3 KB) - added by costdev 19 months ago.
Option 1 - Keep role="contentinfo" for Twenty Twelve footer.
54079.2.diff (26.6 KB) - added by costdev 19 months ago.
Option 2 - Keep role="contentinfo" for Twenty Twelve footer and add contentinfo class with CSS selector adjustments.

Download all attachments as: .zip

Change History (25)

@costdev
21 months ago

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


21 months ago

#2 @mukesh27
21 months ago

  • Keywords needs-refresh added

Hi, @costdev thanks for the ticket and patch.

I think we have to update CSS if any it uses the role selector. In my quick search, I found below 1 instance.

https://github.com/WordPress/WordPress/blob/master/wp-content/themes/twentytwelve/style.css#L678

footer[role="contentinfo"] {
	border-top: 1px solid #ededed;
	clear: both;
	font-size: 12px;
	font-size: 0.857142857rem;
	line-height: 2;
	max-width: 960px;
	max-width: 68.571428571rem;
	margin-top: 24px;
	margin-top: 1.714285714rem;
	margin-left: auto;
	margin-right: auto;
	padding: 24px 0;
	padding: 1.714285714rem 0;
}
footer[role="contentinfo"] a {
	color: #686868;
}
footer[role="contentinfo"] a:hover {
	color: #21759b;
}

#3 @costdev
21 months ago

Excellent point @mukesh27!

Per W3C:

<footer>

contentinfo when in context of the
body element. The footer element is
not a contentinfo landmark when it is
a descendant of the following HTML
sectioning elements:

  • article
  • aside
  • main
  • nav
  • section

As "context of the body element" can mean inside any number of non-sectioning elements, we can't use body > footer as the selector, as child themes may add one or more wrappers to contain some content and the footer but still use the base styling referenced above.

To avoid having a selector like this:

*:not(article):not(aside):not(main):not(nav):not(section) > footer {
    /* Rules */
}

I think we could just add a .contentinfo class on the relevant footer elements in this theme, and adjust the CSS selectors accordingly:

footer.contentinfo {
    /* Rules */
}

footer.contentinfo a {
    /* Rules */
}

footer.contentinfo a:hover {
    /* Rules */
}

The likelihood of a child theme or plugin having other footer elements with a .contentinfo class seems very low, whereas something more general like .primary or similar feels a bit risky. This way, the update shouldn't unintentionally affect other footer elements.

However, it's possible that sites that use this theme have custom styles that target footer[role="contentinfo"]. I don't think there's anything we can do about that - the custom styles would need to be updated.

What are your thoughts?

Last edited 21 months ago by costdev (previous) (diff)

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


20 months ago

#5 @ryokuhi
20 months ago

  • Milestone changed from Awaiting Review to 5.9

Hello @costdev, and thanks for your patience!
We reviewed the ticket today during the weekly Accessibility Team's bug-scrub.
Can you confirm that, with role="navigation" being addressed in #54054, the only thing that has to be addressed is the use of the role attribute in default themes?

#6 @costdev
20 months ago

@ryokuhi Thanks for reviewing the ticket today! :)

That's right, only the role attribute in default themes needs to be addressed.

#7 @costdev
20 months ago

  • Focuses css added
  • Keywords 2nd-opinion added

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


19 months ago

#9 @craigfrancis
19 months ago

As a second person checking, I could not find any issues; where I focused on core themes, and a general search in the rest of the repo (CSS/JS), I could only find the same [role="contentinfo"] in "twentytwelve/style.css"; everything else didn't seem to be affected by this patch (e.g. role=separator and section[role="banner"]).

Last edited 19 months ago by craigfrancis (previous) (diff)

This ticket was mentioned in Slack in #core-themes by costdev. View the logs.


19 months ago

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


19 months ago

#12 @sabernhardt
19 months ago

  • Component changed from General to Bundled Theme

Child themes can replace the stylesheet or footer template without editing the other. So keeping the role for Twenty Twelve would be safer.

If adding the class (without removing the role), the three styles could cover both situations:

footer[role="contentinfo"],
footer.contentinfo {
...
footer[role="contentinfo"] a,
footer.contentinfo a {
...
footer[role="contentinfo"] a:hover,
footer.contentinfo a:hover {

Then someone could simply revise the footer template in a child theme without needing to edit in two places.

Last edited 19 months ago by sabernhardt (previous) (diff)

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


19 months ago

@costdev
19 months ago

Option 1 - Keep role="contentinfo" for Twenty Twelve footer.

@costdev
19 months ago

Option 2 - Keep role="contentinfo" for Twenty Twelve footer and add contentinfo class with CSS selector adjustments.

#14 @costdev
19 months ago

  • Keywords needs-refresh 2nd-opinion removed

Patch refreshed with two options:

  1. 54079.1 - Keep role="contentinfo" for Twenty Twelve footer.
  2. 54079.2 - The same as above, but also add class="contentinfo" along with adjustments to Twenty Twelve's style.css. The intention here would be to phase out the targeting of footer[role="contentinfo"] by child themes, who will hopefully opt for the footer.contentinfo class selector instead.

I don't have a preference between the patches. We have to keep the role="contentinfo" on the Twenty Twelve footer, so I'm happy for either a Component Maintainer or Committer to make the final call on which of the two latest patches is the better choice.

#15 @costdev
19 months ago

Pinging Component Maintainers @desrosj and @ianbelanger to notify that this is now ready for final review after great feedback including from the Accessibility and CSS teams.

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


19 months ago

#17 @joedolson
19 months ago

I think that the first option is the best choice.

Mostly because Twenty Twelve is pretty old, and maintenance or creation of new child themes off it is probably uncommon at this point - so the imagined "phase out" will, frankly, be unlikely to happen.

It's also not fool proof; a child theme that replaces the footer but inherits the style sheet won't inherit the changed styles, so there is a definite possibility that the style sheet change will break some sites.

#18 @costdev
19 months ago

Thanks for your feedback @joedolson!

I agree with you that the first option is the better one.
The patch is ready for commit when someone has time.

#19 @sabernhardt
19 months ago

  • Keywords commit added

#20 @hellofromTonya
19 months ago

  • Owner set to hellofromTonya
  • Status changed from new to reviewing

#21 @hellofromTonya
19 months ago

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

In 52013:

Bundled Themes: Remove the "role" attribute on HTML elements with a default landmark role.

With IE11 no longer supported, the primary problem with assistive technology support for native HTML5 elements no longer applies.

This commit removes the role attribute from the following HTML5 elements with default landmark roles, per formerly required role attributes and W3C.

Follow-up to [17669], [21261], [23452], [24832], [29892], [38833], [40851], [43842], [46271], [49216].

Props costdev, craigfrancis, joedolson, mukesh27, ryokuhi, sabernhardt.
Fixes #54079.

#22 @sabernhardt
17 months ago

  • Keywords has-dev-note added; commit removed
Note: See TracTickets for help on using tickets.