Title: [228714] trunk
Revision
228714
Author
timothy_hor...@apple.com
Date
2018-02-19 15:30:27 -0800 (Mon, 19 Feb 2018)

Log Message

REGRESSION (r219342): Touch event coordinates and elementFromPoint coordinates differ
https://bugs.webkit.org/show_bug.cgi?id=182910
<rdar://problem/37533950>

Reviewed by Simon Fraser.

Source/WebCore:

We reverted other changes to the definition of client coordinates
in r219829 due to compatibility concerns. However, we failed to revert
r219342 on trunk, leaving elementFromPoint() using coordinates relative
to the layout viewport.

Add a currently off-by-default setting to switch on layout-viewport-relative
client coordinates and guard the elementFromPoint changes behind it.
A future patch should roll r219829 back in also behind this setting, so
that everything remains consistent regardless of which coordinate space we choose.

* dom/TreeScope.cpp:
(WebCore::absolutePointIfNotClipped):
* page/Settings.yaml:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::hitTest):

LayoutTests:

* fast/dom/elementFromPoint-scaled-scrolled.html:
Revert changes to this test made in r219342.

* fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html:
* fast/dom/elementFromPoint-scaled-scrolled-layout-viewport-expected.txt:
Add a test that is equivalent to elementFromPoint-scaled-scrolled.html after r219342,
which turns on the new setting. This test is disabled on iOS (like it was
in r219342) because it needs window.scrollTo.

* platform/ios-wk2/fast/dom/elementFromPoint-relative-to-viewport-expected.txt:
This now passes.

* platform/ios/TestExpectations:
Re-mark-failing a test that was un-marked-failing by r219342.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (228713 => 228714)


--- trunk/LayoutTests/ChangeLog	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/LayoutTests/ChangeLog	2018-02-19 23:30:27 UTC (rev 228714)
@@ -1,3 +1,26 @@
+2018-02-19  Tim Horton  <timothy_hor...@apple.com>
+
+        REGRESSION (r219342): Touch event coordinates and elementFromPoint coordinates differ
+        https://bugs.webkit.org/show_bug.cgi?id=182910
+        <rdar://problem/37533950>
+
+        Reviewed by Simon Fraser.
+
+        * fast/dom/elementFromPoint-scaled-scrolled.html:
+        Revert changes to this test made in r219342.
+
+        * fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html:
+        * fast/dom/elementFromPoint-scaled-scrolled-layout-viewport-expected.txt:
+        Add a test that is equivalent to elementFromPoint-scaled-scrolled.html after r219342,
+        which turns on the new setting. This test is disabled on iOS (like it was
+        in r219342) because it needs window.scrollTo.
+
+        * platform/ios-wk2/fast/dom/elementFromPoint-relative-to-viewport-expected.txt:
+        This now passes.
+
+        * platform/ios/TestExpectations:
+        Re-mark-failing a test that was un-marked-failing by r219342.
+
 2018-02-19  Daniel Bates  <daba...@apple.com>
 
         Do not block authentication challenge to navigated resources

Modified: trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt (228713 => 228714)


--- trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt	2018-02-19 23:30:27 UTC (rev 228714)
@@ -1,21 +1,3 @@
-This test applies page scale and scrolls the page, and checks that elementFromPoint returns the correct element.
-
-On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
-
-
-PASS successfullyParsed is true
-
-TEST COMPLETE
-PASS document.elementFromPoint(225, 125) is b1
-PASS document.elementFromPoint(525, 425) is b2
-PASS document.elementFromPoint(225, 125) is b1
-PASS document.elementFromPoint(525, 425) is b2
-PASS document.elementFromPoint(115, 15) is b1
-PASS document.elementFromPoint(415, 315) is b2
-PASS document.elementFromPoint(-85, 15) is null
-PASS document.elementFromPoint(215, 315) is b2
-PASS document.elementFromPoint(525, 425) is b2
-PASS successfullyParsed is true
-
-TEST COMPLETE
-
+B1B2B3
+This test is successful if elementFromPoint returns the correct element.
+B3

