Mobile version of Navigation should have focus trap

I agree with mherchel that when a user enters the mobile nav, keyboard focus must be trapped within it until the user:

* Selects the Close/OK/X button
* Selects the ESC key
* Clicks outside of the menu with their mouse

However I disagree on the WCAG success criteria that this violates. I believe that this violates 2.1.2 No Keyboard Trap (Level A). Under an example in this SC titled "A modal dialog box", it mentions:

A Web application brings up a dialog box. At the bottom of the dialog are two buttons, Cancel and OK. When the dialog has been opened, focus is trapped within the dialog; tabbing from the last control in the dialog takes focus to the first control in the dialog. The dialog is dismissed by activating the Cancel button or the OK button.

The WCAG 2.4.7 Focus Visible (Level AA) SC refers to the visible focus indicator for focusable elements.

I like Mike's idea.

Probably it is possible to merge that core/misc/focus-trap.js logic with existing arrow navigation:

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/navig...

Or move it on core level out of module.

imo it should be possible to ignore desktop somehow.

The existing tabbingmanager feature has tests, this one should too

I believe this would be SC 2.4.11 Focus Not Obscured (Minimum) (Level AA). It directly addresses user openable, persistent disclosures and uses slide-out navigation as an example.

When the additional content is opened, it takes focus and the tab ring is constrained to the new content until it is dismissed. This modality is somewhat like a dialog, in that a user cannot navigate beyond the opened content by keyboard without dismissing it first (typically by pressing Esc). However, unlike in a modal dialog, in some implementations a pointer user may be able to interact with content outside the opened section without dismissing it. Since this pattern potentially creates an inequitable experience between keyboard and pointer users, it should be used cautiously. That said, it does prevent the opened content from obscuring the keyboard focus in the main content, and thus should pass this SC. This is described and demonstrated in a short video in the Knowbility article in the reference section, under the section heading Keep keyboard focus in the slide-out navigation until it's closed.

Tagging

Can I ask to help get this through faster that maybe we move the olivero change to a follow up?

Rebased.

If the "mobile" navigation should be a modal dialog, I think adding keyboard listeners are no longer the way to implement it.

- Hijacking Tab, Shift-Tab prevents keyboard users from navigating to the browser UI while the navigation is open.
- Keyboard listeners don't prevent screen reader users from navigating beyond the navigation.

These were always problems with the keyboard listeners but the alternative was more challenging to implement.

Instead, the mobile navigation should be contained within a <dialog> element that's opened using .showModal(). The dialog will be modal, preventing access to the rest of the page (including by screen readers), while not preventing keyboard access to the browser UI. It closes when the ESC key is pressed or when a close button is used. The close button does not have to be scripted:

<form action="." method="dialog">
  <button>Close</button>
</form>

To close the dialog when a pointer clicks outside of it (aka "light dismiss"), you can use <dialog closedby="any">. closedby is new and not supported in all browsers but it doesn't take many lines of JavaScript to check for support and implement a fallback.

However the dialog is closed, focus will automatically return to the button that opened it.

I would go even further and propose implementing it using the show-modal Invoker command instead, using JavaScript as a fallback in browsers that don't support them (or add the full Invoker command polyfill as a resource any module can use).

So this is going to be our first "a11y-bugsmash" ONE!

Starting the triage

Can we update the summary to use the standard template. Sounds like agreement for the solution is to keep the scope to the navigation module and only trigger the trap on mobile? Someone correct me if I'm wrong.

Also if possible could we get small gifs of the trap before/after?

For test coverage a javascript test could be added or maybe even just expanding an existing one. Something like

put focus at the top
Doing a loop based on the number of elements (would have to know that) press the tab key
verify focus is still on the top.

@mherchel:

With the patch in #25, the "Skip to main content" link and the "Expand sidebar" button are still focusable when the sidebar is open. So, the focus disappears for a bit when tabbing through them.

Is this expected?

I was mistaken. The skip link isn't focusable. But the hidden Expand sidebar button is.

After that, the next item in the ring is the Home Drupalicon button at top-left of sidebar, but it doesn't have a focus indicator.

