766635 - New Tabs UI closes tab instead of scrolling

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Alex Keybl [:akeybl]]

STR:
1. open up a few tabs (5 or so, but occurs with less)
2. open up the tabs UI
4. put your finger down on a tab, and quickly swipe up and a bit to the right

Expected:
Scroll down to the bottom of the list quickly.

What actually happened:
The tab underneath your finger at the beginning of the swipe upwards is swiped off to the right, closing the tab

Comment 2

[Wesley Johnston (:wesj)]

I wouldn't call this a dup. Your complaint in bug 764812 is that you only dragged the tab a little to the side and it closed. The complaint here is that he's scrolling vertically, which should disable closing the tab, but it isn't.

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch

Most of this is caused by our fling detector. It wasn't paying attention to the fact that flings can be diagonal. I added a few little tweaks as well though to make sure

1.) We don't scroll the list items and the list at the same time and
2.) To give a little more weight to vertical scrolls over horizontal ones.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

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


>-                if (Math.abs(distanceX) > Math.abs(distanceY)) {
>+                // check if this scroll is more horizontal than vertical. Weight vertical drags a little higher
>+                // by using a multiplier
>+                if (Math.abs(distanceX) > Math.abs(distanceY)*1.5) {

Add spaces: Math.abs(distanceY) * 1.5
Use a named const for the 1.5: SWIPE_VERTICAL_WEIGHT

>+            // also make sure that the swipe is in a mostly horizontal direction
>+            if (Math.abs(velocityX) > Math.abs(velocityY*1.5) &&

Same

Comment 5

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/859048b45a64

Comment 6

[Daniel Holbert [:dholbert]]

Looks like TabsTray.java gets build with -Werror, which means that the warning this cset introduced was treated as a build error and caused Android opt redness.

The warning/error is:
 https://tbpl.mozilla.org/php/getParsedLog.php?id=12869250&tree=Mozilla-Inbound
{
/builds/slave/m-in-andrd/build/mobile/android/base/TabsTray.java:51: possible loss of precision
found   : double
required: float
    private static final float SWIPE_VERTICAL_WEIGHT = 1.5;
}

Presumably you either need to either declare that as a double, or add an "f" suffix to "1.5f" (if that works in Java like it does in C++)

Comment 7

[Wesley Johnston (:wesj)]

Added an f

http://hg.mozilla.org/integration/mozilla-inbound/rev/4913b412caaa

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/859048b45a64
https://hg.mozilla.org/mozilla-central/rev/4913b412caaa

Comment 9

[Wesley Johnston (:wesj)]

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

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 713450
User impact if declined: accidental tab closing
Testing completed (on m-c, etc.): landed today 6-22
Risk to taking this patch (and alternatives if risky): pretty low risk. mobile only. fixes a bug in a new feature 
String or UUID changes made by this patch: now.

Comment 10

[Alex Keybl [:akeybl]]

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

[Triage Comment]
I expect our testing to find any regressions here. Approved for Aurora 15.

Comment 11

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/424d62d5fdf7
https://hg.mozilla.org/releases/mozilla-aurora/rev/aba0a9914379

Comment 12

[Paul Feher]

Verified fixed on:
Htc Desire Z (2.3.3)
Using:
Nightly Fennec 16.0a1 (2012-07-10)
Aurora Fennec 15.0a2 (2012-07-10)

The is fixed, the user can scroll the tab-list vertically without closing tabs by mistake.