Copied: trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-layout-viewport-expected.txt (from rev 228713, trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt) (0 => 228714)


--- trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-layout-viewport-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-layout-viewport-expected.txt	2018-02-19 23:30:27 UTC (rev 228714)
@@ -0,0 +1,20 @@
+This test applies page scale and scrolls the page, and checks that elementFromPoint returns the correct element.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS document.elementFromPoint(225, 125) is b1
+PASS document.elementFromPoint(525, 425) is b2
+PASS document.elementFromPoint(225, 125) is b1
+PASS document.elementFromPoint(525, 425) is b2
+PASS document.elementFromPoint(115, 15) is b1
+PASS document.elementFromPoint(415, 315) is b2
+PASS document.elementFromPoint(-85, 15) is null
+PASS document.elementFromPoint(215, 315) is b2
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Copied: trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html (from rev 228713, trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html) (0 => 228714)


--- trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html	                        (rev 0)
+++ trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html	2018-02-19 23:30:27 UTC (rev 228714)
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+    body {
+        width: 2000px;
+        height: 2000px;
+    }
+    button {
+        position: absolute;
+        width: 50px;
+        height: 50px;
+    }
+    #b1 {
+        left: 200px;
+        top: 100px;
+    }
+    #b2 {
+        left: 500px;
+        top: 400px;
+    }
+</style>
+<script src=""
+</head>
+<body _onload_="runTest();" style="width:2000px;height:2000px;margin:0px;padding:100px">
+<button id="b1"></button>
+<button id="b2"></button>
+<script>
+function runTest() {
+    description("This test applies page scale and scrolls the page, and checks that elementFromPoint returns the correct element.");
+    if (window.internals) {
+        window.internals.settings.setVisualViewportEnabled(true);
+        window.internals.settings.setClientCoordinatesRelativeToLayoutViewport(true);
+        window.internals.setPageScaleFactor(2, 0, 0);
+    }
+    window.scrollTo(100, 100);
+
+    // The layout viewport hasn't been scrolled.
+    shouldBe("document.elementFromPoint(225, 125)", "b1");
+    shouldBe("document.elementFromPoint(525, 425)", "b2");
+
+    window.scrollTo(200, 200);
+
+    // b1 is now offscreen, but still within the layout viewport.
+    shouldBe("document.elementFromPoint(225, 125)", "b1");
+    shouldBe("document.elementFromPoint(525, 425)", "b2");
+
+    window.scrollTo(500, 400);
+    shouldBe("document.elementFromPoint(115, 15)", "b1");
+    shouldBe("document.elementFromPoint(415, 315)", "b2");
+
+    window.scrollTo(700, 400);
+    shouldBeNull("document.elementFromPoint(-85, 15)");
+    shouldBe("document.elementFromPoint(215, 315)", "b2");
+
+    finishJSTest();
+}
+</script>
+<script src=""
+</body>
+</html>

Modified: trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html (228713 => 228714)


--- trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html	2018-02-19 23:30:27 UTC (rev 228714)
@@ -1,68 +1,24 @@
 <!DOCTYPE html>
 <html>
 <head>
-<style>
-    body {
-        width: 2000px;
-        height: 2000px;
-    }
-    button {
-        position: absolute;
-        width: 50px;
-        height: 50px;
-    }
-    #b1 {
-        left: 200px;
-        top: 100px;
-    }
-    #b2 {
-        left: 500px;
-        top: 400px;
-    }
-</style>
-<script src=""
+<script src=""
 </head>
 <body _onload_="runTest();" style="width:2000px;height:2000px;margin:0px;padding:100px">
