Main-window keyboard shortcuts don't work in Metro

RESOLVED FIXED

Status

RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Tracking

Trunk
x86_64
Windows 8.1

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [metro-preview][completed-elm])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
browser/metro/browser.xul defines a number of keyboard shortcuts like Accel-T (new tab) and Accel-L (focus location bar), but these aren't working for some reason; their command handlers are never called.
(Assignee)

Comment 1

6 years ago
For our cross-process key handling, we ignore key events in chrome (by disabling the keyset using "KeyFilter" in browser.js) until after the content process handles them and sends us back a Browser:KeyPress message.

We are never getting that message from content, because the "keypress" listener in Content.js is never getting called for some reason.  We need to fix this, or just remove all the cross-process key event handling code.
(Assignee)

Comment 2

6 years ago
Created attachment 658931 [details] [diff] [review]
remove cross-process keyboard code

This fixes the problem, though if we might want to go multi-process in the future then it would be better to figure out why Content.js never receives "keypress" events.
Attachment #658931 - Flags: review?(jmathies)
(Assignee)

Comment 3

6 years ago
Created attachment 658932 [details] [diff] [review]
remove cross-process keyboard code

Sorry, forgot to qrefresh.
Attachment #658931 - Attachment is obsolete: true
Attachment #658931 - Flags: review?(jmathies)
Attachment #658932 - Flags: review?(jmathies)
(Assignee)

Updated

6 years ago
Attachment #658932 - Attachment is patch: true

Comment 4

6 years ago
Comment on attachment 658932 [details] [diff] [review]
remove cross-process keyboard code

good riddance!
Attachment #658932 - Flags: review?(jmathies) → review+
Nice to see this fixed. Why was this only happening in Metro?
(Assignee)

Comment 6

6 years ago
http://hg.mozilla.org/projects/elm/rev/0489ded051ec

(In reply to David Bolter [:davidb] from comment #5)
> Why was this only happening in Metro?

The broken code was part of the Metro front-end code; it was never used in the desktop UI.  The code was riginally from e10s Fennec; I'm still not sure why it wasn't working correctly on Windows/Metro.  Based on past work like bug 776240 it looks like this code used to work; it's probably still a good idea to figure out when and how it regressed...
Whiteboard: metro-preview → [metro-preview][completed-elm]
(Assignee)

Comment 7

6 years ago
Pushed a followup to remove some more unused code that was spewing errors to the console trying to access the removed "Browser.keyFilter":
https://hg.mozilla.org/projects/elm/rev/6b02ff1d267d
(Assignee)

Updated

6 years ago
Blocks: 785473

Updated

6 years ago
Component: Keyboard Navigation → General
Product: Firefox → Firefox for Metro
Version: unspecified → Trunk
(Assignee)

Comment 8

6 years ago
Resolving bugs in the Firefox for Metro product that are fixed on the elm branch.  Sorry for the bugspam.  Search your email for "bugspam-elm" if you want to find and delete all of these messages at once.
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
OS: Windows 8 Metro → Windows 8.1
You need to log in before you can comment on or make changes to this bug.