Add aria-current attribute to navigation items

Adding https://www.w3.org/WAI/WCAG21/Understanding/name-role-value

Added aria-current attribute on pager and breadcrumb. Still needs to be added on menu items.

Rather than each theme needing to implement this, would it be possible to do as Luke.Leber suggests and have the attribute added more globally? It should probably be done anywhere the is-active class is applied to a menu link. For example, in the JS file /core/misc/active-link.es6.js and in the ActiveLinkResponseFilter.php class.

I've created a new MR with the approach of adding the aria-current="page" attribute everywhere (that I can find) that the is-active class is being added to a menu link. I've also created two patches, one for 10.1.x/10.0.x, and one for 9.5.x, in case anyone wants to use them.

According to the ARIA spec for the aria-current attribute, it should be added to a set of navigation links to programmatically indicate the current page among that set of links. It needs to be added to the link, not the list item or other element. And if the current item is not a link, the aria-current attribute is not needed. For reference see https://www.w3.org/TR/wai-aria-1.2/#aria-current and https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attribut....

On the latest patch for 3038523-aria-current-link-core-9.5.x-22.patch and 3038523-aria-current-link-core-10.x-22.patch,
I added menu items to the main navigation and looked at the is-active class and I do not see the aria-current attribute added.

I tested the Drupal 9.5 patch from #22, and it worked for me locally! I'm not sure why it didn't work for @irene_dobbs?

However, it looks like this also needs to update web/core/tests/Drupal/Tests/Core/EventSubscriber/ActiveLinkResponseFilterTest.php because there's a test failure in CI.

I've updated the MR to include a fix for ActiveLinkResponseFilterTest. I'm also attaching new patches for 9.5.x and 10.1.x/10.0.x.

Before testing the issue summary seem incomplete. Added the template for reference.

Updated issue summary.

For clarity I'm hiding the patches as MR 4959 is what would be committed.

Following the steps in the IS I can confirm the links with is-active don't have an aria attribute
Applying the MR
Can confirm the links do have aria-attribute now

Worth noting that the is-active class being set by the toolbar is unaffected with this MR.

Thanks for the review @smustgrave. I took a look at the admin toolbar on the standard install and noted that aria-current is being set to "page" for the active link in the toolbar. To reproduce:

1. Install the latest version of Drupal from this issue MR using the standard profile.
2. Log in as an administrator.
3. In the Manage menu, click Appearance.
4. While on the Appearance page, inspect the code for the Appearance link.
5. Note the is-active class is set and the aria-current attribute is set.
6. Click the Settings tab/link.
7. While on the Settings page, inspect the code for the Settings tab/link.
8. Note the is-active class is set and the aria-current attribute is set.
9. While still on the Settings page, inspect the code for the Appearance tab/link.
10. Note the is-active class is set, but the aria-current attribute is NOT set.
11. Click the Olivero tab/link.
12. While on the Olivero page, inspect the code for the Olivero tab/link.
13. Note the is-active class is set and the aria-current attribute is set.
14. While still on the Olivero page, inspect the code for the Settings tab/link.
15. Note the is-active class is set, but the aria-current attribute is NOT set.

This seems like the appropriate behavior to me - the links in the hierarchy above where you are have the visual styling, but only the actual current page has the aria-current attribute.

Should the aria-current attribute be used only on menus or should this also work on other kinds of links too?

For example, if I click "Find out more", should that have aria-current?

If that's the case, should we do it in a follow-up or should it be done here?

From reading https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attribut...

When you have a group of related elements, such as several links in a breadcrumb or steps in a multi-step flow, with one element in the group styled differently from the others to indicate to the sighted user that this is the current element within its group, the aria-current should be used to inform the assistive technology user what has been indicated via styling.

I don't think the Find out more link mentioned would count.

I've always believed that ad-hoc self-referencing links can be avoided by a solid content strategy.

...however I did want to raise one other question: what about Single Page Application-type use cases? Is this an outlier, or should we confirm that same page links don't have a fragment?

  <!-- None of these should probably have the attribute...? -->
  <menu>
    <a href="#">Top</a>
    <a href="#second">Second section</a>
    <a href="#third">Third section</a>
    <a href="#fourth">Fourth section</a>
  </menu>

@Luke.Leber because all we're doing is adding the aria-current attribute in places where core is already adding the is-active class, I would hope that the ActiveLinkResponseFilter class would do the right thing with menus that contain anchor links. If core is not already adding the is-active class where it needs adding, or if it's adding the class to multiple anchor links in the same menu, it might be best to fix that in a separate issue.

larowlan → changed the visibility of the branch 3038523-add-aria-current-attribute to hidden.

Issue credits

larowlan → closed merge request !4959

Took this for a manual test, tested menu links, breadcrumbs, local tasks, pagers. All looked good.

Read the MDN spec on it and it looks like we're doing things the correct way.

Committed to 11.x

Added and published a change-record

Thanks for fixing this for D11. Is there any chance that the patch will be updated for D10.2 (the patch at #25 fails to apply)

The 10.0/10.1 patch still applies manually to 10.2.x with a bunch of offsets. I've uploaded a new version that should apply to 10.2.x cleanly.

Automatically closed - issue fixed for 2 weeks with no activity.

Thank you @cboyden for the 10.2.x patch. I have applied this to my v10.2.5 Drupal site using composer. The patch applied cleanly and appears to provide the expected a11y enhancement.

I checked out the patch #40 for 10.2.x to reroll it for 10.3 and all the changes seem to be in core, though I don't see it noted. In case this is helpful to anyone else.