Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some keyboard shortcuts steal focus from modal dialogs #55899

Open
afercia opened this issue Nov 6, 2023 · 4 comments
Open

Some keyboard shortcuts steal focus from modal dialogs #55899

afercia opened this issue Nov 6, 2023 · 4 comments
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Nov 6, 2023

Description

Discovered while investigating #55894

When a modal dialog is open, no other UI should be perceivable or be interacted with. The purpose of a modal dialog is, in fact, to prevent interaction with the rest of the UI.

However, some actions that are available via keyboard shortcuts, do steal focus from an open modal dialog. This should never happen. Focus should always stay within a modal dialog until users explicitly want to close the modal dialog.

Step-by-step reproduction instructions

  • Go to the post editor.
  • Click on a paragraph.
  • Make an edit to the paragraph so to create an Undo step.
  • Open a modal dialog e.g. press Command+K to open the Command Center or press Command+Option+H to open the Keyboard shortcuts modal dialog.
  • While the modal dialog is open, try the following keyboard shortcuts and observe they all move focus outside the modal dialog.
    • Command+Z to undo the edit you previously made to the paragraph.
    • Shift+Command+Z to redo your edit.
    • Control+Option+O to open the Document Overview.
    • Option+F10 to move focus to the selected block toolbar.
    • Potentially any other keyboard shortcut that moves focus to the selected block or other parts of the UI.
  • Observe that when focus gets stealed from the open modal dialog, it is possible to navigate all the UI behind the modal dialog by pressing the Tab key.
  • With screen readers, this has potentially disastrous effects because all the content outside the modal dialog gets aria-hidden="true" to be made 'inert'.

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@afercia afercia added [Type] Bug An existing feature does not function as intended [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components labels Nov 6, 2023
@Rishit30G
Copy link
Contributor

Thanks for sharing the issue,

Here’s the screencast of the issue for a quick overview:

Screen.Recording.2025-02-13.at.11.47.37.AM.mov

@stokesman
Copy link
Contributor

I wanted to add another shortcut this surfaces with is the region navigation shortcut 😢.

Also a related issue #41503.

I had tried fixing the above by having Modal make all other content inert #53829 (which would likely fix this too) but ran across a rather obscure bug that seemed like a blocker. Every so often the inert content would remain inert even after the attribute was removed. It seemed like an actual browser bug. I should probably revisit and see if it still happens.

@afercia
Copy link
Contributor Author

afercia commented Feb 19, 2025

@stokesman yes the reason why inert isn't used for the Modal yet is that, well... the browsers inert implementation is buggy. There have been other conversations about using inert and so far the recommendation is to not use it. Hopefully browsers support will improve so that the usage of inert can be reconsidered. Anyways, the usage of inert should be very well considered to avoid bugs like, for example, the one in #67078

@stokesman
Copy link
Contributor

the usage of inert should be very well considered to avoid bugs like, for example, the one in #67078

Makes sense. Thanks for highlighting that issue.

I had tried fixing the above by having Modal make all other content inert #53829 (which would likely fix this too)

I wanted to report that PR keeps focus from moving out of the modal for these shortcuts I’ve tested:

  • region navigation
  • document overview
  • navigate to the nearest toolbar

Undo and redo can still escape the focus trap. The editor’s iframe is somehow able to gain focus despite being inside of an inert ancestor. If that can be solved it could qualify a fix for this issue but it looks to be less than straightforward. Additionally, it’s worth noting that would still leave the effects of some shortcuts happening behind the modal and that seems like an issue too.

Another route to avoiding this issue would be disabling keyboard shortcuts while a modal is open. Some discussion on how to approach disabling shortcuts is in #18755. An API could be added to the keyboard-shortcuts package and used throughout Gutenberg’s modals. That would leave third-party use of modals with this potential pitfall. There is a more general solution that could be applied to the base Modal component as an alternative #18755 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Package] Components /packages/components [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

3 participants