PanelMultiView: Arrow keys don't navigate context menus

RESOLVED FIXED in Firefox 68

Status

()

defect
P3
normal
RESOLVED FIXED
4 months ago
2 months ago

People

(Reporter: Jamie, Assigned: Jamie)

Tracking

unspecified
Firefox 68
Points:
---

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox66 unaffected, firefox67 unaffected, firefox68 fixed)

Details

Attachments

(1 attachment)

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?

Flags: needinfo?(gijskruitbosch+bugs)

(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?

Flags: needinfo?(gijskruitbosch+bugs)

(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.

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.

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.

Pushed by jteh@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9518f9e5ecf9
PanelMultiView: Don't override keyboard navigation in context menus. r=Gijs
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 68

== 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

(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?

Flags: needinfo?(fstrugariu)

you are right

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

Flags: needinfo?(fstrugariu)
Regressions: 1456725
See Also: → 1561581
You need to log in before you can comment on or make changes to this bug.