Closed Bug 1030113 Opened 6 years ago Closed 6 years ago
Regression: Menu breaks on Firefox 29+ on tablets with hardware menu
61.81 KB, image/png
3.90 KB, patch
|Details | Diff | Splinter Review|
63.23 KB, image/png
83.76 KB, image/png
Currently, tablets with a hardware menu button can encounter a situation where the menu button does not open the in-browser menu anymore. This has been reported frequently on SUMO. Steps to Reproduce (that I can reproduce the issue on) * Enter settings via the menu and back out * Attempt to hit the menu button again Hitting the menu button afterwards does nothing. The only console output on button press: I/InputDispatcher( 2299): Delivering key to: action: 0x0 D/InputReader( 2299): Input event: value=0 when=1034627826000. Regression range incoming.
Shipped broken in Firefox 29 and 30. Last good revision: c9ea463d36c3 (2013-12-20) First bad revision: 7bc1fb6a21ae (2013-12-21) Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c9ea463d36c3&tochange=7bc1fb6a21ae No inbound-builds available.
The above was filed against a Samsung GALAXY Tab 3 10" Android 4.2
Bug 940997 seems rather likely. We have this device in MV and SF so any devs in either office should be able to fix this. Note that this is an x86 device.
Regression and seems to affect quite a few users.
Assignee: nobody → kbrosnan
tracking-fennec: ? → 33+
Kevin, any news on this subject? Thanks
This needs an assignee for 31; devices are in MV/SF.
Assignee: kbrosnan → nobody
QA Contact: kbrosnan
The first bad revision is: changeset: 161571:6d3504faf95d user: Chenxia Liu <firstname.lastname@example.org> date: Fri Dec 20 10:35:10 2013 -0800 summary: Bug 910189: Part 1 - Add "restore defaults" hooks into settings. r=margaret
Yikes, this looks like something is going wrong with the reflection hack. I'll take a look, and if all else fails, we can try faking a menu item button and maybe use our own GeckoMenu item or something.
Assignee: nobody → liuche
Ian, I removed this atrocious hack that we had that was causing users to not be able to access our menu on some devices with a hardware button. This turns the 3-dot icon into text though (which is consistent for other apps on these devices, using text instead of icons). Thoughts?
Lucas, I'm not sure if there is actually a way to force the 3-dot menu on some of these devices, other than that atrocious reflection hack - any ideas? I also tried nesting a menu inside an actionbar item, using our 3-dot menu drawable as the icon, but these devices seem to want to use the text instead of the icon, so that didn't work - there was just a clickable blank box where the item should be.
Attachment #8450693 - Flags: feedback?(lucasr.at.mozilla)
Comment on attachment 8450693 [details] [diff] [review] Patch: Remove reflection hack Review of attachment 8450693 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Chenxia Liu [:liuche] from comment #10) > Created attachment 8450693 [details] [diff] [review] > Patch: Remove reflection hack > > Lucas, I'm not sure if there is actually a way to force the 3-dot menu on > some of these devices, other than that atrocious reflection hack - any > ideas? Yeah, I realize the hw menu has all sorts of discoverability issues but bending the platform to force a different behaviour leads to issues like this. Let's not do this. > I also tried nesting a menu inside an actionbar item, using our 3-dot > menu drawable as the icon, but these devices seem to want to use the text > instead of the icon, so that didn't work - there was just a clickable blank > box where the item should be. Let's just let the platform do its thing.
Attachment #8450693 - Flags: feedback?(lucasr.at.mozilla) → feedback+
Chenxia, are you going to push that commit? 31 beta 8 will go to build today.
I'm not sure if we should uplift this immediately without very much baking, even though it's a very small change, and actually decreases the code complexity significantly. Screenshots to follow.
The first screenshot that was uploaded is for displaying the menu item on all devices (including those with hardware menu items), but this always displays as a text item, which may be more prominent than we want the "Restore defaults" item.
OK. Thanks for the feedback. FYI, beta 9 is going to build next Thursday. We probably won't accept a patch later than this.
Attachment #8451944 - Flags: review?(mark.finkle) → review+
STR: On previously affected devices, you should now be able to do the following: 1. Hit HW menu button to bring up Fennec menu. 2. Open Settings, and then hit back. 3. Tap on the hw menu button again to open the Fennec menu. Note: On devices with a HW menu key, this affects how Customize > Search > "Restore defaults" menu item is displayed - the HW menu button must be pressed to display "Restore defaults." On all other devices, the behavior should remain the same (with the 3-dot menu).
We don't build beta 9 for Firefox for Android.
Chenxia, can we have an uplift request for aurora (32)? Except if Kevin or Mark disagree, I won't take it in 31.
Comment on attachment 8451944 [details] [diff] [review] Patch: Remove reflection hack Approval Request Comment [Feature/regressing bug #]: Bug 910189 [User impact if declined]: Users of some devices (samsung it seems) with hardware buttons will not be able to access the Settings more than once without restart [Describe test coverage new/current, TBPL]: nightly [Risks and why]: low, actually removes riskier code. Almost just code removal [String/UUID change made/needed]: None
Attachment #8451944 - Flags: approval-mozilla-aurora?
Comment on attachment 8451944 [details] [diff] [review] Patch: Remove reflection hack Aurora+ Can you please verify the fix before 32 moves to Beta?
Attachment #8451944 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.