New issue
Advanced search Search tips

Issue 729031 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Can't cancel touchmove from a non-passive touchmove listener added from passive touchstart handler

Project Member Reported by rbyers@chromium.org, Jun 2 2017

Issue description

Chrome 60.0.3107.4.
Repro: https://jsfiddle.net/altano/a41cLLkd/

The non-passive listener is added too late to affect the scrolling decision. This could matter for example in a case like Facebook's "Long press on Like to customize your reaction" where touchstart needs to be passive for good document scrolling performance (though ideally there'd be no need to add/remove the touchmove listener dynamically).

With this fixed there will be a race condition: if the user starts scrolling quickly than the scroll will have started before the non-passive touchmove listener gets added.  But that's probably OK, not unlike the race condition involved in changing 'touch-action: pan-y' to 'touch-action: pan-up' when at scrollTop==0.  The only legitimate use case I can think of here is around long-press gestures, where there really shouldn't be a race in practice.
 
BTW we debated a bit whether this should be considered by-design (since it results in deterministic behavior) or a bug (which, when fixed, can only be leveraged in race scenarios).  We decided that given the long press scenario (where the race is unlikely to matter in practice) it was reasonable to consider it a bug.
Labels: Hotlist-Input-Dev
Project Member

Comment 3 by bugdroid1@chromium.org, Jun 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75fe739ccdd005ba0faa8eda191c03a94cc269cb

commit 75fe739ccdd005ba0faa8eda191c03a94cc269cb
Author: Dave Tapuska <dtapuska@chromium.org>
Date: Tue Jun 06 15:29:45 2017

Allow a passive touchstart listener to add a blocking touchmove listener.

We used the touchstart result for all touchmove results. This would
prevent being able to add a blocking touchmove inside a passive
touchstart listener. Hit test the first touchmove that has exceeded the
slop region.

BUG= 729031 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: If362f45ea5b233892830cd699a3433d2ccc4162a
Reviewed-on: https://chromium-review.googlesource.com/523182
Reviewed-by: Timothy Dresser <tdresser@chromium.org>
Reviewed-by: Alexandre Elias <aelias@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#477293}
[modify] https://crrev.com/75fe739ccdd005ba0faa8eda191c03a94cc269cb/cc/input/input_handler.h
[modify] https://crrev.com/75fe739ccdd005ba0faa8eda191c03a94cc269cb/cc/trees/layer_tree_host_impl.cc
[modify] https://crrev.com/75fe739ccdd005ba0faa8eda191c03a94cc269cb/cc/trees/layer_tree_host_impl.h
[modify] https://crrev.com/75fe739ccdd005ba0faa8eda191c03a94cc269cb/cc/trees/layer_tree_host_impl_unittest.cc
[modify] https://crrev.com/75fe739ccdd005ba0faa8eda191c03a94cc269cb/ui/events/blink/input_handler_proxy.cc
[modify] https://crrev.com/75fe739ccdd005ba0faa8eda191c03a94cc269cb/ui/events/blink/input_handler_proxy.h
[modify] https://crrev.com/75fe739ccdd005ba0faa8eda191c03a94cc269cb/ui/events/blink/input_handler_proxy_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment