818494 - Swipe to close tab action is sensitive

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Paul Feher]

Nightly 20.0a1 (2012-12-05)
Device: Samsung Galaxy SII
OS: Android 4.0.1

Steps to reproduce:
1. Open multiple tabs.
2. Swipe to close a few tabs. 

Expected:
The tab thumbnail should be closed after it covers a reasonable distance to avoid accidentally closing the tab.

Actual
The tab is closed almost instantly after the swipe action begins.

Comment 1

[Lucas Rocha (:lucasr)]

Ian, what do you think?

Comment 2

[Adrian Tamas (:AdrianT)]

This is an issue also on Firefox Mobile 18 beta 4. Here is a video from the Samsung Galaxy Tab 2 7.0 running Android 4.0.4: http://youtu.be/8d6hVhM3TTI

Comment 3

[Ian Barlow (:ibarlow)]

That video definitely looks a lot more sensitive than we want, I agree.

If we looked at the Android app switcher as a comparison, the threshold to close a tab seems to be roughly 25-30% the width of the screen. I feel like we did that originally for our tab tray too, did that get changed recently?

Comment 4

[Adrian Tamas (:AdrianT)]

I also believe this to be a regression. Bug 764812 fixed this when the issue was first seen.

Comment 5

[Aaron Train [:aaronmt]]

Wes, wasn't this fixed in 17?

Comment 6

[Lucas Rocha (:lucasr)]

(In reply to Aaron Train [:aaronmt] from comment #5)
> Wes, wasn't this fixed in 17?

No, this has been fixed in 19 (see bug 772940). This bug is a dup of bug 787335.

Comment 7

[Adrian Tamas (:AdrianT)]

https://bugzilla.mozilla.org/show_bug.cgi?id=787335 has been verified fix. This issue is still reproducible on Nightly 20.0a1 2012-12-14.

Comment 8

[Wesley Johnston (:wesj)]

Attachment: Patch

getScaledMinimumFlingVelocity is actually a very very small number (62px/sec on my Nexus 7). The stock browser uses a larger number to determine this (1500/2). This feels a bit high to me tbh. 500 felt way too low. I decided to leave it here because I'd rather err on the hard side. Build at:

http://dl.dropbox.com/u/72157/fennec-slideClose.apk

Comment 9

[Lucas Rocha (:lucasr)]

Comment on attachment 693101 [details] [diff] [review]
Patch

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

Nice.

::: mobile/android/base/TabsTray.java
@@ +343,5 @@
>          private int mSwipeThreshold;
>          private int mMinFlingVelocity;
> +        // same value the stock browser uses for after drag animation velocity in pixels/sec
> +        // http://androidxref.com/4.0.4/xref/packages/apps/Browser/src/com/android/browser/NavTabScroller.java#61
> +        private static final float MIN_VELOCITY = 750;

nit: move this to the top of the class.

Comment 10

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a319fd7b88df

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

nominate for tracking since this is an issue with a feature that was introduced in 18

Comment 12

[Wesley Johnston (:wesj)]

Comment on attachment 693101 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Swipe to close rewrite (bug 772940)
User impact if declined: Swipe to close is very very easy to trigger
Testing completed (on m-c, etc.): landed on mc yesterday. i'd like to at least make sure UX has had a chance to test this, but it matches the stock browser behavior
Risk to taking this patch (and alternatives if risky): Very low risk. Changing a pref number
String or UUID changes made by this patch: None.

Comment 13

[Wesley Johnston (:wesj)]

Comment on attachment 693101 [details] [diff] [review]
Patch

I don't think that bug 772940 is actually in 18. Remove beta request.

Comment 14

[Ed Morley [:emorley]]

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

Comment 15

[Wesley Johnston (:wesj)]

Just wanted to make sure UX had played with this in nightlies and was ok with pushing this to Aurora.

Comment 16

[Wesley Johnston (:wesj)]

UX says its good on irc!

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f8b55879b2de

Comment 18

[Paul Feher]

Verified fixed on:
Nightly 20.0a1 (2013-01-02)
Aurora 19.0a2 (2013-01-02)