599145 - Fix double tap radius calculation

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

This patch fixes the following bugs related to TapDouble events:

* The double-click radius depends on the browser scale, while it should be constant in client coordinates.

* clientX1/Y1 and clientX2/Y2 are always the same, so the distance between the two taps is actually ignored.  Regression from http://hg.mozilla.org/mobile-browser/rev/0da42190d7b6

* (After the above bug is fixed) if you tap twice in within 400ms but outside of the 100px radius, both taps are swallowed silently.

This patch fixes the above bugs, and moves the relevant logic from browser.js to input.js.

Comment 1

[Benjamin Stover (:stechz)]

Comment on attachment 478087 [details] [diff] [review]
patch

This patch is awesome! Only one concern here.

>   /** Endpoint of _commitAnotherClick().  Finalize a double tap.  */
>   _doDoubleClick: function _doDoubleClick() {
>     let mouseUp1 = this._downUpEvents[1];
>     // sometimes the second press event is not dispatched at all
>     let mouseUp2 = this._downUpEvents[Math.min(3, this._downUpEvents.length - 1)];
>     this._cleanClickBuffer();
> 
>+    let dx = mouseUp1.clientX - mouseUp2.clientX;
>+    let dy = mouseUp1.clientY - mouseUp2.clientY;
>+
>+    if (dx*dx + dy*dy < kDoubleClickRadius*kDoubleClickRadius) {
>+      this._dispatchTap("TapDouble", mouseUp1);
>+    } else {
>+      this._dispatchTap("TapSingle", mouseUp1);
>+      this._dispatchTap("TapSingle", mouseUp2);
>+    }
>+  },

Sending two single taps is a change in functionality. Although this makes perfect sense, I worry about what this may mean practically. If two taps come in very fast succession, I fear it is either A) a botched double tap or B) some sort of touch glitch. Dispatching two single taps could end up activating links, which probably isn't what the user intended. Is there any particular reason to add this?

Comment 2

[Matt Brubeck (:mbrubeck)]

After trying this out on a device for a while, I didn't see any problems with accidental taps or link activations.

Comment 3

[Benjamin Stover (:stechz)]

Comment on attachment 478087 [details] [diff] [review]
patch

Go go go!

Comment 4

[Matt Brubeck (:mbrubeck)]

http://hg.mozilla.org/mobile-browser/rev/d389eb72c4a0

Comment 5

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Verified on nightly :
Mozilla/5.0 (Android; Linux armv71; rv2.0b7pre) Gecko/20101007 Firefox/4.0b7pre Fennec/4.0b2pre

Verified on Beta 1:
Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b7pre) Gecko/201001006 Firefox/4.0b7pre Fennec/4.0b1
Mozille/5.0 (Android; Linux armv71; rv:2.0b7pre) Gecko20101006 Firefox/4.0b7pre Fennec/4.0b1