-<button id="b1"></button>
-<button id="b2"></button>
+<button style="width:50px;height:50px;">B1</button><button style="width:50px;height:50px;">B2</button><button style="width:50px;height:50px;">B3</button>
+<div>This test is successful if elementFromPoint returns the correct element.</div>
+<div id="result"></div>
 <script>
 function runTest() {
-    description("This test applies page scale and scrolls the page, and checks that elementFromPoint returns the correct element.");
-    if (window.internals) {
-        window.internals.settings.setVisualViewportEnabled(true);
+    if (window.internals)
         window.internals.setPageScaleFactor(2, 0, 0);
-    }
-    window.scrollTo(100, 100);
+    window.scrollTo(100,100);
 
-    // The layout viewport hasn't been scrolled.
-    shouldBe("document.elementFromPoint(225, 125)", "b1");
-    shouldBe("document.elementFromPoint(525, 425)", "b2");
+    if (window.testRunner)
+        testRunner.dumpAsText();
 
-    window.scrollTo(200, 200);
-
-    // b1 is now offscreen, but still within the layout viewport.
-    shouldBe("document.elementFromPoint(225, 125)", "b1");
-    shouldBe("document.elementFromPoint(525, 425)", "b2");
-
-    window.scrollTo(500, 400);
-    shouldBe("document.elementFromPoint(115, 15)", "b1");
-    shouldBe("document.elementFromPoint(415, 315)", "b2");
-
-    window.scrollTo(700, 400);
-    shouldBeNull("document.elementFromPoint(-85, 15)");
-    shouldBe("document.elementFromPoint(215, 315)", "b2");
-
-    window.scrollTo(0, 0);
-    if (window.internals)
-        window.internals.setPageScaleFactor(0.5, 0, 0);
-    // b2 is now technically outside the 800x600 scaled viewport rect,
-    // but should still be found.
-
-    shouldBe("document.elementFromPoint(525, 425)", "b2");
-
-    finishJSTest();
+    document.getElementById("result").innerText = document.elementFromPoint(125, 25).innerText;
 }
 </script>
-<script src=""
+</script>
 </body>
-</html>
+</html>
\ No newline at end of file

Modified: trunk/LayoutTests/platform/ios/TestExpectations (228713 => 228714)


--- trunk/LayoutTests/platform/ios/TestExpectations	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/LayoutTests/platform/ios/TestExpectations	2018-02-19 23:30:27 UTC (rev 228714)
@@ -2857,7 +2857,7 @@
 webkit.org/b/163046 js/regress-141098.html [ Pass Failure ]
 
 # Test relies on window.scrollTo
-fast/dom/elementFromPoint-scaled-scrolled.html [ Skip ]
+fast/dom/elementFromPoint-scaled-scrolled-layout-viewport.html [ Skip ]
 fast/zooming/client-rect-in-fixed-zoomed.html [ Skip ]
 fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html [ Skip ]
 fast/visual-viewport/zoomed-scroll-into-view-fixed.html [ Skip ]
@@ -2953,6 +2953,8 @@
 webkit.org/b/171760 imported/w3c/i18n/bidi/bidi-plaintext-011.html [ ImageOnlyFailure ]
 webkit.org/b/171760 imported/w3c/i18n/bidi/block-plaintext-005.html [ ImageOnlyFailure ]
 
+webkit.org/b/172019 imported/w3c/web-platform-tests/cssom-view/elementFromPoint.html [ Failure ]
+
 webkit.org/b/172547 http/tests/xmlhttprequest/xmlhttprequest-50ms-download-dispatch.html [ Failure ]
 
 webkit.org/b/172610 media/ios/autoplay-only-in-main-document.html [ Skip ]

Modified: trunk/LayoutTests/platform/ios-wk2/fast/dom/elementFromPoint-relative-to-viewport-expected.txt (228713 => 228714)


--- trunk/LayoutTests/platform/ios-wk2/fast/dom/elementFromPoint-relative-to-viewport-expected.txt	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/LayoutTests/platform/ios-wk2/fast/dom/elementFromPoint-relative-to-viewport-expected.txt	2018-02-19 23:30:27 UTC (rev 228714)
@@ -20,8 +20,8 @@
 Test Initial
 PASS unscrolledBoxInitial is '0'
 PASS scrolledDownBoxInitial is '5'
