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)

ARM
Android
defect
Not set
normal

Tracking

(firefox15 verified, firefox16+ verified, firefox17+ verified, fennec15+)

VERIFIED FIXED
Firefox 17
Tracking Status
firefox15 --- verified
firefox16 + verified
firefox17 + verified
fennec 15+ ---

People

(Reporter: vlad, Assigned: sriram)

Details

(Keywords: regression)

Attachments

(3 files, 4 obsolete files)

Attached image screenshot
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.
tracking-fennec: --- → ?
Keywords: regression
It's even harder in landscape (Galaxy Nexus).
Product: Fennec → Firefox for Android
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.
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
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)
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.
(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.
(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.
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.
(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
Attached image better image showing the problem (obsolete) —
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.
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
(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.
Assignee: nobody → sriram
Attached patch Patch (obsolete) — Splinter Review
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)
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-fennec: ? → 15+
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 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-
I have missed large*. I'll add it. But I'm not sure about a11y here.
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. :)
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.
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.
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.
Attached patch Patch (obsolete) — Splinter Review
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)
Attached patch Patch (obsolete) — Splinter Review
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)
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).
Attached patch Patch: BetterSplinter Review
(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 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+
Just a note for a11y: Sriram's better patch fixes things in a way that does not require any changes for a11y. \o/
Attachment #652837 - Attachment is obsolete: true
Attachment #652837 - Flags: review?(mark.finkle)
Lovely!

Tested on mozilla-inbound (08/20)
https://hg.mozilla.org/mozilla-central/rev/ce8976e74c03
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
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 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+
Status: RESOLVED → VERIFIED
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: