Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years 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 3 years ago.
54079.1.diff (25.3 KB) - added by costdev 3 years ago.
Option 1 - Keep role="contentinfo" for Twenty Twelve footer.
54079.2.diff (26.6 KB) - added by costdev 3 years 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
3 years ago

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


3 years ago

#2 @mukesh27
3 years 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
3 years 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 3 years ago by costdev (previous) (diff)

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


3 years ago

#5 @ryokuhi
3 years 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
3 years ago

@ryokuhi Thanks for reviewing the ticket today! :)

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

#7 @costdev
3 years ago

  • Focuses css added
  • Keywords 2nd-opinion added

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


3 years ago

#9 @craigfrancis
3 years 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 3 years ago by craigfrancis (previous) (diff)

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


3 years ago

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


3 years ago

#12 @sabernhardt
3 years 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 3 years ago by sabernhardt (previous) (diff)

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


3 years ago

@costdev
3 years ago

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

@costdev
3 years ago

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

#14 @costdev
3 years 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
3 years 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.


3 years ago

#17 @joedolson
3 years 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
3 years 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
3 years ago

  • Keywords commit added

#20 @hellofromTonya
3 years ago

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

#21 @hellofromTonya
3 years 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
3 years ago

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