[RLS] iframe hit test rects do not match pre-RLS |
||||
Issue descriptionfast/events/touch/compositor-touch-hit-rects-iframes.html is failing for two reasons: 1) An additional "scrolling" line in the expectation file. This is fine. 2) The iframe hit test rect looks wrong: no-rls: iframe-doc: #document (10, 284, 385, 58) rls: iframe-doc: #document (10, 284, 400, 25)
,
Mar 1 2018
I have this down to a minimized testcase:
----------------------------------8<----------------------------------
<!doctype html>
<iframe id="iframe" style="width: 300px; height: 300px;" srcdoc="
<!doctype html>
<div id='tests' style='position: absolute; font-size: 100px;'>
a<br>b<br>c<br>d<br>e<br>f<br>g<br>h<br>i
</div>
<script>
function handler() { };
document.addEventListener('touchstart', handler, {passive: false});
</script>"></iframe>
<script>
window.addEventListener('load', function(evt) {
window.internals.forceCompositingUpdate(document);
var rects = window.internals.touchEventTargetLayerRects(document);
for (var i = 0; i < rects.length; ++i) {
console.log('[' + i + ']: ');
console.log(rects[i].layerAssociatedNode);
var rect = rects[i].layerRelativeRect;
console.log(rect.x + ', ' + rect.y + ', ' + rect.width + ', ' + rect.height);
}
});
</script>
----------------------------------8<----------------------------------
(with RLS)
run-webkit-tests touchrect.html
[object HTMLDocument]
10, 10, 300, 300
(without RLS)
run-webkit-tests --additional-driver-flag=--disable-blink-features=RootLayerScrolling touchrect.html
[object HTMLDocument]
10, 10, 285, 1043
This testcase has a 300x300 iframe with scrolling contents of size 285x1043. With RLS, the touch event rect is clipped to 300x300. Without RLS, the touch event rect is not clipped to the iframe. Which is correct?
There is also a minor issue of whether or not to include the scrollbar area in the touch event rect. With RLS, the scrollbar is inside the touch event rect. Without RLS, the scrollbar is not inside the touch event rect.
,
Mar 1 2018
This difference is visible using the "Scroll performance issues" checkbox in devtools under the rendering tab. I think the RLS result where the touch event area is clipped to the iframe seems more correct. I made a similar testcase with overflow: scroll and a touch event rect covering the scrolled contents. In this case, both RLS and !RLS have a non-clipped touch event area.
,
Mar 2 2018
I can only speculate here since I don't have first-hand knowledge, but I _suspect_ unclipped may be fine. IIRC the rects are relative to the scrolling contents layer. Hit testing in CC should proceed from the root down. A clipping layer would remove a hit test before we reach down to the contents layer and check the touch rects. I think scrollbar should be included in the rect. These rects are also used for touch-action. Intuitively, if I mark an iframe as 'touch-action: none', I would expect it to apply to the scrollbars as well. That said, we may wish to preserve the existing behavior. +dtapuska@ who has more context here.
,
Mar 2 2018
That would mean we have an existing bug, both for RLS and !RLS, for overflow: scroll where our touch rects are too large and do not cover the scroll bar. For example: look at the inspector touch rects in this overflow: scroll testcase: http://output.jsbin.com/tatabec/quiet For completeness, here's a testcase for the iframe case: http://output.jsbin.com/qenajof/quiet
,
Mar 2 2018
I'm not sure it's a bug. Because you're putting the handler on the actual content, and scrollbars belong to the parent, not including scrollbars looks correct to me. DevTools doesn't clip nor apply the scroll translation to the touch rect but I suspect that's just a DevTools rendering error, rather than a bug in the actual rects. There are differences between !RLS and RLS in the iframe case. The scrollbar is now included, which I would actually categorize as fixing a bug when RLS is turned on. The fact that the document is now clipped is also correct because the document layer itself is the size of the iframe. This shouldn't have a user-visible effect though. A better example might be a handler on an element of the content: http://output.jsbin.com/pexecizadu/1. The rect is correct as you scroll, both with and without RLS.
,
Mar 2 2018
With that, I think the change in height in the layout test is correct and should just be rebaselined. We're applying the touch rect to the scroll clipping layer, rather than the scrolling contents layer which is really what the RLS change is about (that #document is now a scroller, rather than just a long contents layer) and the two should be equivalent (because event pre-RLS, only the visible part of the content could be hit tested anyway).
,
Mar 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9979302f68a06ff77f588b90338ae61651a84931 commit 9979302f68a06ff77f588b90338ae61651a84931 Author: Philip Rogers <pdr@chromium.org> Date: Fri Mar 02 20:14:05 2018 [RLS] Rebaseline fast/.../compositor-touch-hit-rects-iframes.html This patch rebaselines the following test: fast/events/touch/compositor-touch-hit-rects-iframes.html Now that root layer scrolling is enabled, there is an additional "scrolling" string in the output, the iframe-doc size includes scrollbars, and the iframe-doc size is clipped. A non-RLS baseline has been added as well. This test is marked as being flaky for https://crbug.com/522648 but that should be revisited after this patch lands because the test output no longer is flaky. Bug: 817442 Change-Id: I1653b3578b4b67d7d3d1f838935507d71778c133 Reviewed-on: https://chromium-review.googlesource.com/941418 Reviewed-by: David Bokan <bokan@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#540601} [modify] https://crrev.com/9979302f68a06ff77f588b90338ae61651a84931/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/9979302f68a06ff77f588b90338ae61651a84931/third_party/WebKit/LayoutTests/fast/events/touch/compositor-touch-hit-rects-iframes-expected.txt [add] https://crrev.com/9979302f68a06ff77f588b90338ae61651a84931/third_party/WebKit/LayoutTests/flag-specific/disable-blink-features=RootLayerScrolling/fast/events/touch/compositor-touch-hit-rects-iframes-expected.txt
,
Mar 2 2018
|
||||
►
Sign in to add a comment |
||||
Comment 1 by szager@chromium.org
, Mar 1 2018Status: Assigned (was: Available)