Closed Bug 946661 Opened 6 years ago Closed 6 years ago
Taps > 300ms are ignored if APZC content disables zooming
No description provided.
To clarify this bug, https://bugzilla.mozilla.org/show_bug.cgi?id=922896 regressed https://bugzilla.mozilla.org/show_bug.cgi?id=915645, any press over 300ms is ignored as it triggers a tapconfirmed directly which is ignored on zoomable content
This is half the fix that was in https://bugzilla.mozilla.org/show_bug.cgi?id=942929, split out into its own patch and avoiding extra state variables. Now we only run double tap timeout if the controller has told us it ignored the tapup, so we wont get tapconfirmed events following a tapup that has been handled which means the controller can handle tapconfirmed unconditionally.
Attachment #8344578 - Flags: review?(bugmail.mozilla)
This adds tests to ensure we don't regress this again. The MediumPress test fails on current m-c and passes with your patch. Please try to add similar tests for additional changes to this code as it tends to be quite brittle.
Attachment #8344649 - Flags: review?(dale)
Comment on attachment 8344578 [details] [diff] [review] Dont send tapconfirmed event when tapup was handled Review of attachment 8344578 [details] [diff] [review]: ----------------------------------------------------------------- This seems reasonable to me. Please also add a comment in APZC::OnSingleTapUp next to the "&& !mAllowZoom" condition that says if mAllowZoom is true we wait for a call to OnSingleTapConfirmed before dispatching to content.
Attachment #8344578 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8344649 [details] [diff] [review] Tests! Thanks a lot, I had made a start on the tests however I was doing https://bugzilla.mozilla.org/show_bug.cgi?id=942929 at the same time and attempting to test both, also the test framework is fairly new to me but much easier to understand seeing tests that go along with the patch.
Attachment #8344649 - Flags: review?(dale) → review+
Added the comment and rebased on top of the patch you created for the tests
Pushed the 3 patches to try: https://tbpl.mozilla.org/?tree=Try&rev=e0905361a062
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
Based on comment 0 this is a regression in Gecko 27. I don't think we need to uplift to the beta branch but we should uplift to aurora and get it in B2G 1.3 at least.
Comment on attachment 8344649 [details] [diff] [review] Tests! (This request is for both patches) [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 922896 User impact if declined: doing a tap where your finger is down for 300-500ms is not treated as a tap. Testing completed (on m-c, etc.): on m-c, in gtest Risk to taking this patch (and alternatives if risky): pretty low risk. it's in code that is used on B2G and metro but nowhere else. String or IDL/UUID changes made by this patch: none
Attachment #8344649 - Flags: approval-mozilla-aurora?
blocking-b2g: 1.3? → ---
Attachment #8344649 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.