1703617 - Accessibility review for native context menus

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, task, P1)

Description

[Markus Stange [:mstange]]

Hi all,

over the past weeks/months I've been working on making Firefox use native context menus on macOS, rather than our historical non-native emulations. This work is tracked in bug 34572. As of a few days ago, native context menus are now ready for testing on Nightly and most known bugs are fixed. Native context menus are currently behind two off-by-default prefs: browser.proton.enabled and widget.macos.native-context-menus, and we have bug 1700109 and bug 1700679 for enabling those prefs by default.

Description:
This is a platform change to use the Cocoa classes NSMenu and NSMenuItem for all Firefox context menus. Other types of menus, such as toolbarbutton dropdown menus, bookmark toolbar folder dropdowns, or <select> popups, will remain non-native for now. The menu's root NSMenu is opened using -[NSMenu popUpContextMenu:withEvent:forView:].
The NSMenu contents are created based on the XUL structure inside the <menupopup>, i.e. based on the <menupopup>, <menuitem>, <menu> and <menuseparator> elements.
Interactions with the native menu are mirrored into XUL-land in such a way to keep existing front-end code working. For example:

• Opening and closing the menu fires popupshowing/popupshown/popuphiding/popuphidden events as expected.
• Open menus have menupopupElement.state == "open".
• Hovered menu items fire DOMMenuItemActive events.
• When a menu item is clicked / activated, its command event fires.
• When a checkbox/radio menu item is activated, the checked attribute is updated as appropriate.
• Dynamic DOM changes during the popupshowing event are reflected in the menu contents. And some DOM changes (such as changes to the menuitem label) are reflected even if they change while the menu is open.

How do we test this?

1. Set the two prefs mentioned above.
2. Right-click to open a context menu, anywhere where Firefox uses context menus.

When will this ship? I'm trying to ship this in 89 because it's considered part of the MR1 release.
Tracking bug/issue: bug 34572
Design documents (e.g. Product Requirements Document, UI spec): bug 34572 comment 22 (somewhat outdated)
Engineering lead: Markus Stange
Product manager: Martin Balfanz

Please describe the accessibility guidelines you considered and what steps you've taken to address them:
I've skimmed them but they don't seem particularly relevant, given that this is a rather low-level change.

Describe any areas of concern to which you want the accessibility team to give special attention:
My initial expectation was that native context menus should give us "native accessibility" for free, because we'll be behaving more like any other native macOS app, and not so much like a special snowflake. So I wasn't expecting much trouble. However, as shown by bug 1703482, clearly there is at least some amount of trouble.
Other areas that deserve special attention are:

• Not sure if relevant, but we should make sure that focus works as expected during menu interactions.
• If there are "notifications" for various menu state changes, we should make sure that these fire as appropriate. And we should make sure we're not double-firing them. For example, since we mirror state changes from the native menu into Gecko, I could imagine that both macOS and Gecko may try to notify for certain changes.

Comment 1

[Markus Stange [:mstange]]

