New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

No touch are ever events fired if the first finger went down in an area with no handler

Reported by splinte...@gmail.com, Apr 14 2014

Issue description

Steps to reproduce the problem:
1. open http://patrickhlauke.github.io/touch/tests/event-listener.html
2. place one finger on the touchscreen, anywhere outside of the button
3. with the first finger still on the screen, tap the button with a second finger

What is the expected behavior?
Android4.3/Firefox, iOS7/Safari still fire the expected touchstart > touchmove > touchend sequence (but no mouse compat events, as those are only generated for single-finger interactions)

What went wrong?
Chrome M34, M35 beta, but also Android4.3/"Browser" and related (e.g. Opera etc) fire absolutely no events?

Did this work before? N/A 

Chrome version: 34.0.1847.114  Channel: stable
OS Version: 4.3
Flash Version: 

See https://www.youtube.com/watch?v=llPo5KKQAck

 

Comment 1 by rbyers@chromium.org, Apr 14 2014

Cc: tdres...@chromium.org
Labels: -Cr-Blink-JavaScript -Pri-2 -Type-Bug -OS-Android Cr-Blink-Input Pri-1 Type-Bug-Regression OS-All M-34
Owner: jdduke@chromium.org
Status: Assigned
I can repro this and it does look like a bug.  Thank you very much for the report!

This reproduces whenever the first finger lands on a region without a touch event handler (and so gets the NO_CONSUMER_EXISTS ACK), now we won't even send the touchstart for a subsequent finger.  I suspect we regressed this in http://crrev.com/246893 (first landed in M34) as part of  issue 295075 .  We enter the 'DROP_TOUCHES_IN_SEQUENCE' state and so TouchEventQueue::QueueEvent decides not to send the second touchstart. 

