1071758 - Tapping sometimes triggers two clicks

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Attachment: 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.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch

Comment 4

[Brian Nicholson (: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?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/integration/mozilla-inbound/rev/025db34f7e8b

Comment 7

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/025db34f7e8b

Comment 9

[Kartikaya Gupta (email:[e-mail])]

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.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kartikaya Gupta (email:[e-mail]) 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.

Comment 11

[Catalin Suciu [:csuciu]]

I'm not able to reproduce this issue on Firefox 35 Beta 1.
Verifying as fixed

Comment 12

[Stefan Deiac]

Based on comment 11 the qe-verify flag can be removed.