Closed
Bug 777026
Opened 12 years ago
Closed 12 years ago
Software menu button (3-dots) are hard to hit
Categories
(Firefox for Android Graveyard :: Theme and Visual Design, defect)
Tracking
(firefox15 verified, firefox16+ verified, firefox17+ verified, fennec15+)
VERIFIED
FIXED
Firefox 17
People
(Reporter: vlad, Assigned: sriram)
Details
(Keywords: regression)
Attachments
(3 files, 4 obsolete files)
171.00 KB,
image/png
|
Details | |
50.58 KB,
image/png
|
Details | |
3.79 KB,
patch
|
mfinkle
:
review+
lsblakk
:
approval-mozilla-aurora+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
With the change in how we render the menus, the 3dots button has become very hard to hit. In the attached screenshot, the top is FF14 from the Market, and the bottom is m-c build from today. In the m-c build, the hit target for the 3dots button is only the box which starts at the rightmost bottom pixel of the S-curve from the tab switcher. Note that this extends beyond the 3 dots themselves -- hitting the dots will open up the tab switcher instead. In FF14, the split was right down the middle of the S curve, which is what I'd expect. To get the 3dots to highlight, I had to press basically at the edge of my screen.
Reporter | ||
Updated•12 years ago
|
tracking-fennec: --- → ?
Updated•12 years ago
|
Keywords: regression
Comment 1•12 years ago
|
||
It's even harder in landscape (Galaxy Nexus).
status-firefox15:
--- → affected
status-firefox16:
--- → affected
status-firefox17:
--- → affected
Product: Fennec → Firefox for Android
Updated•12 years ago
|
tracking-firefox17:
--- → ?
Comment 2•12 years ago
|
||
I haven't been seeing this at all on my phone, but it is pretty brutal on the Nexus 7 -- I was experiencing basically what Vlad is describing.
Comment 3•12 years ago
|
||
Store complaint https://play.google.com/store/apps/details?id=org.mozilla.firefox_beta&reviewId=03176906256282472373 Sriram/Ian, we should get this fixed for Beta
Component: General → Theme and Visual Design
Summary: 3dots button very hard to hit on m-c trunk → Software menu button (3-dots) are hard to hit
Assignee | ||
Comment 4•12 years ago
|
||
But like how? There are no squishy buttons in android. Everything is rectangular. And we are placing a rectangle over another with almost 50% overlap. If we move menu's z-order to be above tabs button's, then menu will be hit most of the times. I've been thinking of this problem and just came up with one crazy idea. 1. Basically we need to monitor the tap on the button - overriding onTouch -- as onClick doesnt tell us where the click was. (cost: okayish) 2. Now convert the button to a bitmap (cost: high to pretty high) 3. See if the click was was on a pixel that wasn't transparent (cost: okayish) 4. Take care of the scenario where the orange touch state is preserved (cost: high) 5. If the click was on a transparent pixel - declare it was not for tabs button and pass it to its parent. The parent should ideally pass it to the menu button. But the process of converting the button to a bitmap --- mmmm... I'm not sure if we want to take this crazy approach to solve this. It's almost like developing a multi touch game -- but for a button. :) (P.S.: In the history of UI development, never ever someone had come up with a "squishy button" to have the touch fall exactly within its boundaries :P There are rounded cornered buttons. But they still accept touch areas outside them :D)
Comment 5•12 years ago
|
||
Can we do something like comment 4 but simpler: instead of using the actual transparent pixels to define a curved boundary for the button, just define a rectangular one? For example, if the tap is in the left 60% of the tab button, then handle it; if it is in the right 40% then pass it to the menu button instead. If we can just get back to approximately the same rectangular touch areas that were used in Fx14, that would be great.
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Matt Brubeck (:mbrubeck) from comment #5) > Can we do something like comment 4 but simpler: instead of using the actual > transparent pixels to define a curved boundary for the button, just define a > rectangular one? For example, if the tap is in the left 60% of the tab > button, then handle it; if it is in the right 40% then pass it to the menu > button instead. > > If we can just get back to approximately the same rectangular touch areas > that were used in Fx14, that would be great. How would that translate to a case where we don't have menu button? Ideally "org.mozilla.gecko.TabsButton" (when we implement it), wouldn't really know about the presence of Menu button. Hence we cannot do it for "tabs button without tail" scenario.
Comment 7•12 years ago
|
||
(In reply to Aaron Train [:aaronmt] from comment #3) > Store complaint > > https://play.google.com/store/apps/details?id=org.mozilla. > firefox_beta&reviewId=03176906256282472373 > > Sriram/Ian, we should get this fixed for Beta The comment in the store appears to be different from the issue Vlad described. The store comment seems to be referring to the tab menu where we show the "+" next to a disabled button, which has been causing some confusion. I filed a bug 779148 against that. We should definitely fix it, I agree. Just want to make sure we are all talking about the same thing.
Comment 8•12 years ago
|
||
Vlad, Aaaron, can you confirm that you are finding the menu button *next to the url bar* hard to hit, and it isn't the menu button you see when the tab menu is open? Just want to make that super clear. If it is indeed the menu button next to the URL bar, another solution would be to elongate the dark area in which the menu button sits. But that also means I will need to re-cut about 30+ images, so I want to make absolutely sure we're all talking about the same thing before we change anything.
Comment 9•12 years ago
|
||
(In reply to Ian Barlow (:ibarlow) from comment #8) > Vlad, Aaaron, can you confirm that you are finding the menu button *next to > the url bar* hard to hit In landscape? Very much yes. In portrait? No. I agree with the store comment for landscape. I'm using the same old Galaxy Nexus
Reporter | ||
Comment 10•12 years ago
|
||
We're talking about the menu button next to the URL bar, as seen in the original screenshot. Here's a better image showing the problem. I see people describing a 50%/50% split -- but that's *not* what's happening. The only area that registers as a hit on the 3dots button is all the way to the right of the swoosh (in the pink) -- notice that the dots themselves aren't even fully included! Everything in the light blue registers as a press on the tab button. The green outline is I think what -should- register as a menu press, and I think what people are describing as what should be happening. But it's not.
Reporter | ||
Comment 11•12 years ago
|
||
Er, the green outline isn't in the right spot there, my image editor didn't save. It should be further over to the left, in the middle of the swoop, as here.
Attachment #649715 -
Attachment is obsolete: true
Comment 12•12 years ago
|
||
(In reply to Vladimir Vukicevic [:vlad] [:vladv] from comment #11) > Created attachment 649719 [details] > better image showing the problem > > Er, the green outline isn't in the right spot there, my image editor didn't > save. It should be further over to the left, in the middle of the swoop, as > here. +1: The green outline is more or less what we want. So clearly the menu selection area has moved over on us somehow. Oddly, I haven't seen this on my Galaxy Nexus, but I *have* seen it on my Nexus 7 recently.
Updated•12 years ago
|
Assignee: nobody → sriram
Assignee | ||
Comment 13•12 years ago
|
||
This patch adds an invisible layer over the tabs button (over the overlapping portion), which helps us pass the touches without it propagating to the tabs button.
Attachment #649867 -
Flags: review?(mark.finkle)
Comment 14•12 years ago
|
||
Tracking for 17 and 16 as well since this is a good candidate for uplift to improve usability, depending on the risk assessment we'd consider this for Beta too.
tracking-firefox16:
--- → +
Updated•12 years ago
|
tracking-fennec: ? → 15+
Comment 15•12 years ago
|
||
Comment on attachment 649867 [details] [diff] [review] Patch I am worried about the affect of this patch on accessibility. Adding the extra, transparent button will likely confuse users who depend on focus order. Using the same contentDescription for the hidden menu button and the visible menu button will also cause some confusion. Also, why didn't you add this to layout-large-v11/browser_toolbar_menu.xml ?
Comment 16•12 years ago
|
||
Comment on attachment 649867 [details] [diff] [review] Patch What's the plan here for a11y? Will we just try to hide the transparent button from the focus order? I'd rather have no contentDescription for it than have two elements with "menu"? Also, why didn't you add this to layout-large-v11/browser_toolbar_menu.xml ?
Attachment #649867 -
Flags: review?(mark.finkle) → review-
Assignee | ||
Comment 17•12 years ago
|
||
I have missed large*. I'll add it. But I'm not sure about a11y here.
Comment 18•12 years ago
|
||
Comment on attachment 649867 [details] [diff] [review] Patch In order to answer this in a qualified manner, I need to know a) why this extra button is necessary in the first place and b) why it doesn't replace the other one that was there previously, but gets added instead? Will the button that was there previously still accept touch events/click events simulated by TalkBack, or will it become inert? If the latter, then that button should no longer be accessible. I agree with Mark that having two accessible menu buttons is not a good idea. :)
Comment 19•12 years ago
|
||
In addition, with the swipe gesture in jelly Bean that allows to go to all accessible items, just removing the contentDescription is not sufficient. The one button that the user is no longer supposed to hit should be made inaccessible, then.
Assignee | ||
Comment 20•12 years ago
|
||
http://developer.android.com/reference/android/view/TouchDelegate.html <-- I found this -- just the one we want -- probably. I'll try using this with our tabs button to restrict it's "touch area" to a normal square.
Assignee | ||
Comment 21•12 years ago
|
||
Unfortunately, TouchDelegate isn't working as expected. The parent dispatches to any target that might receive first. If none can receive the event, and if the event has to be handled by the parent, and if there is a touch delegate, then it is passed to the delegated view. In our case, there is a target -- tabs button -- ready to receive the event. Hence we won't hit the touch delegate. The other option was to spoof Tabs button to have a smaller hitTarget. While touch delegate might not work here, I thought of overriding getHitRect(), to spoof the bounds of tabs button. Unfortunately, again, this method is not used by the View, while figuring out if the event's coordinates are in the view. It uses raw mLeft, mRight, etc, instead of something returned by getHitRect(). So, that approach doesn't work too.
Assignee | ||
Comment 22•12 years ago
|
||
This patch has been updated with the change for layout-large-v11 too.
Attachment #649867 -
Attachment is obsolete: true
Attachment #652806 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 23•12 years ago
|
||
This patch uses a TouchDelegate on the Tabs button. Couple of things: 1. mTabs.getWidth() would return 0, if the view's layout isn't done yet. As per SO suggestions, its better to post the touchdelegate on the parent. 2. There is an android bug, where mDelegateTargeted is not set to false, when the event is not given to target on ACTION_DOWN. Hence the class is copied and fixed locally.
Attachment #652806 -
Attachment is obsolete: true
Attachment #652806 -
Flags: review?(mark.finkle)
Attachment #652837 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 24•12 years ago
|
||
TailTouchDelegate extends TouchDelegate, as I didn't want to extend Tabs button to add a new method, "setTailDelegate()" (as setTouchDelegate() expects TouchDelegate as the argument).
Assignee | ||
Comment 25•12 years ago
|
||
(late realization and reaction). This leaves the Android implementation as-is. Instead sends a ACTION_CANCEL just in case, the delegated view didn't handle ACTION_DOWN. The android implementation resets mDelegateTargeted on ACTION_CANCEL -- and hence works. ACTION_CANCEL woudn't cause problems for us though.
Attachment #652885 -
Flags: review?(mark.finkle)
Comment 26•12 years ago
|
||
Comment on attachment 652885 [details] [diff] [review] Patch: Better Nice. I assume we no longer need the other patch. If not, obsolete it please.
Attachment #652885 -
Flags: review?(mark.finkle) → review+
Comment 27•12 years ago
|
||
Just a note for a11y: Sriram's better patch fixes things in a way that does not require any changes for a11y. \o/
Assignee | ||
Comment 28•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce8976e74c03
Assignee | ||
Updated•12 years ago
|
Attachment #652837 -
Attachment is obsolete: true
Attachment #652837 -
Flags: review?(mark.finkle)
Comment 29•12 years ago
|
||
Lovely! Tested on mozilla-inbound (08/20)
Comment 30•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ce8976e74c03
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Assignee | ||
Comment 31•12 years ago
|
||
Comment on attachment 652885 [details] [diff] [review] Patch: Better [Approval Request Comment] Bug caused by (feature/regressing bug #): UI design User impact if declined: Frustration of users while pressing menu button. Testing completed (on m-c, etc.): Landed on m-c yesterday. Risk to taking this patch (and alternatives if risky): None. String or UUID changes made by this patch: None.
Attachment #652885 -
Flags: approval-mozilla-beta?
Attachment #652885 -
Flags: approval-mozilla-aurora?
Comment 32•12 years ago
|
||
Comment on attachment 652885 [details] [diff] [review] Patch: Better Since this is mobile-only and seems low risk in that it just adds responsiveness for the user we'll take this on branches. Please land before tomorrow for our final Beta go to build.
Attachment #652885 -
Flags: approval-mozilla-beta?
Attachment #652885 -
Flags: approval-mozilla-beta+
Attachment #652885 -
Flags: approval-mozilla-aurora?
Attachment #652885 -
Flags: approval-mozilla-aurora+
Assignee | ||
Comment 33•12 years ago
|
||
pushed to aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/5d4a947e8fdb
Assignee | ||
Comment 34•12 years ago
|
||
and to beta: https://hg.mozilla.org/releases/mozilla-beta/rev/6fc9e89951a9
Updated•12 years ago
|
Updated•12 years ago
|
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
•