Closed Bug 1586496 Opened 10 months ago Closed 9 months ago

Second click is ignored if fast and in a different spot in Fenix

Categories

(Core :: Panning and Zooming, defect, P2)

69 Branch
ARM
Android
defect

Tracking

()

RESOLVED FIXED
mozilla72
Tracking Status
firefox-esr68 71+ verified
firefox70 --- wontfix
firefox71 --- wontfix
firefox72 --- fixed

People

(Reporter: jaume, Assigned: botond)

Details

(Whiteboard: [fennec68.3])

Attachments

(2 files, 1 obsolete file)

Attached file ff.html

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

Given a mobile friendly site (<meta name="viewport" content="width=device-width, initial-scale=1"> or touch-action: manipulation;), when two taps happen in different parts of the screen in a small time frame, the second one is ignored and not treated as a click.

I have attached a minimal test html file (which is also uploaded at https://www.cosarara.me/jaume/ff.html ), where you can tap any of the two elements as fast as you want and clicks will always register (as long as you keep tapping the same spot), but if you alternate between the two elements, only the taps on one of them register.

You can also use the same element with different fingers to the same effect, proving this is not a matter of different elements but simply distance.

If you slow down leaving over 350ms between clicks, both will register.

Actual results:

touchstart and touchend events always fire, but the second click event does not.

Expected results:

Both clicks should fire, like it would happen if both were done on the same spot.

Ignoring the event makes firefox seem slow and unresponsive.

This works as expected on desktop firefox using a mouse, and also tapping on firefox focus (8.0.16) and chrome for android 77.0.
It fails the same way on Firefox Preview v2.0.0 package 12.0.0 gecko 7.0.

I have managed to reproduce your issue on Release 68.1.1 and Nightly 68.2a1 (2019-10-05) using a Samsung Galaxy S8+ (Android 8.0.0). I will set this issue as new.

Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Unspecified → Android
Hardware: Unspecified → ARM

I have also managed to reproduce the issue on Firefox Preview and submitted a separate issue: https://github.com/mozilla-mobile/fenix/issues/5844.

Asking the GV team to triage this as this may be platform related.

Product: Firefox for Android → GeckoView
Version: Firefox 68 → 69 Branch

Randall says that is an APZ issue and should be moved to the graphics component.

Moving to APZ component

Component: General → Panning and Zooming
Product: GeckoView → Core
Summary: Second click is ignored if fast and in a different spot → Second click is ignored if fast and in a different spot in Fenix

Just to clarify, this is in both Fenix and Fennec (although maybe the fix will never reach the later).

Looks like we unconditionally go into the one-touch-pinch code which doesn't do anything on this page (since it cannot be zoomed), and probably prevents us from firing the click.

Not sure what the best way to handle this is, maybe we need to check if zooming is possible and if not, just handle the second touch as a regular touch down without one-touch-pinch.

The priority flag is not set for this bug.
:botond, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(botond)

I haven't had a chance to confirm the diagnosis or investigate further yet, but I plan to. Will park the bug with me in the meantime.

Assignee: nobody → botond
Flags: needinfo?(botond)
Priority: -- → P2

(In reply to Kartikaya Gupta (email:kats@mozilla.com) (away until Feb-2020) from comment #7)

Looks like we unconditionally go into the one-touch-pinch code which doesn't do anything on this page (since it cannot be zoomed), and probably prevents us from firing the click.

I don't think this is the problem, because the linked code is in HandleInputTouchMove(), but the issue occurs even if only touch-star / touch-end events are involved.

Rather, it appears the problem is that if a second tap is close enough in time to be a double-tap (i.e. we're still in the GESTURE_FIRST_SINGLE_TOUCH_UP state) but far in distance, we bail by setting the state to GESTURE_NONE, which prevents that second tap from triggering a gesture event such as a single-tap.

Attached file Bug 1586496 - Add a gtest. r=tnikkel (obsolete) —

Depends on D51479

Pushed by bballo@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7cec59e15970
If two taps are close in time but far in distance, still allow the second one to start a gesture. r=tnikkel
https://hg.mozilla.org/integration/autoland/rev/8aa8ed80ea43
Add a gtest. r=tnikkel
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla72

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

I have applied the patch on fennec esr68 and it works perfectly. Will it be officially backported there?

(In reply to Jaume Delclòs Coll from comment #17)

I have applied the patch on fennec esr68 and it works perfectly.

Great, thanks for checking!

Will it be officially backported there?

I think the patch is small and low-risk enough to backport. I would like to give it a chance to bake on nightly for a week or two first.

Comment on attachment 9105919 [details]
Bug 1586496 - If two taps are close in time but far in distance, still allow the second one to start a gesture. r=tnikkel

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: To make the fix available to users of Fennec which is on the 68esr branch
  • User impact if declined: A tap may be ignored if it's close in time to a previous tap (but far enough in distance not to be a double-tap)
  • Fix Landed on Version: 72
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Small, localize changed to APZ gesture handling code
  • String or UUID changes made by this patch:
Attachment #9105919 - Flags: approval-mozilla-esr68?
Attachment #9105920 - Flags: approval-mozilla-esr68?

Comment on attachment 9105919 [details]
Bug 1586496 - If two taps are close in time but far in distance, still allow the second one to start a gesture. r=tnikkel

OK for uplift, useful fix for Fennec.

Attachment #9105919 - Flags: approval-mozilla-esr68? → approval-mozilla-esr68+

It looks like the test would need to be rebased across the prefs refactor to apply cleanly. We could do that, but I think the incremental value of having the test run on the esr68 branch is low enough that we can just leave it backed out. Thanks!

Flags: needinfo?(botond)
Attachment #9105920 - Attachment is obsolete: true
Attachment #9105920 - Flags: approval-mozilla-esr68?
Flags: in-testsuite+
Whiteboard: [fennec68.3]

I have tested the issue on 68.3.0 RC using a Google Pixel 3a (Android 9) and Google Pixel 3 XL (Android 9) and the issue no longer occurs.

You need to log in before you can comment on or make changes to this bug.