827969 - gesture detector is reporting taps as double taps

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia, defect, P2)

Description

[David Flanagan [:djf]]

This is a regression from bug 823619.

Under the new event model, when the user taps, we get these events:

  touchstart touchend mousemove mousedown mouseup click

Apps that use gesture_detector.js (but do not use mouse_event_shim.js) will get these events from the gesture detector:

  tap tap dbltap

That is because the gesture detector generates a tap for the touchstart/touchend pair and the mousedown/mouseup pair.

This is a regression, and we should block on it.

Fortunately, I've got a fix in my patch for bug 827551. The fix is to modify gesture detector so that if we ever receive a touch event, we stop listening for mouse events.

I'll attach the patch soon.

Comment 1

[David Flanagan [:djf]]

Nominating blocking.

Cc'ing cjones since it is a regression from 823619. (Sorry I didn't catch this one, Chris!)

The only place I see a symptom of this bug is when you preview a photo in the camera app by tapping on the filmstrip.  tapping on the photo zooms in even though it is supposed to be a double tap to zoom.

Comment 2

[David Flanagan [:djf]]

Attachment: link to patch on github

The attached pull request fixes this bug and also 827551.  Looking for a reviewer...

Comment 3

[David Flanagan [:djf]]

Comment on attachment 699381 [details]
link to patch on github

Chris,

Is this something you can review?

Comment 4

[James Lal [:lightsofapollo]]

re 3: I did a quick review... One nit and one question. I don't understand 100% of GD but this makes sense to me.

r=lightsofapollo

Comment 5

[David Flanagan [:djf]]

Comment on attachment 699381 [details]
link to patch on github

Switching review to James, and marking his r+.

And requesting approval to land from Vivien, in case this doesn't get marked blocking.

NOTE: If blocking-basecamp+ is set, just land it for now.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 823619

User impact if declined: taps may be generate dbltap events in some apps.
Testing completed: yes, this patch fixes the photo viewer in the camera app and does not break other users of gesture detector like gallery and  browser

Risk to taking this patch (and alternatives if risky):

Comment 6

[David Flanagan [:djf]]

It looks like I reviewed the patch myself, but actually, I just set the flag after I got a r+ from lightsofapollo

Comment 7

[Chris Peterson [:cpeterson]]

LGTM. I added some comments on the GitHub PR, but nothing that should delaying this fix from landing.

Comment 8

[David Flanagan [:djf]]

I've updated the github PR to address nits from both Chris and James.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/28f2eb778f5d12e1d13fcd965160519d6593bf2e

Comment 10

[amiller]

Device: Unagi
Build: 20130114073222
Does not repro on this build and device.