Closed
Bug 1391770
Opened 7 years ago
Closed 7 years ago
Doing a one-touch-pinch gesture with second tap far away from the first works when it shouldn't
Categories
(Core :: Panning and Zooming, defect, P2)
Tracking
()
VERIFIED
FIXED
mozilla57
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
botond
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
Do a one-touch-pinch gesture (touchdown, touchup, touchdown, touchmove) but make the second touchdown far away from the first. It's easier if you use two fingers far away on the screen. This shouldn't work, but it does. What's worse is that if you do a double-tap gesture in this fashion (one tap far away from the second) we actually quickly enter one-touch-pinch, change the zoom level, and then exit one-touch-pinch, making it look like the zoom level just "snapped" to some arbitrary value. (The value actually depends on the distance between your two fingers on the y-axis).
Assignee | ||
Comment 1•7 years ago
|
||
Note that prior to bug 1111333 landing, this scenario of doing a double-tap with two fingers far away would go through [1] and the MoveDistanceIsLarge() check would cause the gesture to abort. However, since bug 1111333 landed, the double-tap goes through [2] which enters the one-touch pinch state. This is important because in correcting this, we need to make sure that we don't accidentally end up detecting double-tap gestures. [1] http://searchfox.org/mozilla-central/rev/123ab8fe26a3ef2438b8f918dfe548187edb885b/gfx/layers/apz/src/GestureEventListener.cpp#258 [2] http://searchfox.org/mozilla-central/rev/e8c36327cd8c9432c69e5e1383156a74330f11f2/gfx/layers/apz/src/GestureEventListener.cpp#283
Assignee | ||
Comment 2•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=14384a6c69ca45f42c10ce9f4b79e2e82cfbdeb8
Assignee | ||
Updated•7 years ago
|
status-firefox55:
--- → disabled
status-firefox56:
--- → affected
status-firefox57:
--- → affected
Priority: -- → P2
Comment hidden (mozreview-request) |
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8898970 [details] Bug 1391770 - Don't allow a faraway second tap to start a one-touch-pinch gesture. https://reviewboard.mozilla.org/r/170316/#review175568 ::: gfx/layers/apz/src/GestureEventListener.h:150 (Diff revision 1) > > void TriggerSingleTapConfirmedEvent(); > > + bool MoveDistanceExceeds(ScreenCoord aThreshold); > bool MoveDistanceIsLarge(); > + bool SecondTapIsFar(); These methods can be |const|. ::: gfx/layers/apz/src/GestureEventListener.cpp:189 (Diff revision 1) > + CancelLongTapTimeoutTask(); > + CancelMaxTapTimeoutTask(); > + mSingleTapSent = Nothing(); > + SetState(GESTURE_NONE); > + } else { > + // Otherwise, reset the touch start position so that the one-touch-pinch At this point, we could still get either a double-tap or a one-touch-pinch, right? If so, could we clarify the comment, along the lines of, "... so that, if this turns into a one-touch-pinch gesture, it uses the ...".
Attachment #8898970 -
Flags: review?(botond) → review+
Assignee | ||
Comment 5•7 years ago
|
||
Thanks, updated patch with both changes.
Comment hidden (mozreview-request) |
Pushed by kgupta@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b920455971b0 Don't allow a faraway second tap to start a one-touch-pinch gesture. r=botond
Comment 8•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b920455971b0
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8898970 [details] Bug 1391770 - Don't allow a faraway second tap to start a one-touch-pinch gesture. Approval Request Comment [Feature/Bug causing the regression]: bug 1111333 [User impact if declined]: performing a "double-tap" type of gesture with the two taps far apart can result in a weird zoom [Is this code covered by automated tests?]: partially [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: not very [Why is the change risky/not risky?]: fairly small code change, code is well-understood [String changes made/needed]: none
Attachment #8898970 -
Flags: approval-mozilla-beta?
Comment 10•7 years ago
|
||
Hi Ioana, could you help find someone to verify if this issue was fixed as expected on the latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(ioana.chiorean)
Comment 11•7 years ago
|
||
Comment on attachment 8898970 [details] Bug 1391770 - Don't allow a faraway second tap to start a one-touch-pinch gesture. Looks ok to uplift to beta 7, low risk change to pinch/zoom. We can verify this in beta once it lands.
Attachment #8898970 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 12•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/fa9cdc61ccc2
Comment 13•6 years ago
|
||
Pixel XL - Android 8.0 Using Nightly 59.0a1, Beta 58.0b3 and Release 57.0 I followed the bug for several weeks in order to verify it on different screens and hardware and i am not able to reproduce it anymore.
Status: RESOLVED → VERIFIED
Flags: needinfo?(ioana.chiorean)
Updated•1 year ago
|
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•