-FAIL scrolledRightBoxInitial should be 3. Was 2.
-FAIL scrolledDownAndRightBoxInitial should be 8. Was 7.
+PASS scrolledRightBoxInitial is '3'
+PASS scrolledDownAndRightBoxInitial is '8'
 PASS successfullyParsed is true
 
 TEST COMPLETE

Modified: trunk/Source/WebCore/ChangeLog (228713 => 228714)


--- trunk/Source/WebCore/ChangeLog	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/Source/WebCore/ChangeLog	2018-02-19 23:30:27 UTC (rev 228714)
@@ -1,3 +1,27 @@
+2018-02-19  Tim Horton  <timothy_hor...@apple.com>
+
+        REGRESSION (r219342): Touch event coordinates and elementFromPoint coordinates differ
+        https://bugs.webkit.org/show_bug.cgi?id=182910
+        <rdar://problem/37533950>
+
+        Reviewed by Simon Fraser.
+
+        We reverted other changes to the definition of client coordinates
+        in r219829 due to compatibility concerns. However, we failed to revert
+        r219342 on trunk, leaving elementFromPoint() using coordinates relative
+        to the layout viewport.
+
+        Add a currently off-by-default setting to switch on layout-viewport-relative
+        client coordinates and guard the elementFromPoint changes behind it.
+        A future patch should roll r219829 back in also behind this setting, so
+        that everything remains consistent regardless of which coordinate space we choose.
+
+        * dom/TreeScope.cpp:
+        (WebCore::absolutePointIfNotClipped):
+        * page/Settings.yaml:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::hitTest):
+
 2018-02-19  Eric Carlson  <eric.carl...@apple.com>
 
         [Extra zoom mode] Don't allow PiP media playback

Modified: trunk/Source/WebCore/dom/TreeScope.cpp (228713 => 228714)


--- trunk/Source/WebCore/dom/TreeScope.cpp	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/Source/WebCore/dom/TreeScope.cpp	2018-02-19 23:30:27 UTC (rev 228714)
@@ -299,7 +299,8 @@
     if (!document.frame() || !document.view())
         return std::nullopt;
 
-    if (document.frame()->settings().visualViewportEnabled()) {
+    const auto& settings = document.frame()->settings();
+    if (settings.visualViewportEnabled() && settings.clientCoordinatesRelativeToLayoutViewport()) {
         document.updateLayout();
         if (!document.view() || !document.hasLivingRenderTree())
             return std::nullopt;

Modified: trunk/Source/WebCore/page/Settings.yaml (228713 => 228714)


--- trunk/Source/WebCore/page/Settings.yaml	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/Source/WebCore/page/Settings.yaml	2018-02-19 23:30:27 UTC (rev 228714)
@@ -731,3 +731,7 @@
 
 resourceLoadStatisticsDebugMode:
   initial: false
+
+clientCoordinatesRelativeToLayoutViewport:
+  initial: false
+  onChange: setNeedsRecalcStyleInAllFrames

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (228713 => 228714)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2018-02-19 22:50:35 UTC (rev 228713)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2018-02-19 23:30:27 UTC (rev 228714)
@@ -4931,7 +4931,8 @@
     ASSERT(!isRenderFragmentedFlow());
     LayoutRect hitTestArea = renderer().view().documentRect();
     if (!request.ignoreClipping()) {
-        if (renderer().settings().visualViewportEnabled()) {
+        const auto& settings = renderer().settings();
+        if (settings.visualViewportEnabled() && settings.clientCoordinatesRelativeToLayoutViewport()) {
             auto& frameView = renderer().view().frameView();
             LayoutRect absoluteLayoutViewportRect = frameView.layoutViewportRect();
             auto scaleFactor = frameView.frame().frameScaleFactor();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to