To me, the Expand sidebar button shouldn't be tab-able when the sidebar is open, because it's hidden and there is a close button.

I also think the sidebar should be closable with the ESCAPE key and that the close button should be tab-able. But those are separate issues.

The three vertical dots menu toggle button (top-right of screen) is also tab-able with the sidebar open. Seems like it shouldn't be.

Just going to chime in if we can still update the summary and track open questions in th remaining tasks section? May help others

hm i've checked out MR14669 and quickly tested on safari 26.2 on macOS 26.2 as well as latest edge and in each case the focus wasnt trapped within the mobile navigation.

and yep the close button should be reachable by keyboard same as there should be an option to close the mobile navigation by pressing the ESC key. but from my perspective if moved to a follow up those followup(s) and this issue should go into the same new version of drupal.

This means there's no way to close (that I can find) via keyboard. We'll need a followup for this

Yeah, I realized this after the fact.

So that we don't cause a regression (removing the only way to close by keyboard), would it be in scope to change the close button in this issue?

I'm working on updating the IS.

I think this can be tested with functional javascript.

Updated the IS, but still needs screenshots.

Removed Portland2024 tag.

I made a prototype to demonstrate the approach I've suggested. It uses a modal dialog when the navigation should cover the page. When the viewport is wide enough, it's non-modal and beside the page.

There's an effort to use native dialogs throughout core: #2158943: Add a native dialog element to deprecate the jQuery UI dialog.

I wonder if whatever we do with dialog here needs to be coordinated with that effort.

@cwilcox808 Nice prototype!

Maybe what we do here will influence that global effort 🙂.

re #41 & #42 recorded one video (no_trap.mp4) clearing the cache again and then tabbing through the interface and recorded a second video (rotor.mp4) with the rotor. the latter illustrates that everything is available in the AOM and therefore in consequence also in the rotor.

Funny I tested the MR and while tabbing I ended up out of the webpage and tabbing the actual development bar in the browser

i am on tahoe 26.2, tested in safari 26.2, and the latest firefox developer edition, and the latest version of edge. in all of them the same result i am able to tab out. and no_trap.mp4 illustrated that, there i'Ve tabbed out with the keyboard.

and testing out oliveros mobile menu, you mean go to for example the front page in olivero? that is what i actually did in the no_trap.mp4 (the behavior on a claro page is identical)

uh now i'Ve realized what you meant with the olivero menu, i was completely on the wrong train of thought. recorded another video (menu.mp4). i've activated keycastr. that way you are able to see which keystrokes were entered. i was only tabbing through the page with a mobile viewport. first opening the navigation sidebar then tabbing on, tabbing out of the navigation sidebar and then when reaching the menu button for olivero opening it and then tabbing within the olivero menu. there in contrast to the navigation sidebar the focus is trapped. hope that helps.

update: also tested the same approach in the latest safari technology preview, firefox developer edition, and edge, and in all three the same behavior like illustrated in the menu.mp4 video for safari 26.2

nope that is the tab key in keycastr,. arrows would look like that: arrows.mp4

and cmd-r is simply reloading the page so the tabbing starts from the top of the DOM.

i guess i found one interesting detail, checkout aggregation.mp4 (recorded in edge)

first with the aggregation turned on (that was the case for me in previous videos) when i go to the front page and tab i tab through the navigation sidebar and onto the front page and am able to to expand also the olivero nav. then i go back to the aggregation page and deactivate aggregation. when i then go to the front page the focus is trap. but as you notice i experience the same like @smustgrave in #46, when the end of the navigation sidebar is reached instead of jumping directly to the top of the sidebar again the tab order takes a detour to the toolbar of the browser.

i experience the same like @smustgrave in #46, when the end of the navigation sidebar is reached instead of jumping directly to the top of the sidebar again the tab order takes a detour to the toolbar of the browser.

From my interpretation of #19, this is preferred.

Sounds like we don't want to prevent users from getting to the browser UI via keyboard, and that we don't really want a "trap".

Followup for the Close button: #3572169: Put the mobile sidebar close button into the TAB sequence

