1547635 - PanelMultiView: Arrow keys don't navigate context menus

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect, P3)

Description

[James Teh [:Jamie]]

STR:

1. Pin a toolbar button to the overflow menu so the overflow button appears.
2. Open the overflow using the keyboard (control+l, tab to the Library button, right arrow to the overflow button, press space).
3. Open the context menu for the overflow item by pressing the applications key or shift+f10.
4. Press down arrow.
   • Expected: The first item in the context menu should be selected.
   • Actual: The second item in the overflow is focused.

The PanelMultiView keydown event is overriding the context menu's keyboard handling. I wouldn't have thought this should happen; it's a capturing window event listener, but not a system group listener... but apparently, it does.

Gijs, do you have any idea how I can get around this? Is there some way I can detect whether the context menu is open in the keydown handler and bale if so?

Comment 1

[:Gijs (he/him)]

(In reply to James Teh [:Jamie] from comment #0)

Gijs, do you have any idea how I can get around this? Is there some way I can detect whether the context menu is open in the keydown handler and bale if so?

I think there's an "open" or "state" property on the menupopup element that you could check.

This might also affect the toolbar keyboard nav for the context menu there?

Comment 2

[James Teh [:Jamie]]

(In reply to :Gijs (he/him) from comment #1)

I think there's an "open" or "state" property on the menupopup element that you could check.

Assuming i can find the right menupopup. I'll take a look.

This might also affect the toolbar keyboard nav for the context menu there?

It doesn't; works as expected. However, it doesn't use a capturing listener, nor is the listener on the window, which is probably why it behaves more nicely.

Comment 3

[James Teh [:Jamie]]

Btw, I don't believe this affects 67, since overflow didn't use PMV key nav there and I think overflow is the only place where a PMV has a context menu.

Comment 4

[James Teh [:Jamie]]

Attachment: Bug 1547635: PanelMultiView: Don't override keyboard navigation in context menus.

Normally, context menu keyboard handling takes precedence.
However, because we have a capturing window keydown listener, our listener takes precedence.
Therefore, we need to check for an open context menu and suppress our keyboard handling in this case.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9518f9e5ecf9
PanelMultiView: Don't override keyboard navigation in context menus. r=Gijs

Comment 6

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/9518f9e5ecf9

Comment 7

[Florin Strugariu [:Bebe]]

== Change summary for alert #20860 (as of Wed, 08 May 2019 14:14:01 GMT) ==

Improvements:

7% raptor-tp6-docs-firefox loadtime linux64-shippable opt 1,034.04 -> 959.17
6% raptor-tp6-docs-firefox linux64-shippable opt 849.33 -> 794.44
5% raptor-tp6-docs-firefox fcp linux64-shippable opt 965.88 -> 918.25

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20860

Comment 8

[:Gijs (he/him)]

(In reply to Florin Strugariu [:Bebe] from comment #7)

== Change summary for alert #20860 (as of Wed, 08 May 2019 14:14:01 GMT) ==

Improvements:

7% raptor-tp6-docs-firefox loadtime linux64-shippable opt 1,034.04 -> 959.17
6% raptor-tp6-docs-firefox linux64-shippable opt 849.33 -> 794.44
5% raptor-tp6-docs-firefox fcp linux64-shippable opt 965.88 -> 918.25

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=20860

This changeset can't really have affected pageloads, so this is almost certainly mis-attributed. Is there another cset that might be responsible for this change?

Comment 9

[Florin Strugariu [:Bebe]]

you are right

it was Bug 1548358 Consider to finish forget skippable phase sooner if there is idle time to use