Pinch End cause Double tap event fired

RESOLVED FIXED in mozilla30

Status

()

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: romaxa, Assigned: drs)

Tracking

29 Branch
mozilla30
All
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
I've noticed that in latest trunk if sequence of events is:
1) Pinch started with 2 points
2) Pinch ended because one point was released
3) very quickly released second point right after first

Double Tap get's fired.

This happen because:
1) http://hg.mozilla.org/mozilla-central/annotate/e931763f41b5/gfx/layers/ipc/GestureEventListener.cpp#l275 - Generate extra Touch Start 
2) http://hg.mozilla.org/mozilla-central/annotate/e931763f41b5/gfx/layers/ipc/GestureEventListener.cpp#l81 - set state to GESTURE_WAITING_SINGLE_TAP with that
3) http://hg.mozilla.org/mozilla-central/annotate/e931763f41b5/gfx/layers/ipc/GestureEventListener.cpp#l172 - set state to GESTURE_WAITING_DOUBLE_TAP
4) http://hg.mozilla.org/mozilla-central/annotate/e931763f41b5/gfx/layers/ipc/GestureEventListener.cpp#l150 - finally handle Double Tap


This does not happen if no extra Touch Start fired or Second point released with delay > MAX_TAP_TIME
Doug, can you look into this?
Assignee: nobody → bugzilla+drs
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → All
Version: unspecified → 29 Branch
(Assignee)

Comment 2

5 years ago
I'm unable to repro this on a build from an hour ago. Are there any extra steps or anything that you may have missed?
Status: NEW → UNCONFIRMED
Ever confirmed: false
Flags: needinfo?(romaxa)
(Reporter)

Comment 3

5 years ago
This was reproduced on Qt embedding setup, which may have different timings on Pinch behavior.
Do you hit GestureEventListener.cpp#l275 at all?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(romaxa)
(Assignee)

Comment 4

5 years ago
(In reply to Oleg Romashin (:romaxa) from comment #3)
> This was reproduced on Qt embedding setup, which may have different timings
> on Pinch behavior.
> Do you hit GestureEventListener.cpp#l275 at all?

Yes. I only went as far as to check that, doing the STR, and checking whether or not I hit GestureEventListener.cpp#l150, which I didn't.
(Assignee)

Comment 5

5 years ago
Created attachment 8373452 [details] [diff] [review]
Set GEL's state back to GESTURE_NONE after spoofing a touch start on pinch end

After talking with :romaxa on IRC, I was still unable to repro the original issue. However, I was able to catch a single tap that was being fired at the end of pinches. This patch seems to fix this.

I believe the best way to fix this would be to have a flag on GEL to indicate whether or not it's spoofing events and it should ignore them until that flag is unset. But I don't think that much machinery is necessary for a single case like this. If we add any more spoofing, we might consider switching to that approach.
Attachment #8373452 - Flags: review?(bugmail.mozilla)
Comment on attachment 8373452 [details] [diff] [review]
Set GEL's state back to GESTURE_NONE after spoofing a touch start on pinch end

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

Romaxa, does this patch fix the issue you were seeing?
Attachment #8373452 - Flags: feedback?(romaxa)
(Reporter)

Comment 7

5 years ago
Comment on attachment 8373452 [details] [diff] [review]
Set GEL's state back to GESTURE_NONE after spoofing a touch start on pinch end

yep, this works fine
Attachment #8373452 - Flags: feedback?(romaxa) → feedback+
Comment on attachment 8373452 [details] [diff] [review]
Set GEL's state back to GESTURE_NONE after spoofing a touch start on pinch end

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

Eventually I'd like to change this interaction so that touches don't keep bouncing back and forth between APZC and GEL but for now this is fine.
Attachment #8373452 - Flags: review?(bugmail.mozilla) → review+

Comment 9

5 years ago
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #8)
> Eventually I'd like to change this interaction so that touches don't keep
> bouncing back and forth between APZC and GEL but for now this is fine.

That would be really nice. Mutually recursive functions with side effects is a receipt for good headache.
In addition to that I think GEL would be much easier to debug if it'd been refactored to a Mealy machine.
(Assignee)

Comment 11

5 years ago
(In reply to dmitry.rojkov from comment #9)
> That would be really nice. Mutually recursive functions with side effects is
> a receipt for good headache.
> In addition to that I think GEL would be much easier to debug if it'd been
> refactored to a Mealy machine.

Feel free to do that. We could really use more people working on refactors like this.
(Assignee)

Updated

5 years ago
Attachment #8373452 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/ec8e723e0991
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.