Closed
Bug 1229967
Opened 8 years ago
Closed 8 years ago
Hardware menu button no longer closes the menu
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox45 affected, firefox46 verified, firefox47 fixed, firefox48 fixed)
RESOLVED
FIXED
Firefox 48
People
(Reporter: mcomella, Assigned: JanH, Mentored)
References
Details
(Whiteboard: [lang=java])
Attachments
(1 file)
58 bytes,
text/x-review-board-request
|
ahunt
:
review+
lizzard
:
approval-mozilla-aurora+
lizzard
:
approval-mozilla-beta+
lizzard
:
approval-mozilla-release+
|
Details |
After bug 1209967, an open menu cannot be closed by a hardware menu key. I realized this was an issue but after digging into it for a little to no avail, figured it wasn't worth fixing over shipping the new feature. Work-around is to click outside the menu, as you would with a software menu key.
Reporter | ||
Comment 2•8 years ago
|
||
Capella, since you filed bug 1233153, I'm guessing you use a device with a hardware menu button as your primary device. How important is it that the hardware menu button close the 3-dot menu? I figured the change to always have the software menu button on the screen was more important than the ability to close the menu again by the hardware button (when you can click outside of it), but I don't use a device with a hardware menu button.
Flags: needinfo?(markcapella)
Comment 3•8 years ago
|
||
I haven't looked, but was assuming this'd be easy to fix. The GS3 for example was pretty popular so I can't be the only one who will fall into this case. If I couldn't hack a personal fix (as an average user), my options would be adjust to open one way / close another, or just use the 3-dot all the time, and both options there kinda feel wrong.
Flags: needinfo?(markcapella)
Reporter | ||
Comment 4•8 years ago
|
||
We spoke about this today during the front-end team meeting: Finkle doesn't think this is high priority if 1) we can close the menu by touching outside of it and 2) we can close the menu via the back button. On my GS4, I have confirmed that both of these are true. Sebastian also mentioned that the hardware menu button is completely ignored in the Toolbar API so if we ever switched to that, we'd be doing worse. Also, users probably don't expect the hardware menu button to always work. But we should remain open to feedback from hardware menu key users.
Comment 5•8 years ago
|
||
Thanks. I find "users probably don't expect the hardware menu button to always work", to be troubling, and agree we/moz should be alert to further user feedback. (I understand my phone is a little long in the tooth age-wise).
Reporter | ||
Comment 6•8 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #4) > Sebastian also mentioned that the > hardware menu button is completely ignored in the Toolbar API so if we ever > switched to that, we'd be doing worse. Also, users probably don't expect the > hardware menu button to always work. But it seems this is not always the case: 11:23 <@mfinkle> mcomella, sebastian: i have a samsung galaxy s 3, with hardware menu button 11:23 <@mfinkle> mcomella, sebastian: chrome supports both, menu button and 3-dot However, I don't think this changes the priority.
Comment 7•8 years ago
|
||
Or, for consistency, maybe we should just ignore the hardware menu button completely. This way, it's not unexpected if it doesn't close the menu, since it won't have opened it in the first place. Sebastian mentioned that Google apps are moving in this direction, so let's move with the platform leaders.
Comment 8•8 years ago
|
||
+1 for blocking entirely ... The annoying part is in breaking the flow /after/ opening the menu then getting briefly stranded mentally trying to close/undo the thing you just did. I'd personally rather the workflow block occurs at the point where I need to learn the new forward-path, not after I've gone down a rabbit hole :)
Comment 9•8 years ago
|
||
Thinking about this more... Given that other Google apps don't ignore the menu button, I'm starting to think that we should try to find a way to close the menu when the user presses the menu button. I still don't think this is high priority, but it's something we should try to fix at some point.
Reporter | ||
Comment 10•8 years ago
|
||
I'll look at this on the side (or after) my Telemetry work. If you want to work on it sooner, please take it. Note that the menu button changes landed in 45 so we should try to uplift if possible.
Assignee: nobody → michael.l.comella
Comment 11•8 years ago
|
||
I've just registered just to say you killed my UI experience with this menu button change. I have a SGS4 with HW buttons and I don't want to see the three dots button. I'm always tapping the menu instead of my current tabs I don't want to see that button taking up the limited space of the address bar! I don't like to see SS buttons, that's why I always buy phones with HW buttons! And Samsung Galaxy series are one of the most popular phones out there. I don't like that 45.0b change at all!
Comment 12•8 years ago
|
||
I don't like to see SW buttons* Also, I've using the beta for years and this was my first big disappointment with new "features".
Reporter | ||
Comment 13•8 years ago
|
||
comment 11 & comment 12 are more relevant to bug 1236156 – let's continue the discussion there.
Assignee | ||
Comment 14•8 years ago
|
||
I haven't investigated very closely why exactly the hardware menu button stopped closing the menu, but in any case after a bit of digging around, the solution seems relatively simple: Override OnKeyDown() in GeckoMenu and close the menu from there if the menu button was pressed.
Assignee: michael.l.comella → jh+bugzilla
Assignee | ||
Comment 15•8 years ago
|
||
This restores the functionality to close the menu by pressing the hardware menu key. Review commit: https://reviewboard.mozilla.org/r/45509/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/45509/
Attachment #8740025 -
Flags: review?(s.kaspari)
Comment 16•8 years ago
|
||
Comment on attachment 8740025 [details] MozReview Request: Bug 1229967 - Handle onKeyDown() for the menu key in GeckoMenu. r=sebastian The code looks good but I do not have an Android 4+ devices with menu key. @ahunt: Do you have such a device at the office and can verify that this is working?
Attachment #8740025 -
Flags: review?(s.kaspari) → review?(ahunt)
Assignee | ||
Comment 17•8 years ago
|
||
Just to note that the hardware menu key can now also close the menu on the tabs tray screen (New Private Tab/Close All Tabs), which apparently didn't work even before bug 1209967 landed (tested with FF44 release).
Comment 18•8 years ago
|
||
driveby: This works great on my GS3 hardware menu button for both main menu and tabs menu cases.
Comment 19•8 years ago
|
||
Comment on attachment 8740025 [details] MozReview Request: Bug 1229967 - Handle onKeyDown() for the menu key in GeckoMenu. r=sebastian https://reviewboard.mozilla.org/r/45509/#review42451 Looks good to me! Works on this Nexus S (?) with 4.1 too
Attachment #8740025 -
Flags: review?(ahunt) → review+
Assignee | ||
Comment 20•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3683acc1e9b6
Keywords: checkin-needed
Comment 21•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/d7ccace9b11c
Keywords: checkin-needed
Comment 22•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/d7ccace9b11c
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Reporter | ||
Comment 23•8 years ago
|
||
Thanks Jan! :)
Assignee | ||
Comment 24•8 years ago
|
||
Comment on attachment 8740025 [details] MozReview Request: Bug 1229967 - Handle onKeyDown() for the menu key in GeckoMenu. r=sebastian Approval Request Comment [Feature/regressing bug #]: bug 1209967 [User impact if declined]: The main menu can no longer be closed via the hardware menu key (where available). [Describe test coverage new/current, TreeHerder]: manual [Risks and why]: Low, we just listen for the menu key in GeckoMenu if the menu is visible and close it in response. String/UUID change made/needed]: none
Attachment #8740025 -
Flags: approval-mozilla-beta?
Attachment #8740025 -
Flags: approval-mozilla-aurora?
Assignee | ||
Updated•8 years ago
|
Comment 25•8 years ago
|
||
Comment on attachment 8740025 [details] MozReview Request: Bug 1229967 - Handle onKeyDown() for the menu key in GeckoMenu. r=sebastian 46 migrated from beta to release channel today so we should also aim this on m-r.
Attachment #8740025 -
Flags: approval-mozilla-release?
Comment 26•8 years ago
|
||
Comment on attachment 8740025 [details] MozReview Request: Bug 1229967 - Handle onKeyDown() for the menu key in GeckoMenu. r=sebastian Fix for menu key, please uplift to aurora, beta, and m-r. We will be doing the 46 RC build from m-r on Monday.
Attachment #8740025 -
Flags: approval-mozilla-release?
Attachment #8740025 -
Flags: approval-mozilla-release+
Attachment #8740025 -
Flags: approval-mozilla-beta?
Attachment #8740025 -
Flags: approval-mozilla-beta+
Attachment #8740025 -
Flags: approval-mozilla-aurora?
Attachment #8740025 -
Flags: approval-mozilla-aurora+
Comment 27•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/d6d1d04e3dbd
Comment 28•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/9a9fe42b023a
Comment 29•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-release/rev/f903b9c02a2f
Comment 30•8 years ago
|
||
Verified as fixed on Lenovo A536 (Android 4.4.2) on Firefox 46 Beta 12
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•