910191 - Swiping away tabs sometimes doesn't work properly

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

On a Galaxy Q running 2.3.6 with latest nightly:

1. Open a bunch of tabs
2. Hold the device in portrait mode (this doesn't repro in landscape as far as i can tell)
3. Open the tab switcher
4. Swipe away a tab, but instead of swiping it completely off the switcher, drag it to about 3/4 of the way off and then stop and lift your finger.

Expected: the tab either completes swiping away and closes or comes back to the normal position.

Actual: the tab stays at the 3/4 position where it was left (and is still slightly transparent). The tab does not close. Touching the tab again seems to trigger the tab close, but the animation doesn't look right.

I can reliably reproduce this, but there's a bit of magic to it. I find that if I drag the tab very slowly then this doesn't happen. It works best if I accelerate my finger to a normal fling speed but then bring it to a stop at around the 3/4 position and then lift my finger. I run into this frequently while actually trying to close tabs.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

I'm also seeing similar issues in landscape when I swipe away a tab - there is a 2-3 second pause between swiping away the tab and the next tab getting selected. It just feels really slow. And then to hide the tab tray, I should be able to just tap the thumbnail once, but I have to do it twice, which is also weird. I think in general somebody needs to look at the tab-closing behaviour on a slow Gingerbread device and tune it a little.

Comment 2

[Lucas Rocha (:lucasr)]

Attachment: Don't use OnPreDrawObserver on pre-HC devices

Oh well, OnPreDrawObserver seems busted on pre-HC Android.

Comment 3

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 805507 [details] [diff] [review]
Don't use OnPreDrawObserver on pre-HC devices

Review of attachment 805507 [details] [diff] [review]:
-----------------------------------------------------------------

I faced a similar problem while working on another bug. I guess the isAlive() should be in the listener too. May be a followup patch?

Comment 4

[Lucas Rocha (:lucasr)]

(In reply to Sriram Ramasubramanian [:sriram] from comment #3)
> Comment on attachment 805507 [details] [diff] [review]
> Don't use OnPreDrawObserver on pre-HC devices
> 
> Review of attachment 805507 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I faced a similar problem while working on another bug. I guess the
> isAlive() should be in the listener too. May be a followup patch?

I wondered about the same thing. I'll add the check just in case.

Comment 5

[Lucas Rocha (:lucasr)]

Pushed: https://hg.mozilla.org/integration/fx-team/rev/a0cad0d1d7fc

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a0cad0d1d7fc

Comment 7

[Lucas Rocha (:lucasr)]

Comment on attachment 805507 [details] [diff] [review]
Don't use OnPreDrawObserver on pre-HC devices

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 871651
User impact if declined: Swiping tabs will consistently fail on pre-Honeycomb devices.
Testing completed (on m-c, etc.): Local tests on multiple devices. Landed on m-c. 
Risk to taking this patch (and alternatives if risky): Low risk, affected devices will simply avoid the unreliable code path.  
String or IDL/UUID changes made by this patch: n/a

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/51a7fba55698

Not sure if you want to request beta approval for this or not given bug 913402.