Perhaps the right fix is to change the 'drop all events in this touch sequence' state to a 'drop all events for the active touches' state.  When a new touchstart comes we must send it and again decide what state to enter based on the ACK (either remain in this state if there's still no consumer or enter FORWARD_ALL_TOUCHES/FORWARD_TOUCHES_UNTIL_TIMEOUT). 

Jared, do you want to take this?  Presumably it's too late to fix this in M34 (and it's  probably rare enough that it's OK), but we should perhaps consider merging a fix to M35 if it's low risk.

Note that a work around is to install a dummy touch event handler on the document (but note that can degrade scroll start performance).

I verified that this reproduces on Aura as well.

Comment 2 by rbyers@chromium.org, Apr 14 2014

Summary: No touch are ever events fired if the first finger went down in an area with no handler (was: No touch events fired on a button element if any other fingers already on touchscreen)

Comment 3 by jdduke@chromium.org, Apr 14 2014

Yeah, Chrome on Android appears to have always (at least since M26 or so) had this behavior.  We can make this change, but should we only allow it if the first touch point has not exceed the slop threshold?

Comment 4 by rbyers@chromium.org, Apr 14 2014

Labels: -Pri-1 -Type-Bug-Regression -M-34 Pri-2 Type-Bug M-36
Confirmed this is broken in M33, thanks.  I could have sworn I'd tested this scenario when I was working on cc touch hit testing, but it must have been only on Aura that I checked.  Since we haven't heard complaints before, I think that means this is Pri-2 for M36 (still work fixing though).

>  We can make this change, but should we only allow it if the first touch point has not exceed the slop threshold?

Well in the touchmove absorption model we'd always send it.  In the touchcancel model, once we've sent the GestureScrollBegin (and so the touchcancel for the first finger) then yes we shouldn't be sending touchstart for subsequent fingers since event handling has already been disrupted. 
Cc: zeeshanq@chromium.org

Comment 6 by rbyers@chromium.org, Apr 28 2014

I see blink is broken here too - I'll work on a fix for blink now.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 29 2014

Labels: merge-merged-git-svn
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5abd744ba1a9acf6a169fdb0aa015009ab7792c1

commit 5abd744ba1a9acf6a169fdb0aa015009ab7792c1
Author: jdduke@chromium.org <jdduke@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Tue Apr 29 19:25:53 2014 +0000

Forward secondary touch points even if the first had no consumer

Previously, if the first touch point in a sequence had no consumer, no further
touches from that sequence would be forwarded to the renderer.  Instead, only
prevent touches from being forwarded if *none* of the current touch points have
a consumer when scrolling begins.

Also fix a corner case where follow-up touches triggered acks from deferred
async touchmoves would never be forwarded.

BUG= 363321 

Review URL: https://codereview.chromium.org/259763010

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266934 0039d316-1c4b-4281-b951-d872f2087c98


Project Member

Comment 8 by bugdroid1@chromium.org, Apr 29 2014

------------------------------------------------------------------
r266934 | jdduke@chromium.org | 2014-04-29T19:25:53.800706Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/touch_event_queue.h?r1=266934&r2=266933&pathrev=266934
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/touch_event_queue_unittest.cc?r1=266934&r2=266933&pathrev=266934
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/input/touch_event_queue.cc?r1=266934&r2=266933&pathrev=266934

Forward secondary touch points even if the first had no consumer

Previously, if the first touch point in a sequence had no consumer, no further
touches from that sequence would be forwarded to the renderer.  Instead, only
prevent touches from being forwarded if *none* of the current touch points have
a consumer when scrolling begins.

Also fix a corner case where follow-up touches triggered acks from deferred
async touchmoves would never be forwarded.

BUG= 363321 

Review URL: https://codereview.chromium.org/259763010
-----------------------------------------------------------------

Comment 9 by rbyers@chromium.org, Apr 29 2014

Owner: rbyers@chromium.org
Thanks, over to me for the remaining issues in blink.
Status: Started
Up for review here: https://codereview.chromium.org/259413003/
Status: Fixed
Blink side fixed by https://src.chromium.org/viewvc/blink?view=rev&revision=173245
Project Member

Comment 12 by bugdroid1@chromium.org, May 5 2014

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=173245

------------------------------------------------------------------
r173245 | rbyers@chromium.org | 2014-05-02T23:01:46.322147Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/touch/multi-touch-inside-iframes.html?r1=173245&r2=173244&pathrev=173245
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/touch/multi-touch-inside-nested-iframes.html?r1=173245&r2=173244&pathrev=173245
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/touch/multi-touch-partial-sequence.html?r1=173245&r2=173244&pathrev=173245
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/EventHandler.cpp?r1=173245&r2=173244&pathrev=173245
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/touch/multi-touch-inside-iframes-expected.txt?r1=173245&r2=173244&pathrev=173245
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/touch/multi-touch-inside-nested-iframes-expected.txt?r1=173245&r2=173244&pathrev=173245
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/EventHandler.h?r1=173245&r2=173244&pathrev=173245
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/events/touch/multi-touch-partial-sequence-expected.txt?r1=173245&r2=173244&pathrev=173245

Handle touch events containing touches not previously reported to blink

Chromium has a couple optimizations which suppress sending touch events
to the renderer (for example, when they're at a place or time known not
to contain touch event handlers).  Our security fix in
 http://crbug.com/148567  assumed we'd always see every touchstart event,
so if we got an event which had a point we'd never seen we'd
effectively ignore the event completely.

Touch events do their hit-test on touchstart, and every TouchEvent
contains a touches array for all active touches.  So if we're going
to skip sending some touchstart events, we need to define how those
active touch points show up in the events for other points.  This
problem is similar to how touches should show up for fingers that
are down on iframes outside the bounds of the document receiving
touch events.  I think it's important that we include such touches
(otherwise an app could mis-recognize a two-finger gesture as a
single finger one), but their 'target' should be irrelevant since
we know it wasn't capable of receiving the touch events.  So here
I just set the target to the document.  This also updates the
cross-iframe case, we were previously hiding such points completely.

I've split the main loop across points into two stages (each with
their own loop).  First we do any necessary hit tests, including
establishing the active touch sequence document if it wasn't
previously determined.  Then we create all the Touch objects,
where we may rely on the active touch sequence document determined
by some later point.

This CL also contains a number of overdue cleanups to account for the
fact that we now only need to hit-test on touchstart, and that there's
only ever a single document receiving touch events at any time.
Also rather than use a funky ID+1 key for our touch target HashMap,
just use the ID but ensure HashMap supports 0-value keys by using
UnsignedWithZeroKeyHashTraits.

BUG= 363321 

Review URL: https://codereview.chromium.org/259413003
-----------------------------------------------------------------
Project Member

Comment 13 by bugdroid1@chromium.org, May 5 2014

The following revision refers to this bug:
  http://src.chromium.org/viewvc/blink?view=rev&rev=173271

------------------------------------------------------------------
r173271 | sigbjornf@opera.com | 2014-05-04T04:40:44.379370Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/EventHandler.cpp?r1=173271&r2=173270&pathrev=173271

Oilpan: fix build after r173245.

Remove unused allocation of a TouchList object.

R=
BUG= 363321 

Review URL: https://codereview.chromium.org/264773015
-----------------------------------------------------------------

Sign in to add a comment