455361 - too easy to click on link when kinetic scrolling

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

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

when you click to stop a kinetic scroll, too often you end up clicking on something you didn't mean to

Comment 1

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

Attachment: don't start timer if ending kinetic

this patch applies over the patch attached to bg 455352

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 338691 [details] [diff] [review]
don't start timer if ending kinetic

>diff --git a/chrome/content/deckbrowser.xml b/chrome/content/deckbrowser.xml

>+            if (self.dragData.kineticId) {
>                self._endKinetic(); 
>+            } else {
>+              this._lastMouseDown = aEvent;
>+              this.deckbrowser._dragStartTimeout = setTimeout(function () {
>+                                                     self._dragStartTimer();
>+                                                     }, 200);
>+            }

As it is you might as well return early here after calling _endKinetic(). If you want to be able to drag still you'll need to adjust the check in mousemove, otherwise there'll be no way to trigger a pan without another mouseup.

>+            
>             var zoomLevel = this.deckbrowser._zoomLevel;

Can you get rid of this while you're touching this? It's unused now.

Comment 3

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

Attachment: sets the last mouse down event to null so we don't act on it

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Attachment: alternative patch

basically the same patch, but with a few minor changes:
- fix right mouse button handling in mouseup (return early if button!=0)
- keep "dragData.dragging" true while kinetic scrolling is happening
- move the clearing of _panEventTracker to startKinetic, since we don't use them after that
- moves the clearTimeout in mouseup outside of the target check, fixing a bug where we kept panning if you mouseup outside of the canvas
- otherwise refactor mouseup logic so that it's clearer what is going on

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 338915 [details] [diff] [review]
alternative patch

Oops, typo in this patch.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Fixes typo in the previous patch (this.deckbrowser -> this now that it's moved out of the event handler), and makes sure to always clear the drag timer in mouseup.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

http://hg.mozilla.org/mobile-browser/rev/175d81156b3b

Comment 8

[Aakash Desai [:aakashd]]

I tried two methods:

1. panning down with the starting touch point being a link/button
2. panning down with the ending touch being a link/button

both cases did not result in a "click", so verified FIXED:

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20090820
Fennec/1.0b3pre

and

Mozilla/5.0 (Macintosh; U; Intel Mac OSX 10.5; en-US; rv:1.9.2a2pre)
Gecko/20090808 Fennec/1.0b3pre