Closed Bug 1071758 Opened 5 years ago Closed 5 years ago

Tapping sometimes triggers two clicks

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 35
Tracking Status
firefox35 --- verified
fennec 35+ ---

People

(Reporter: kats, Assigned: kats)

References

()

Details

(Keywords: regression)

Attachments

(2 files)

Attached file Test page
STR:
1. Load up the attached test page in nightly (I'm using a Nexus 4)
2. Zoom in so that the right column takes up about the whole screen width (for some reason the bug doesn't manifest if you're zoomed out, or maybe it's just more intermittent)
3. click on the "X" in the top-right corner of the bug box

Expected results:
The bug box you clicked "X" on should disappear and everything below it should move up

Actual results:
The bug box you clicked on disappears, and things move up. The bug box that is now where the old one was *also* disappears, and everything below it moves up again.

If it doesn't reproduce the first time try again with the next bug box.

I'm not actually sure if it's two click events that are getting dispatched or not bug consider that's all that the page listens for I'm guessing that's what is happening. I suspect this is a regression from bug 788073 since it wasn't happening on yesterday's Nightly.
Attachment #8493920 - Attachment mime type: text/plain → text/html
Also it seems to happen more reliably with a "slow tap" - leave your finger down for 100ms or so rather than doing a super quick tap.
tracking-fennec: --- → ?
Assignee: nobody → bnicholson
tracking-fennec: ? → 35+
Keywords: regression
I investigated this a bit more and based on what I found it's actually a very old bug. I guess I only just started noticing it? What's happening is that for medium-length taps we get calls to onDown, onShowPress, onSingleTapUp and onSingleTapConfirmed in JavaPanZoomController. Both onSingleTapUp and onSingleTapConfirmed end up dispatching the Gesture:SingleTap because mMediumPress is set to true and getAllowDoubleTapZoom() is also true.
Assignee: bnicholson → bugmail.mozilla
Blocks: 876060
No longer blocks: 788073
Attached patch PatchSplinter Review
Attachment #8496645 - Flags: review?(bnicholson)
Comment on attachment 8496645 [details] [diff] [review]
Patch

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

::: mobile/android/base/gfx/JavaPanZoomController.java
@@ +1362,2 @@
>              sendPointToGecko("Gesture:SingleTap", motionEvent);
>          }

Makes sense, though I don't quite understand why we need to call sendPointToGecko at all in onSingleTapUp. What would happen if you removed this, then called sendPointToGecko in onSingleTapConfirmed unconditionally?
Attachment #8496645 - Flags: review?(bnicholson) → review+
onSingleTapConfirmed doesn't get called if the user does a double-tap. However, if we're not using the double-tap for a block zoom, we still need to dispatch a click event to content. The other reason we hook into onSingleTapUp is so that we don't have to wait for the 300ms double-tap delay in cases where we know we're not going to zoom on a double-tap anyway.
bnicholson said he wasn't able to reproduce this error with bug 788073 backed out, so it's possible that this error was masked before. I didn't dig into the details exactly, but I'll put it back as a dependency.
Blocks: 788073
https://hg.mozilla.org/mozilla-central/rev/025db34f7e8b
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 35
Flags: in-testsuite?
Flags: qe-verify+
I've started seeing this problem again actually, in the last few days. I'll file a new bug once I have some time to hammer down the STR, if somebody doesn't beat me to it.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #9)
> I've started seeing this problem again actually, in the last few days. I'll
> file a new bug once I have some time to hammer down the STR, if somebody
> doesn't beat me to it.

Never mind, I can't reproduce it. I might have actually double-pressed or something.
I'm not able to reproduce this issue on Firefox 35 Beta 1.
Verifying as fixed
Status: RESOLVED → VERIFIED
Based on comment 11 the qe-verify flag can be removed.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.