Setting TouchRegion to whole main frame's view when there is any window event targets for touch seems incorrect |
||
Issue descriptionWhen accumulating touch regions, we currently add a region for the whole page if there is a handler on (local) DOMWindow. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/page/scrolling/ScrollingCoordinator.cpp?rcl=27e6b04c54ae2f7d868e6f6c7aa0c5d94d8b4498&l=1095 This is probably incorrect as the "window" is no necessarily that of the main frame in the page. Example: ---------------------- | | | -----div A---- | | | passive | | | | | | | -------------- | | | | --X------------- | | | non-passive | | | ---------------- | ---------------------- Assume both "div A" and "region X" have touch handlers where the one for "X" is non-passive. If a touch gesture is applied inside "div A it is expected that the event is dispatched as non-blocking. If "X" is a <div> any gesture inside "div A" is dispatched as non-blocking (InputHandlerProxy::EventDisposition == DID_HANDLE_NON_BLOCKING and event is not cancelable). This is correct. If "X" is an <iframe> with a touch handler on its window any gesture inside "div A" is dispatched as blocking (InputHandlerProxy::EventDisposition = DID_NOT_HANDLE and event is cancelable). The difference in terms of touch regions is that in the former case we only get a region for element "X" but in the latter case the whole page is added with whitelisted touch action of 0 (kTouchActionNone).
,
May 3 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6e8a9c51a036ab9ea128c912f756fafa9f0b4dc4 commit 6e8a9c51a036ab9ea128c912f756fafa9f0b4dc4 Author: Ehsan Karamad <ekaramad@chromium.org> Date: Thu May 03 04:37:02 2018 Fix for finding touch regions of Window targets Currently if a DomWindow in the page has a touch handler, the whole region for local frame root is added as touch region. This is incorrect as only the window for corresponding to the document/frame should be considered -- which is the same treatment that is given to |document|. Bug: 830626 Change-Id: Ieebd4116e9753f800fe2534cb0c89da812d22c56 Reviewed-on: https://chromium-review.googlesource.com/1005368 Reviewed-by: Robert Flack <flackr@chromium.org> Commit-Queue: Ehsan Karamad <ekaramad@chromium.org> Cr-Commit-Position: refs/heads/master@{#555665} [modify] https://crrev.com/6e8a9c51a036ab9ea128c912f756fafa9f0b4dc4/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc [modify] https://crrev.com/6e8a9c51a036ab9ea128c912f756fafa9f0b4dc4/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc
,
May 3 2018
Based on comment #2 the bug in the description should be resolved. I am not sure if we want to do more for the case in comment #1.
,
May 11 2018
Closing this bug since the original issue is resolved now. |
||
►
Sign in to add a comment |
||
Comment 1 by ekaramad@chromium.org
, Apr 9 2018While looking at this I also noticed that if we have a setup with N-nested documents: document #1 | ->document #2: | ... | -> document #N: | -> <div></div> with all documents index 2 and above having touch handlers then we keep a region for each of them. This also does not look OK.