Closed Bug 969860 Opened 7 years ago Closed 7 years ago

Pinch End cause Double tap event fired

Categories

(Core :: Panning and Zooming, defect)

29 Branch
All
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: romaxa, Assigned: drs)

References

Details

Attachments

(1 file)

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
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)
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)
(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.
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)
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+
(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.
(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.
Attachment #8373452 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/ec8e723e0991
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.