TIL we're not supposed to remove event-related tags when the event is over, unless no one from the event actually worked on the issue.

So seems like this is working as advertised right? Next step would be to get test coverage then. I'm not familiar enough with nightwatch to try that but feel javascript test should be possible too right?

finnsky changed the visibility of the branch 3443571-focus-trap to hidden.

I think my cat wanted to join the Drupal community.

Not sure that the fully native dialog implementation meets Drupal's supported browser requirements, though.

At least, closedby isn't supported on Safari.

We have to resolve this. This is something that we consider a blocker.

Added #3572169: Put the mobile sidebar close button into the TAB sequence and #3572628: Mobile sidebar should close by ESCAPE key to the Remaining tasks so that they don't fall through the cracks.

Don't we also need to move focus to the sidebar when it is opened?

#64 closedby and Invoker commands are not supported by all browsers that meet Drupal's system requirements but polyfills will make them work in browsers that lack native support. (Popovers are supported in all the browsers.)

Because the alternatives all require JavaScript to function, I would expect that including some JavaScript temporarily, until all supported browsers natively support the features, would be acceptable.

My prototype also required JavaScript to fully support the desired behavior but even if no JavaScript is run, the navigation can still be accessed in all supported browsers.

for reference. the issues i had in safari were due to some caching issues. a suggestion by @catch on slack solved the problem. by deleting files/css and files/css the focus remained trapped now consistently in safari, firefox and edge. the sole other nitpick i've raised on slack before, that the focus when it gets into the safari ui aka the addressbar was not visible is also obsolete. after the removal of those two directories and the caches newly generated it is also clearly visible now. so from a functional testing perspective this looks good. and also taking into consideration that the feedback on the a11y slack was also positive and confirming in regard of the approach taken here.

This is perfect! thanks @mherchel!

Manually testing I see that I do tab through the entire menu on mobile, as mentioned the sequence does go into the browser inspector, but comes back around to the mobile menu. So I can't get to any content on the page (as expected)

LGTM!

few things

Believe feedback from @nod_ has been addressed

not quite yet :)

Manually testing this still seems to work as advertised.

Committed! Thank you for everyone who worked on this 🙌

I think this has broken Drupal\Tests\demo_umami\FunctionalJavascript\AssetAggregationAcrossPagesTest on HEAD:

Asset Aggregation Across Pages (Drupal\Tests\demo_umami\FunctionalJavascript\AssetAggregationAcrossPages)
     ✔ Front and recipes pages
     ✔ Front and recipes pages authenticated
     ✘ Front and recipes pages editor
       ┐
       ├ Asserting ScriptBytes 206758 is greater or equal to 205500 and is smaller or equal to 206500
       ├ Failed asserting that 206758 ( is equal to 205500 or is greater than 205500 ) and ( is equal to 206500 or is less than 206500 ).
       │
       │ /builds/project/drupal/core/tests/Drupal/Tests/PerformanceTestTrait.php:636
       │ /builds/project/drupal/core/tests/Drupal/Tests/PerformanceTestTrait.php:685
       │ /builds/project/drupal/core/profiles/demo_umami/tests/src/FunctionalJavascript/AssetAggregationAcrossPagesTest.php:78
       ┴
     ✘ Node add pages author
       ┐
       ├ Asserting ScriptBytes 3769883 is greater or equal to 3767685 and is smaller or equal to 3768685
       ├ Failed asserting that 3769883 ( is equal to 3767685 or is greater than 3767685 ) and ( is equal to 3768685 or is less than 3768685 ).
       │
       │ /builds/project/drupal/core/tests/Drupal/Tests/PerformanceTestTrait.php:636
       │ /builds/project/drupal/core/tests/Drupal/Tests/PerformanceTestTrait.php:685
       │ /builds/project/drupal/core/profiles/demo_umami/tests/src/FunctionalJavascript/AssetAggregationAcrossPagesTest.php:106
       ┴

https://git.drupalcode.org/project/drupal/-/jobs/8558724

Fix: https://git.drupalcode.org/project/drupal/-/merge_requests/14826

Committed/pushed the test update, thanks!