Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2669663002:
Move touch slop suppression to TouchEventManager (Closed)
|
Created:
3 years, 10 months ago by lanwei Modified:
3 years, 10 months ago CC:
chromium-reviews, dtapuska+blinkwatch_chromium.org, jam, dtapuska+chromiumwatch_chromium.org, darin-cc_chromium.org, Navid Zolghadr, blink-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMove touch slop suppression from LegacyTouchEventQueue to TouchEventManager
We do not want to suppress pointer events, but the current code suppresses touch events from browser side, so the pointer events are also suppressed. We move the slop region suppression code to TouchEventManager, so all the pointer moves will be dispatched.
BUG=593061
Review-Url: https://codereview.chromium.org/2669663002
Cr-Commit-Position: refs/heads/master@{#448773}
Committed: https://chromium.googlesource.com/chromium/src/+/00f7089132148e96541a6a476a68809790048e81
Patch Set 1 : slop region #
Total comments: 4
Patch Set 2 : slop region #Patch Set 3 : slop region #
Total comments: 11
Patch Set 4 : slop region #
Total comments: 7
Patch Set 5 : slop regesion #
Total comments: 5
Patch Set 6 : slop regesion #Messages
Total messages: 74 (56 generated)
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== slop regesion BUG=593061 ========== to ========== Move touch slop suppression from LegacyTouchEventQueue to TouchEventManager We do not want to suppress pointer events, but the current code suppresses touch events from browser side, so the pointer events are also suppressed. We move the slop region suppression code to TouchEventManager, so all the pointer moves will be dispatched. BUG=593061 ==========
lanwei@chromium.org changed reviewers: + dtapuska@chromium.org, mustaq@chromium.org
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
We should also test touch emulation in the inspector I suspect it is broken.. Specifically we probably need to set moved beyond slop region in here: https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touc... https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/input/TouchEventManager.cpp (right): https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/TouchEventManager.cpp:118: if (event.type() == WebInputEvent::TouchStart) I don't think this matches the consistent state we had before. For example if you get a touch start for the second touch point it will enter suppression mode; lift that finger and it will still be in supression mode. touch slop supression should enter exactly as it was before. it is a touch start and all the touch states are statepressed; likewise it should exit on touchend and touchcancel and if the touchInfo.size() > 1
Patchset #1 (id:20001) has been deleted
https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/input/TouchEventManager.h (right): https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/TouchEventManager.h:98: bool m_suppressingTouchmoves; Please make it clear that this is "suppressingMovesWithinSlop", to rule out possible mixup with other suppressions.
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:40001) has been deleted
On 2017/02/01 15:42:06, dtapuska wrote: > We should also test touch emulation in the inspector I suspect it is broken.. > > Specifically we probably need to set moved beyond slop region in here: > https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touc... > > https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/input/TouchEventManager.cpp (right): > > https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/input/TouchEventManager.cpp:118: if (event.type() > == WebInputEvent::TouchStart) > I don't think this matches the consistent state we had before. For example if > you get a touch start for the second touch point it will enter suppression mode; > lift that finger and it will still be in supression mode. > > touch slop supression should enter exactly as it was before. it is a touch start > and all the touch states are statepressed; likewise it should exit on touchend > and touchcancel and if the touchInfo.size() > 1 https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touc... I thought the movedBeyondSlopRegion is set here for touch emulation.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2017/02/01 20:51:30, lanwei wrote: > On 2017/02/01 15:42:06, dtapuska wrote: > > We should also test touch emulation in the inspector I suspect it is broken.. > > > > Specifically we probably need to set moved beyond slop region in here: > > > https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touc... > > > > > https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... > > File third_party/WebKit/Source/core/input/TouchEventManager.cpp (right): > > > > > https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... > > third_party/WebKit/Source/core/input/TouchEventManager.cpp:118: if > (event.type() > > == WebInputEvent::TouchStart) > > I don't think this matches the consistent state we had before. For example if > > you get a touch start for the second touch point it will enter suppression > mode; > > lift that finger and it will still be in supression mode. > > > > touch slop supression should enter exactly as it was before. it is a touch > start > > and all the touch states are statepressed; likewise it should exit on touchend > > and touchcancel and if the touchInfo.size() > 1 > > https://cs.chromium.org/chromium/src/content/browser/renderer_host/input/touc... > I thought the movedBeyondSlopRegion is set here for touch emulation. I think you might be failing on mac because touch events don't work there right? lgtm; tdresser@ might want to have a look as well.
dtapuska@chromium.org changed reviewers: + tdresser@chromium.org
tdresser@ care to have a look?
lgtm
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lanwei@chromium.org changed reviewers: + rbyers@chromium.org
https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/input/TouchEventManager.cpp (right): https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/TouchEventManager.cpp:118: if (event.type() == WebInputEvent::TouchStart) On 2017/02/01 15:42:06, dtapuska wrote: > I don't think this matches the consistent state we had before. For example if > you get a touch start for the second touch point it will enter suppression mode; > lift that finger and it will still be in supression mode. > > touch slop supression should enter exactly as it was before. it is a touch start > and all the touch states are statepressed; likewise it should exit on touchend > and touchcancel and if the touchInfo.size() > 1 Done. https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/input/TouchEventManager.h (right): https://codereview.chromium.org/2669663002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/core/input/TouchEventManager.h:98: bool m_suppressingTouchmoves; On 2017/02/01 20:17:10, mustaq wrote: > Please make it clear that this is "suppressingMovesWithinSlop", to rule out > possible mixup with other suppressions. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
https://codereview.chromium.org/2669663002/diff/100001/content/browser/render... File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right): https://codereview.chromium.org/2669663002/diff/100001/content/browser/render... content/browser/renderer_host/input/touch_event_queue_unittest.cc:1392: // suppression region for an unconsumed TouchStart. Remove references in this file to slop suppression. https://codereview.chromium.org/2669663002/diff/100001/content/browser/render... content/browser/renderer_host/input/touch_event_queue_unittest.cc:1440: // not be suppressed and its movedBeyondSlopRegion is set to true. Don't refer to suppression here anymore. It implies there's something unique about this event with respect to being suppressed. https://codereview.chromium.org/2669663002/diff/100001/content/browser/render... content/browser/renderer_host/input/touch_event_queue_unittest.cc:1560: // TouchMove's should not be suppressed, even with the original Don't talk about suppression anymore. https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html:41: if (window.chrome && chrome.gpuBenchmarking) { Aren't we trying to use the JS wrapper of this most of the time? Can we use it here? https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/inspector/InspectorInputAgent.cpp (right): https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorInputAgent.cpp:115: movedBeyondSlopRegion = true; How does this value end up being used? Does it matter if this is true or not?
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #4 (id:120001) has been deleted
https://codereview.chromium.org/2669663002/diff/100001/content/browser/render... File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right): https://codereview.chromium.org/2669663002/diff/100001/content/browser/render... content/browser/renderer_host/input/touch_event_queue_unittest.cc:1392: // suppression region for an unconsumed TouchStart. On 2017/02/02 21:30:54, tdresser wrote: > Remove references in this file to slop suppression. Done. https://codereview.chromium.org/2669663002/diff/100001/content/browser/render... content/browser/renderer_host/input/touch_event_queue_unittest.cc:1440: // not be suppressed and its movedBeyondSlopRegion is set to true. On 2017/02/02 21:30:54, tdresser wrote: > Don't refer to suppression here anymore. It implies there's something unique > about this event with respect to being suppressed. Done. https://codereview.chromium.org/2669663002/diff/100001/content/browser/render... content/browser/renderer_host/input/touch_event_queue_unittest.cc:1560: // TouchMove's should not be suppressed, even with the original On 2017/02/02 21:30:54, tdresser wrote: > Don't talk about suppression anymore. Done. https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html:41: if (window.chrome && chrome.gpuBenchmarking) { On 2017/02/02 21:30:54, tdresser wrote: > Aren't we trying to use the JS wrapper of this most of the time? Can we use it > here? Done. https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/inspector/InspectorInputAgent.cpp (right): https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorInputAgent.cpp:115: movedBeyondSlopRegion = true; On 2017/02/02 21:30:54, tdresser wrote: > How does this value end up being used? Does it matter if this is true or not? Yes, inspector-protocol/input/dispatchTouchEvent.html this test creates WebTouchEvent in blink, so the movedBeyondSlopRegion is not set correctly based on the touch movement. We have to hard code to true.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL) win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_TIMED_OUT, no build URL) win_chromium_rel_ng on master.tryserver.chromium.win (JOB_TIMED_OUT, no build URL) win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_TIMED_OUT, no build URL) win_clang on master.tryserver.chromium.win (JOB_TIMED_OUT, no build URL)
Patchset #4 (id:140001) has been deleted
https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/inspector/InspectorInputAgent.cpp (right): https://codereview.chromium.org/2669663002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/inspector/InspectorInputAgent.cpp:115: movedBeyondSlopRegion = true; On 2017/02/03 21:39:33, lanwei wrote: > On 2017/02/02 21:30:54, tdresser wrote: > > How does this value end up being used? Does it matter if this is true or not? > > Yes, inspector-protocol/input/dispatchTouchEvent.html this test creates > WebTouchEvent in blink, so the movedBeyondSlopRegion is not set correctly based > on the touch movement. We have to hard code to true. It's a bit weird that we're setting this on non-move touch events. https://codereview.chromium.org/2669663002/diff/160001/components/test_runner... File components/test_runner/event_sender.cc (right): https://codereview.chromium.org/2669663002/diff/160001/components/test_runner... components/test_runner/event_sender.cc:2245: if (args->GetNext(&arg) && arg == "movedInsideSlopRegion") Can we just get rid of this? It's never used. https://codereview.chromium.org/2669663002/diff/160001/content/browser/render... File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right): https://codereview.chromium.org/2669663002/diff/160001/content/browser/render... content/browser/renderer_host/input/touch_event_queue_unittest.cc:1452: // Tests that TouchMove's are not dropped within the slop suppression region if We're still referring to suppression. https://codereview.chromium.org/2669663002/diff/160001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/160001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html:48: var pointerActions = [{'source': 'touch'}, {'source': 'touch'}]; We're still not using the wrapper JS. Should we be? https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/...
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Patchset #5 (id:180001) has been deleted
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2669663002/diff/160001/components/test_runner... File components/test_runner/event_sender.cc (right): https://codereview.chromium.org/2669663002/diff/160001/components/test_runner... components/test_runner/event_sender.cc:2245: if (args->GetNext(&arg) && arg == "movedInsideSlopRegion") On 2017/02/06 14:25:59, tdresser wrote: > Can we just get rid of this? It's never used. Done. https://codereview.chromium.org/2669663002/diff/160001/content/browser/render... File content/browser/renderer_host/input/touch_event_queue_unittest.cc (right): https://codereview.chromium.org/2669663002/diff/160001/content/browser/render... content/browser/renderer_host/input/touch_event_queue_unittest.cc:1452: // Tests that TouchMove's are not dropped within the slop suppression region if On 2017/02/06 14:25:59, tdresser wrote: > We're still referring to suppression. Removed, we cannot test it anymore. https://codereview.chromium.org/2669663002/diff/160001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/160001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html:48: var pointerActions = [{'source': 'touch'}, {'source': 'touch'}]; On 2017/02/06 14:25:59, tdresser wrote: > We're still not using the wrapper JS. Should we be? > https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/... First, we are not ready to use the pointerevent_common_input.js in layout tests right now, because it is in the LayoutTests/external/ folder, I will move it under LayoutTests/. The actions in this test is not covered in the pointerevent_common_input.js, it is a special test action.
Sorry for the confusion on how to synthesize input here. https://codereview.chromium.org/2669663002/diff/160001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/160001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html:48: var pointerActions = [{'source': 'touch'}, {'source': 'touch'}]; On 2017/02/06 21:36:23, lanwei wrote: > On 2017/02/06 14:25:59, tdresser wrote: > > We're still not using the wrapper JS. Should we be? > > > https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/external/... > > First, we are not ready to use the pointerevent_common_input.js in layout tests > right now, because it is in the LayoutTests/external/ folder, I will move it > under LayoutTests/. > The actions in this test is not covered in the pointerevent_common_input.js, it > is a special test action. Acknowledged. https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html:48: var pointerActions = [{'source': 'touch'}, {'source': 'touch'}]; Let's just use a single literal here. https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-event-in-slop-region.html:48: var pointerActions = [{'source': 'touch'}]; This might as well be a single object literal, instead of a bunch of calls to push, right?
LGTM with above nits.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM with nit https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-event-in-slop-region.html:48: var pointerActions = [{'source': 'touch'}]; On 2017/02/06 21:49:59, tdresser wrote: > This might as well be a single object literal, instead of a bunch of calls to > push, right? +1 Also you can make it slightly easier to read by omitting quotes around the property names. Eg: var pointerActions = [ {source: 'touch'}, {actions: [ {name: 'pointerDown', x: x, y: y}, ... ]}];
The CQ bit was checked by lanwei@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by lanwei@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mustaq@chromium.org, dtapuska@chromium.org, tdresser@chromium.org, rbyers@chromium.org Link to the patchset: https://codereview.chromium.org/2669663002/#ps220001 (title: "slop regesion")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 220001, "attempt_start_ts": 1486503767744740,
"parent_rev": "ed4f8466dbb2bdc88bfee21b9ecabb16132b1316", "commit_rev":
"00f7089132148e96541a6a476a68809790048e81"}
Message was sent while issue was closed.
Description was changed from ========== Move touch slop suppression from LegacyTouchEventQueue to TouchEventManager We do not want to suppress pointer events, but the current code suppresses touch events from browser side, so the pointer events are also suppressed. We move the slop region suppression code to TouchEventManager, so all the pointer moves will be dispatched. BUG=593061 ========== to ========== Move touch slop suppression from LegacyTouchEventQueue to TouchEventManager We do not want to suppress pointer events, but the current code suppresses touch events from browser side, so the pointer events are also suppressed. We move the slop region suppression code to TouchEventManager, so all the pointer moves will be dispatched. BUG=593061 Review-Url: https://codereview.chromium.org/2669663002 Cr-Commit-Position: refs/heads/master@{#448773} Committed: https://chromium.googlesource.com/chromium/src/+/00f7089132148e96541a6a476a68... ==========
Message was sent while issue was closed.
Committed patchset #6 (id:220001) as https://chromium.googlesource.com/chromium/src/+/00f7089132148e96541a6a476a68...
Message was sent while issue was closed.
https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/multi-pointer-event-in-slop-region.html:48: var pointerActions = [{'source': 'touch'}, {'source': 'touch'}]; On 2017/02/06 21:49:59, tdresser wrote: > Let's just use a single literal here. Done. https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... File third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-event-in-slop-region.html (right): https://codereview.chromium.org/2669663002/diff/200001/third_party/WebKit/Lay... third_party/WebKit/LayoutTests/fast/events/pointerevents/pointer-event-in-slop-region.html:48: var pointerActions = [{'source': 'touch'}]; On 2017/02/07 18:51:43, Rick Byers wrote: > On 2017/02/06 21:49:59, tdresser wrote: > > This might as well be a single object literal, instead of a bunch of calls to > > push, right? > > +1 > > Also you can make it slightly easier to read by omitting quotes around the > property names. Eg: > var pointerActions = [ > {source: 'touch'}, > {actions: [ > {name: 'pointerDown', x: x, y: y}, > ... > ]}]; Done. |
