WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 5 weeks ago

Last modified 5 weeks ago

#54054 closed enhancement (fixed)

Remove role="navigation" from `nav` elements

Reported by: ilovewpcom Owned by: joedolson
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch needs-dev-note commit
Focuses: accessibility, coding-standards Cc:

Description

According to HTML5 documentation, adding role="navigation" to a <nav> element is unnecessary (and not permitted).
It should be added to elements like <div> instead, when using the semantically correct <nav> is not possible.

Meanwhile this is the HTML markup for the the_posts_pagination() function, which comes through the _navigation_markup() function in the /wp-includes/link-template.php file.

    $template = '
    <nav class="navigation %1$s" role="navigation" aria-label="%4$s">
        <h2 class="screen-reader-text">%2$s</h2>
        <div class="nav-links">%3$s</div>
    </nav>';

References:
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/Navigation_Role
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/nav

Change History (23)

This ticket was mentioned in PR #1648 on WordPress/wordpress-develop by costdev.


3 months ago

  • Keywords has-patch added

#2 @costdev
3 months ago

  • Focuses accessibility coding-standards added

The PR removes all instances of role="navigation" on nav elements, including in the base themes.

#3 @mukesh27
3 months ago

  • Keywords close added
  • Version 5.8 deleted

Hi thanks for the ticket.

@costdev thanks for the patch.

Same ticket like #48920. Let's wait for other dev to check till now let's mark it close

#4 @costdev
3 months ago

Yeah @mukesh27 the related ticket highlights the same issue in another area - both areas (and more) are covered by this patch, nice find!

The WebAIM survey mentioned in the related ticket is an older version - WebAIM has newer results.

IE11 screen reader usage (WebAIM):
2017: 23.3%
2019: 10.9%
2021: 3.3%

Many screen reader developers are recommending the newer browsers now. For those that don't recommend any particular browser, the screen readers seem to function in at least some of the modern browsers.

It might be worth a discussion (?) about what % usage is the acceptable cut-off point to bring things in line with HTML5 documentation.

Version 1, edited 3 months ago by costdev (previous) (next) (diff)

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


3 months ago

#6 @joedolson
3 months ago

The primary problem with assistive technology support for the native HTML5 elements was directly related to IE - even if the assistive technology supports native landmark elements, that support is only meaningful if the browser also supports them, since otherwise the information wouldn't be added to the accessibility tree that the AT interacts with.

So, in my opinion, with the removal of IE11 support, it's reasonable to also remove role attributes on landmark elements throughout WordPress. There may be some situations where this will reduce accessibility, but they would be vanishingly rare.

The accessibility-ready theme guidelines stopped requiring the use of both the role attribute and the native element in January 2021. (https://make.wordpress.org/themes/handbook/review/accessibility/required/#aria-landmark-roles)

#7 @alexstine
3 months ago

I always got confused in regards to where I should use roles or just HTML5 elements so I used them both. However, if there is no longer a negative effect that could be caused by some browsers/screen readers not fully supporting the native HTML5 elements, this has my support to drop the roles. It makes the code a bit cleaner and simpler.

#8 @costdev
3 months ago

  • Keywords close removed

Thanks for the feedback @joedolson! I'm removing close now that we've reached a stage where all instances of role="navigation" on nav elements can now be removed. I believe the ticket is ready for a milestone and the PR ready for commit if a committer can take this further?

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

#9 @mukesh27
3 months ago

  • Milestone changed from Awaiting Review to 5.9

PR looks good to me.

Let's wait for other dev feedback on PR.

Milestone set to 5.9

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


2 months ago

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


8 weeks ago

#12 @joedolson
8 weeks ago

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

#13 @joedolson
6 weeks ago

  • Summary changed from Remove role="navigation" from the markup of the the_posts_pagination() function to Remove role="navigation" from `nav` elements

This patch reaches well beyond the original ticket scope - removing all role=navigation is definitely broader than just removing it from posts navigation. I think that's reasonable, and the patch looks sound, but I'm going to change the ticket name & re-raise this in an accessibility meeting before moving to commit.

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


6 weeks ago

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


6 weeks ago

#16 @joedolson
6 weeks ago

  • Keywords needs-dev-note commit added
  • Status changed from reviewing to accepted

#17 @joedolson
5 weeks ago

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

In 51967:

General: Remove role="navigation" from nav elements.

Role="navigation" was required for assistive technology to recognize HTML5 element's native ARIA roles while HTML5 and ARIA were being introduced. With the deprecation of IE11, the role attribute is only required when mapping elements that don't have native role.

Props costdev, mukesh27.
Fixes #54054.

#18 @TobiasBg
5 weeks ago

Just to make sure: Where the changes to package.json intentional? The commit message doesn't explain them.

#19 @joedolson
5 weeks ago

Whoops. Will fix that.

#20 @joedolson
5 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#21 @joedolson
5 weeks ago

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

In 51974:

Build/Test Tools: Restore changes to package.json.

Reverts changes to package.json and package-lock.json accidentally committed in [51967].

Props tobiasbg.
Fixes #54054.

#22 @SergeyBiryukov
5 weeks ago

In 52000:

Build/Test Tools: Restore the https URL for browserify-aes.

Follow-up to [51967], [51974].

See #54054.

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


5 weeks ago

Note: See TracTickets for help on using tickets.