Closed
Bug 827208
Opened 11 years ago
Closed 11 years ago
Tabs tray button hit area is too wide
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox19 unaffected, firefox20+ verified, firefox21+ verified, fennec20+)
VERIFIED
FIXED
Firefox 22
People
(Reporter: arky, Assigned: lucasr)
References
Details
(Keywords: regression)
Attachments
(2 files)
4.57 KB,
patch
|
bnicholson
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
2.27 KB,
patch
|
bnicholson
:
review+
akeybl
:
approval-mozilla-aurora+
akeybl
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
User often press tabs icon when they reach for Reader mode icon. Capture the video of this behavior here: http://youtu.be/J1lUV2CY4J0
Comment 1•11 years ago
|
||
I wonder if this might be related to bug 817828. When bug 817828 happens on my phone, tapping the black area opens the tabs tray, so the same hit area may be broken here (even though the black invalidation bug isn't visible in the video). I'm able to reproduce this hit area bug on Nightly, but not Beta.
Comment 2•11 years ago
|
||
Or rather, this was likely caused by bug 709433, which is responsible for bug 817828.
Blocks: 709433
Keywords: regression
Updated•11 years ago
|
Summary: Reader Mode Icon is too close to tabs icon → Tabs tray button hit area is too wide
Comment 3•11 years ago
|
||
That button is *definitely* too wide. http://www.youtube.com/watch?v=_mkCIvuuTI0
Updated•11 years ago
|
tracking-fennec: --- → ?
Assignee | ||
Comment 4•11 years ago
|
||
This is a regression cause by the new view stacking order in the toolbar (implemented in bug 709433). Bug 817828 is unrelated though (it's likely caused by a bug the ShapedButton's drawing code). The toolbar animation relies on the new stacking order. Need to think of a way to reduce the hit area of the tabs tray button.
Updated•11 years ago
|
Component: Reader Mode → General
Updated•11 years ago
|
Assignee: nobody → lucasr.at.mozilla
Comment 5•11 years ago
|
||
Sriram - Did we remove the click delegation code that fixed this before? We might have.
tracking-fennec: ? → 20+
Flags: needinfo?(sriram)
Comment 6•11 years ago
|
||
This might be related to stacking order as Lucas mentioned in comment 4. May be we can delegate the left half of the tail of tabs button to go to the url-bar, which will give it to whatever is there.
Flags: needinfo?(sriram)
Comment 7•11 years ago
|
||
This makes it very hard to hit the reader mode button.
tracking-firefox20:
--- → ?
Comment 8•11 years ago
|
||
We are awesome! because we were the first to use TouchDelegate in Android and file an Android bug on it! :P We are more awesome! because we are using 2 touch delegates on the same view that Android doesn't support! :P :P Basically setTouchDelegate() can have only one touch delegate. So, we internally hold a list of TouchDelegates inside something that extends TouchDelegate! :P :P :P (I promise that I passed Object-Oriented Programming class :( )
Attachment #719205 -
Flags: review?(bnicholson)
Updated•11 years ago
|
Attachment #719205 -
Flags: review?(bnicholson) → review+
Comment 10•11 years ago
|
||
Only the delegate that consumed ACTION_DOWN should consume the subsequent events. Now that there might be 2 or more delegates, ACTION_UP might be consumed by someone causing a stale state to persist in the delegated view.
Attachment #719353 -
Flags: review?(bnicholson)
Updated•11 years ago
|
Attachment #719353 -
Flags: review?(bnicholson) → review+
Updated•11 years ago
|
Keywords: checkin-needed
Comment 11•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c0a0c8227fb2
Keywords: checkin-needed
Comment 12•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/e2b69993b3f4 https://hg.mozilla.org/mozilla-central/rev/c0a0c8227fb2
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 22
Assignee | ||
Comment 13•11 years ago
|
||
It's probably worth uplifting this one to aurora/beta btw.
Comment 14•11 years ago
|
||
Sounds like this has been around since we shipped Reader Mode, so no need to track but feel free to nominate the patch(es) for uplift if low risk enough.
Comment 15•11 years ago
|
||
(In reply to Lukas Blakk [:lsblakk] from comment #14) > Sounds like this has been around since we shipped Reader Mode, so no need to > track but feel free to nominate the patch(es) for uplift if low risk enough. This is a regression from bug 709433 (Fx20). Any chance this can still make beta?
Comment 16•11 years ago
|
||
Comment on attachment 719205 [details] [diff] [review] Patch [Approval Request Comment] Bug caused by (feature/regressing bug #): Animating browser-toolbar to awesombar. User impact if declined: Reader-mode button is hard to hit. Testing completed (on m-c, etc.): Landed in m-c on 02/28 Risk to taking this patch (and alternatives if risky): Very less. We've been using tail-touch-delegate to make sure Menu button gets a fair share. Menu button was on the right side. Now we are using the same logic on the left side. String or UUID changes made by this patch: None.
Attachment #719205 -
Flags: approval-mozilla-beta?
Attachment #719205 -
Flags: approval-mozilla-aurora?
Comment 17•11 years ago
|
||
Comment on attachment 719353 [details] [diff] [review] Patch 2: Fix [Approval Request Comment] Bug caused by (feature/regressing bug #): Animating browser-toolbar to awesombar. User impact if declined: Reader-mode button is hard to hit. Testing completed (on m-c, etc.): Landed in m-c on 02/28 Risk to taking this patch (and alternatives if risky): Very less. We've been using tail-touch-delegate to make sure Menu button gets a fair share. Menu button was on the right side. Now we are using the same logic on the left side. String or UUID changes made by this patch: None.
Attachment #719353 -
Flags: approval-mozilla-beta?
Attachment #719353 -
Flags: approval-mozilla-aurora?
Updated•11 years ago
|
status-firefox19:
--- → unaffected
status-firefox20:
--- → affected
status-firefox21:
--- → affected
tracking-firefox21:
--- → +
Updated•11 years ago
|
Attachment #719205 -
Flags: approval-mozilla-beta?
Attachment #719205 -
Flags: approval-mozilla-beta+
Attachment #719205 -
Flags: approval-mozilla-aurora?
Attachment #719205 -
Flags: approval-mozilla-aurora+
Updated•11 years ago
|
Attachment #719353 -
Flags: approval-mozilla-beta?
Attachment #719353 -
Flags: approval-mozilla-beta+
Attachment #719353 -
Flags: approval-mozilla-aurora?
Attachment #719353 -
Flags: approval-mozilla-aurora+
Comment 18•11 years ago
|
||
Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/14d0c6f36395 https://hg.mozilla.org/releases/mozilla-aurora/rev/baaf828aba27
Comment 19•11 years ago
|
||
Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/2937b7b3c26d https://hg.mozilla.org/releases/mozilla-beta/rev/78fe35adeb21
Updated•11 years ago
|
Updated•11 years ago
|
Status: RESOLVED → VERIFIED
Updated•11 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
•