Problem/Motivation

On the mobile version, the "modal" overlay for the expanded sidebar has an invalid ARIA attribute: aria-expanded.

Accessibility Insights report

  Title: WCAG 4.1.2: Ensure an element's role supports its ARIA attributes (.admin-toolbar-overlay)
  Tags: Accessibility, WCAG 4.1.2, aria-allowed-attr

  Issue: Ensure an element's role supports its ARIA attributes (aria-allowed-attr - https://accessibilityinsights.io/info-examples/web/aria-allowed-attr)

  Target application: admin | Drush Site-Install - https://drupal-core-dev.ddev.site/user/1/edit?pass-reset-token=3M28uFtT7L8cxINMn9OnEPObJbFuTcfPcIyVtmtD9LfeLhvgzJJRDLLVO4s-3MZxDbqgcDOnOQ&check_logged_in=1

  Element path: .admin-toolbar-overlay

  Snippet: <div class="admin-toolbar-overlay" aria-controls="admin-toolbar" data-drupal-admin-styles="" data-once="admin-toolbar-trigger" aria-expanded="true"></div>

  How to fix:
  Fix all of the following:
    ARIA attribute is not allowed: aria-expanded="true"

Screenshot

screenshot of invalid attribute error message from accessibility insights

WCAG success criteria

Name, Role, Value (Level A)

Steps to reproduce

  1. Do a standard installation.
  2. Enable the navigation module.
  3. Log in as admin.
  4. Go to a page that has the navigation sidebar.
  5. Reduce the viewport width until the "mobile" version of the sidebar is displayed (currently ~1000px).
  6. Expand the sidebar if it's closed.
  7. Run a test with Axe or Accessibility Insights.

Code snippet

<div class="admin-toolbar-overlay" aria-controls="admin-toolbar" data-drupal-admin-styles="" data-once="admin-toolbar-trigger" aria-expanded="true"></div>

Proposed resolution

The problem appears to be that the overlay is a simple div with no special role, but it's a pseudo disclosure widget control. The sidebar looks like a hybrid disclosure widget / modal dialog.

aria-expanded can only be used on certain roles.

The web.dev native dialog example doesn't close on a click outside the dialog, so that's not a helpful example.

The Modal Dialog Example of the ARIA Authoring Practices Guide (APG) uses a similar modal overlay ("backdrop") that does not have aria-expanded or aria-controls.

Based on the APG example, I propose:

  • Remove the aria-expanded attribute.
  • Possibly also remove the aria-controls attribute.
  • Update the accessibility test to also check with the sidebar expanded on a smaller viewport.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#25 3551528-25-local-test-axe-invalid-attribute-error.png311.35 KBkentr
Screenshot 2025-10-10 at 10.29.44 AM.png504.31 KBkentr

Issue fork drupal-3551528

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kentr created an issue. See original summary.

shqshqnk made their first commit to this issue’s fork.

shqshqnk’s picture

shqshqnk commented

I am starting on it

shqshqnk opened merge request !13494

shqshqnk’s picture

shqshqnk commented
Status: Active » Needs review

I have removed the invalid aria attributes as well as Moved the overlay to be before the sidebar so that clicking on the overlay shifts the focus back to the sidebar

kentr’s picture

kentr commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Still needs the test updated, or a new test, to prove that it's fixed.

@shqshqnk

as well as Moved the overlay to be before the sidebar so that clicking on the overlay shits the focus back to the sidebar

Please explain this. It sounds like a separate problem that should be in another issue.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

mgifford’s picture

mgifford
he/him
Primary language English
commented

In discussion with @mherchel @rkoller & @kat-shaw we decided that this was a barrier. This just is semantically wrong and needs to be addressed.

mherchel made their first commit to this issue’s fork.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Hiding previous branch. It's out of date, doesn't address the JS changes, and also doesn't point to main

New branch incoming.

mherchel changed the visibility of the branch 3551528-mobile-expanded-sidebar to hidden.

mherchel opened merge request !14736

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Needs review

This should be good to go and is ready for review.

rkoller’s picture

rkoller
he/him
Primary language German
Location Nürnberg, Germany
commented

checked out the MR and the aria-expanded attribute is getting properly removed. looks good to me.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

This is one would ask if we could maybe add a small assertion to an existing test. Since aria bugs popup frequently

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

@smustgrave This would be better served by modifying existing accessibility tests. Sure we can test one specific element if it has aria-expanded. But, there's no reason it would ever be added, except on purpose.

If we test this one <div> for aria attributes, who's to say that we should test all <div>'s and other elements?

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs work » Needs review

Setting to NR to get additional feedback.

smustgrave’s picture

smustgrave commented

But isn’t this just targeting navigation? Think an assertion could be added to that

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

I'm a bit confused. What do you want to test? The only thing that I can think of is that the aria-expanded attribute does not exist.

If thats the case, I'm not sure what the point is. There's almost a zero percent chance someone will add that back. And if we do test for invalid aria-expanded attributes. Why stop at just this element, and just that attribute?

smustgrave’s picture

smustgrave commented

I’ll look at this again later this week

kentr’s picture

kentr commented

But isn’t this just targeting navigation? Think an assertion could be added to that

Not sure if this is what @mherchel meant, but Navigation has its own "general" accessibility test..

The test appears to lack coverage for the "mobile" version, though. I suspect it would have caught this issue if it had coverage.

There's support in #accessibility for expanding Axe test coverage, so I propose adding coverage of the "mobile" version to that test (or as separate test in the same file, since the setup is a little different for "mobile").

I'm happy to do the work, but I might not finish it for a couple of days.

smustgrave’s picture

smustgrave commented

Disregard my previous comments. Sees only bit missing from the summary is

Update the accessibility test to also check with the sidebar expanded on a smaller viewport.

Correct?

kentr’s picture

kentr commented
Status: Needs review » Needs work

@smustgrave:

Sees only bit missing from the summary is

Update the accessibility test to also check with the sidebar expanded on a smaller viewport.

Correct?

Heh. I forgot that was in the issue summary...

Yes, I'd say that's correct.

kentr’s picture

kentr commented
Issue summary: View changes
kentr’s picture

kentr commented
Status: Needs work » Needs review
Issue tags: -Needs tests
StatusFileSize
new 3551528-25-local-test-axe-invalid-attribute-error.png311.35 KB

I extended the Axe test to cover the expanded "mobile" sidebar.

There was random test failure that passed when re-run.

I ran the modified test against main, and it did catch the invalid attribute.

The Test Only job passed (false negative), but it looks like there's an underlying problem with the job and the tests didn't even run.

Here's a screenshot of the local test output showing the invalid attribute failure message from Axe:

Commands to run the single test against main:

git checkout main
git checkout -f 3551528-remove-aria-expanded-nav-overlay -- \
  core/modules/navigation/tests/src/Nightwatch/Tests/a11yTest.js
cd core
yarn install
yarn run test:nightwatch \
 modules/navigation/tests/src/Nightwatch/Tests/a11yTest.js
kentr’s picture

kentr commented

it looks like there's an underlying problem with the job and the tests didn't even run.

Apparently there's no underlying problem.

Looks like the Test Only job only runs PHPUnit.

bernardm28’s picture

bernardm28 commented
Status: Needs review » Reviewed & tested by the community

Seems like a good workaround

  • lauriii committed 87c6eea5 on main 
    fix: #3551528 Mobile expanded sidebar modal overlay has invalid ARIA...

  • lauriii committed 14b4cab4 on 11.x 
    fix: #3551528 Mobile expanded sidebar modal overlay has invalid ARIA...

lauriii closed merge request !14736

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Committed! 🚀

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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