(In reply to Markus Stange [:mstange] from comment #0)

For example, since we mirror state changes from the native menu into Gecko, I could imagine that both macOS and Gecko may try to notify for certain changes.

It turns out this is indeed the case and it is the cause of bug 1703482.

Comment 2

[Morgan Reschenberg [:morgan]]

if I launch the menu with the mouse, sometimes it closes before I can even try to navigate it. It looks like the menu opens on mousedown, and if mouseup isn't very-very soon after, the menu will close on mouse up. Maybe this is similar to bug 1696320? I don't have this problem with non-native menus -- they stay open through a subsequent mouse-up even if its seconds later. I have to mouse-down or esc to get the non-native menu to close.

This happens irrespective of VO, so its a problem for all users. Can you investigate, mstange?

STR:

1. Enable native menus, navigate to any content.
2. Press and hold the right click mouse button
   expected, actual: the menu opens
3. Release the right click button
   expected: the menu stays open
   actual: the menu closes

Comment 3

[Markus Stange [:mstange]]

I'm pretty sure this is expected behavior of native menus and you will see the same in all other apps on macOS.

Comment 4

[Morgan Reschenberg [:morgan]]

(In reply to Markus Stange [:mstange] from comment #3)

I'm pretty sure this is expected behavior of native menus and you will see the same in all other apps on macOS.

Gotcha, yeah just tested in Safari and Chrome. I'm assuming there's not a lot we can do about it, then, if its part of the native implementation? I flag it because users with motor disabilities may find the new menus difficult to use, especially given our current (non-native) behaviour. I also find when you try to open the menu with the mouse while VO is on, this happens nearly every time -- it opens then immediately closes :( maybe the mouse events are getting delayed somehow

Comment 5

[Markus Stange [:mstange]]

(In reply to Morgan Reschenberg [:morgan] from comment #4)

Gotcha, yeah just tested in Safari and Chrome. I'm assuming there's not a lot we can do about it, then, if its part of the native implementation?

That's right, this is outside of our control.

I also find when you try to open the menu with the mouse while VO is on, this happens nearly every time -- it opens then immediately closes :( maybe the mouse events are getting delayed somehow

Interesting! That's not good. But it sounds like a performance issue that we should be able to fix, especially if it's not happening in other browsers.

Comment 6

[Markus Stange [:mstange]]

(In reply to Morgan Reschenberg [:morgan] from comment #4)

I flag it because users with motor disabilities may find the new menus difficult to use, especially given our current (non-native) behaviour.

Hmm, given that it's consistent with all other mac applications, I would not be too concerned about this. If this really is creating usability barriers for some users, they can still turn off the widget.macos.native-context-menus pref. I expect us to keep the pref around for quite some time, because most of the non-native menu code is cross-platform so we can't remove it anyway.

Comment 7

[Morgan Reschenberg [:morgan]]

hihi!

so we've isolated two issues (bugs above) that are regressions from switching to native menus in firefox. as I mentioned on the bugs, both of these issues are reproducible in safari and chrome, indicating they're likely bugs within apple's implementation of VO.

I'm going to reach out to our apple a11y contact to see if they have bugs on these issues internally, but in the meantime they probably shouldn't block shipping native context menus. that said, if we can find a workaround so that firefox doesn't experience these issues, that'd be ideal.

bug 1705157 is likely a focus issue -- when we were using non-native menus, our a11y code managed the focus transition from focused item in content to focused context menu item after the context menu was opened. now that the context menu item focus is being handled on the native side, we're probably experiencing a problem where VO focus in firefox is in a weird transient state. if we can find a way to ensure when a user launches a context menu that the item in content loses focus and the menu item gains focus, that'd likely fix this issue. maybe there's a blur notification that we can use to force VO to stop focusing in firefox? eitan mentioned that the native menu already fires a FocusedUIElementChanged notif, which should pull VO there after its "released" from firefox

bug 1707110 is maybe a dom/targeting issue -- when eitan and I chatted, he noticed that launching the context menu on twitter's password input resulted in an AXShowMenu event that had a landmark as a target, not the input. if we can find a way to specify the event target as the input, that'd fix this.

we have VO event and action debugging tools here run --help after building to see how to use :)

Comment 8

[Morgan Reschenberg [:morgan]]

Overall -- I don't think the above bugs should block this review, though they should definitely be investigated since it looks like the former may be an engine bug (see follow up here: https://bugs.webkit.org/show_bug.cgi?id=225069). The later is inconsistently reproducible in Saf/Chrome and consistently reproducible in FF, so it may be more easy to track down.

Otherwise, native menus look good from an a11y standpoint, so I'll mark this bug as resolved 😀

Comment 9

[James Teh [:Jamie]]

(Removed unintentional comment.)

Comment 10

[Mike Conley (:mconley) (:⚙️)]

For folks scanning bugs for uplift - this isn't marked as fixed for 89, but I presume if all of the macOS native context menu stuff gets uplifted to beta, we can assume that this bug is also "fixed" for 89. Otherwise, there's nothing here to uplift.