WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.53k stars 4.21k forks source link

Fix Menu component ARIA pattern #67078

Open afercia opened 4 days ago

afercia commented 4 days ago

Description

Spotted while inspecting the new 'Preview' menu introduced in the global styles after https://github.com/WordPress/gutenberg/issues/66376

The Menu component is based on Ariakit. At a very first inspection, it appears there's at least two accessibility failures:

1 The menu may render a visually hidden button labeled 'Dismiss popup'. This breaks the ARIA menu pattern.

A menu should only contain menuitems, their variant menuitemradio/checkbox, and groups or separators. The 'Dismiss popup' button can't be tabbed to or focused by using arrows anyways. However, screen reader users can still move to it. I doubt the button label 'Dismiss popup' is translatable. Regardless, this button shouldn't be there in the first place.

Screenshots, where I'm also revealing the visually hidden button by removing some CSS:

Image

Image

It appears the Ariakit Menu uses the Ariakit Dialog internally, and this button rendering is controlled by the modal prop.

I'm not into discussing the Ariakit internal implementation, which I find arguable, but this is not OK. This button should be removed. At the very least, the Gutenberg implementation should explore to set the modal prop to false. However, I'm not sure about potential side effects when changing this prop value. Alternatively, the implementation should be fixed upstream.

2 The Menu may rander an aria-labelledby attribute on the element with role=menu that apparently points to an invalid ID. Actually, the referenced ID is in the DOM so that the labeling is supposed to work, theoretically. This was interesting to debug and it took some time.

By default, the aria-labelledby of the element with role=menu points to the trigger button that opens the menu. As such, the menu is supposed to be labeled with whatever the accessible name of the trigger button is. It's a sensible default.

However, when the modal prop is true, when the Ariakit menu opens it adds (via the internal DIalog component) an inert attribute to the sibling elements, which in WordPress happens to be: <div id="wpwrap" inert>.

This triggers an inconsistent behavior between menus with a trigger button that uses an icon+aria-label and the ones that use a trigger button with visible text. The inconsistency appears to related to browsers behavior as well. I was able to reproduce this bug with Chrome but wasn't with Firefox.

When inspecting the DOM via the Chrome dev tools > Elements > Accessibility panel:

Screenshot:

Image

This isn't surprising because the trigger button, which is responsible to provide the labeling for the menu, is actually inside an inert part of the DOM. Browsers are supposed to make any inert part not perceivable. As such, at least in Chrome, the aria-labelledby points to an element that is in an inert area. I guess this explains the invalid source warning in Chrome.

Why it works for menus with an icon trigger button then? I can only guess that icon buttons have a tooltip that renders outside the inert part of the DOM. It appears this helps Chrome locate the referenced element and compute the accessible name.

I'm not sure what is the expected browsers behavior regarding referenced elements that live within an inert area. Historically, even if a referenced element is visually hidden with CSS or the hidden attribute, browsers are expected to be able to reference them for things like aria-labelledby or aria-describedby. However, the inert case seems ot be different, at least in Chrome.

Regardless, this appears to be one more occurrences of untested usage of the inert attribute. In previous issues and PRs, there have been already reports that the inert attribute is problematic for accessibility and should be used with extreme care. In all cases, its usage should be thoughtfully tested which doesn't appear to be the case for the Ariakit implementation.

Since the inert attribute is only added when the modal prop is true, I suggest to reconsider the usage of this prop or implementa more safe mechanism to make the rest of the page not preceivable. The Gutenberg Modal component uses the aria-hidden attribute for a reason.

Step-by-step reproduction instructions

1

2

Image

Screenshots, screen recording, code snippet

No response

Environment info

No response

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

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

Please confirm which theme type you used for testing.