Use Menu key as accelerator on Android

RESOLVED FIXED

Status

()

Core
Widget: Android
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: mbrubeck, Assigned: mbrubeck)

Tracking

Trunk
All
Android
Points:
---
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(fennec-)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

8 years ago
On Android phones with hardware keyboards, the "Menu" key is the accelerator for keyboard shortcuts:
http://developer.android.com/guide/topics/ui/menus.html#shortcuts

We should support this in Fennec, with our existing keyboard shortcuts (e.g. Menu+L = open location bar, Menu+T = new tab).
Flags: in-litmus?
blocking beta 2 on a decision on whether or not we want this to be our interaction model
tracking-fennec: ? → 2.0b2+
Pinging beltzner for feedback here too.

Matt - how common is this on Android?
Assignee: nobody → mbrubeck
(Assignee)

Comment 3

8 years ago
It's supported in the stock Android browser, and in various other apps.  It's built into the UI framework for displaying menus.

It's not widely advertised or highly discoverable, so most users might not be aware of it (especially since most Android phones now have no hardware keyboard).
(Assignee)

Comment 4

8 years ago
Note: I personally find these keyboard shortcuts very useful on my G1, and implementing them would use the UI and code we already have for keyboard shortcuts on other platforms.
I'm OK with supporting this on Android. Let's do it.
2.0- but we would definitely take a patch for this
tracking-fennec: 2.0b2+ → 2.0-
(Assignee)

Comment 7

8 years ago
Created attachment 483613 [details] [diff] [review]
patch

Use the Menu key as the "control" modifier for key events.  If Menu is released before any other key is pressed, then it generates a command event instead (as it always does currently).

I didn't use Menu as a modifier for motion or gesture events, since I don't think we want to suppress the command event on keyup, in those cases.
Attachment #483613 - Flags: review?(mwu)
(Assignee)

Comment 8

8 years ago
Created attachment 483651 [details] [diff] [review]
patch v2

Get rid of gSym while we're at it.  SYM is not a modifier key.
Attachment #483613 - Attachment is obsolete: true
Attachment #483651 - Flags: review?(mwu)
Attachment #483613 - Flags: review?(mwu)

Comment 9

8 years ago
Comment on attachment 483651 [details] [diff] [review]
patch v2

Might want to set the keyCode to NS_VK_CONTROL for the menu key if that's what it is now. Up to you.
Attachment #483651 - Flags: review?(mwu) → review+
(Assignee)

Comment 10

8 years ago
Comment on attachment 483651 [details] [diff] [review]
patch v2

(In reply to comment #9)
> Might want to set the keyCode to NS_VK_CONTROL for the menu key if that's what
> it is now. Up to you.

We don't actually send regular key events for the menu key.  Because of its weird dual nature, I think this is still a good idea.  Treating it as a normal control key is probably an error.

Requesting approval2.0.  This patch is important for UI polish and for parity (with other Android browsers, and with Firefox on N900 and desktop).  It's low risk: Android only, very local change, only affects hardware keyboards - you can't do key combos on the Android soft keyboard.
Attachment #483651 - Flags: approval2.0?
Attachment #483651 - Flags: approval2.0? → approval2.0+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
(Assignee)

Comment 11

8 years ago
Created attachment 483671 [details] [diff] [review]
patch for checkin
Attachment #483651 - Attachment is obsolete: true
Attachment #483671 - Flags: review+
(Assignee)

Comment 12

8 years ago
http://hg.mozilla.org/mozilla-central/rev/7fb8a5dec505
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.