elementFromPoint() should consider x and y to be in client (layout viewport) coordinates
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Jul 2017 18:01:15 +0000 (18:01 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Jul 2017 18:01:15 +0000 (18:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=172019

Patch by Ali Juma <ajuma@chromium.org> on 2017-07-11
Reviewed by Simon Fraser.

Source/WebCore:

When visual viewports are enabled, this makes TreeScope::nodeFromPoint consider its
input to be in client coordinates, and clips this input to the layout viewport. This change
affects the behavior of document.elementFromPoint() and document.caretRangeFromPoint.

No new tests. Modified an existing test, and made a previously-failing test pass on ios.

* dom/TreeScope.cpp:
(WebCore::TreeScope::nodeFromPoint):
* page/FrameView.cpp:
(WebCore::FrameView::layoutViewportToAbsoluteRect):
(WebCore::FrameView::layoutViewportToAbsolutePoint):
(WebCore::FrameView::clientToLayoutViewportPoint):
* page/FrameView.h:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::hitTest):

LayoutTests:

* fast/dom/elementFromPoint-scaled-scrolled-expected.txt: Updated.
* fast/dom/elementFromPoint-scaled-scrolled.html: Updated.
* platform/ios/TestExpectations:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219342 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt
LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html
LayoutTests/platform/ios/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/TreeScope.cpp
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/rendering/RenderLayer.cpp

index 012f377..8fcc908 100644 (file)
@@ -1,3 +1,14 @@
+2017-07-11  Ali Juma  <ajuma@chromium.org>
+
+        elementFromPoint() should consider x and y to be in client (layout viewport) coordinates
+        https://bugs.webkit.org/show_bug.cgi?id=172019
+
+        Reviewed by Simon Fraser.
+
+        * fast/dom/elementFromPoint-scaled-scrolled-expected.txt: Updated.
+        * fast/dom/elementFromPoint-scaled-scrolled.html: Updated.
+        * platform/ios/TestExpectations:
+
 2017-07-11  Charlie Turner  <cturner@igalia.com>
 
         [GTK] compositing/video/poster.html passing since r218320
index f81fb63..d929d4d 100644 (file)
@@ -1,3 +1,20 @@
-B1B2B3
-This test is successful if elementFromPoint returns the correct element.
-B3
+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
+
index aa809ed..22ba207 100644 (file)
@@ -1,24 +1,60 @@
 <!DOCTYPE html>
 <html>
 <head>
-<script src="../../../../resources/js-test-pre.js"></script>
+<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="../../resources/js-test-pre.js"></script>
 </head>
 <body onload="runTest();" style="width:2000px;height:2000px;margin:0px;padding:100px">
-<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>
+<button id="b1"></button>
+<button id="b2"></button>
 <script>
 function runTest() {
-    if (window.internals)
+    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.setPageScaleFactor(2, 0, 0);
-    window.scrollTo(100,100);
+    }
+    window.scrollTo(100, 100);
 
-    if (window.testRunner)
-        testRunner.dumpAsText();
+    // The layout viewport hasn't been scrolled.
+    shouldBe("document.elementFromPoint(225, 125)", "b1");
+    shouldBe("document.elementFromPoint(525, 425)", "b2");
 
-    document.getElementById("result").innerText = document.elementFromPoint(125, 25).innerText;
+    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>
+<script src="../../resources/js-test-post.js"></script>
 </body>
 </html>
index 30e2882..42c2a60 100644 (file)
@@ -2719,6 +2719,7 @@ webkit.org/b/164594 http/tests/cache/disk-cache/disk-cache-request-headers.html
 webkit.org/b/163046 js/regress-141098.html [ Pass Failure ]
 
 # Test relies on window.scrollTo
+fast/dom/elementFromPoint-scaled-scrolled.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 ]
@@ -2817,8 +2818,6 @@ webkit.org/b/124933 svg/animations/getCurrentTime-pause-unpause.html [ Pass Fail
 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 ]
index d4b228c..ccb8ce6 100644 (file)
@@ -1,3 +1,26 @@
+2017-07-11  Ali Juma  <ajuma@chromium.org>
+
+        elementFromPoint() should consider x and y to be in client (layout viewport) coordinates
+        https://bugs.webkit.org/show_bug.cgi?id=172019
+
+        Reviewed by Simon Fraser.
+
+        When visual viewports are enabled, this makes TreeScope::nodeFromPoint consider its
+        input to be in client coordinates, and clips this input to the layout viewport. This change
+        affects the behavior of document.elementFromPoint() and document.caretRangeFromPoint.
+
+        No new tests. Modified an existing test, and made a previously-failing test pass on ios.
+
+        * dom/TreeScope.cpp:
+        (WebCore::TreeScope::nodeFromPoint):
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layoutViewportToAbsoluteRect):
+        (WebCore::FrameView::layoutViewportToAbsolutePoint):
+        (WebCore::FrameView::clientToLayoutViewportPoint):
+        * page/FrameView.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::hitTest):
+
 2017-07-11  Timothy Hatcher  <timothy@hatcher.name>
 
         Broken build when !USE(REQUEST_ANIMATION_FRAME_DISPLAY_MONITOR)
index 3a08513..280dd20 100644 (file)
@@ -44,6 +44,7 @@
 #include "PointerLockController.h"
 #include "RenderView.h"
 #include "RuntimeEnabledFeatures.h"
+#include "Settings.h"
 #include "ShadowRoot.h"
 #include <wtf/text/CString.h>
 
@@ -301,22 +302,32 @@ Node* TreeScope::nodeFromPoint(const LayoutPoint& clientPoint, LayoutPoint* loca
     if (!frame || !view)
         return nullptr;
 
-    float scaleFactor = frame->pageZoomFactor() * frame->frameScaleFactor();
+    LayoutPoint absolutePoint;
+    if (frame->settings().visualViewportEnabled()) {
+        documentScope().updateLayout();
+        FloatPoint layoutViewportPoint = view->clientToLayoutViewportPoint(clientPoint);
+        FloatRect layoutViewportBounds({ }, view->layoutViewportRect().size());
+        if (!layoutViewportBounds.contains(layoutViewportPoint))
+            return nullptr;
+        absolutePoint = LayoutPoint(view->layoutViewportToAbsolutePoint(layoutViewportPoint));
+    } else {
+        float scaleFactor = frame->pageZoomFactor() * frame->frameScaleFactor();
 
-    LayoutPoint contentsPoint = clientPoint;
-    contentsPoint.scale(scaleFactor);
-    contentsPoint.moveBy(view->contentsScrollPosition());
+        absolutePoint = clientPoint;
+        absolutePoint.scale(scaleFactor);
+        absolutePoint.moveBy(view->contentsScrollPosition());
 
-    LayoutRect visibleRect;
+        LayoutRect visibleRect;
 #if PLATFORM(IOS)
-    visibleRect = view->unobscuredContentRect();
+        visibleRect = view->unobscuredContentRect();
 #else
-    visibleRect = view->visibleContentRect();
+        visibleRect = view->visibleContentRect();
 #endif
-    if (!visibleRect.contains(contentsPoint))
-        return nullptr;
+        if (!visibleRect.contains(absolutePoint))
+            return nullptr;
+    }
 
-    HitTestResult result(contentsPoint);
+    HitTestResult result(absolutePoint);
     documentScope().renderView()->hitTest(HitTestRequest(), result);
 
     if (localPoint)
index c2eda25..00d90d1 100644 (file)
@@ -4936,6 +4936,27 @@ FloatPoint FrameView::documentToClientPoint(FloatPoint p) const
     return p;
 }
 
+FloatRect FrameView::layoutViewportToAbsoluteRect(FloatRect rect) const
+{
+    ASSERT(frame().settings().visualViewportEnabled());
+    rect.moveBy(layoutViewportRect().location());
+    rect.scale(frame().frameScaleFactor());
+    return rect;
+}
+
+FloatPoint FrameView::layoutViewportToAbsolutePoint(FloatPoint p) const
+{
+    ASSERT(frame().settings().visualViewportEnabled());
+    p.moveBy(layoutViewportRect().location());
+    return p.scaled(frame().frameScaleFactor());
+}
+
+FloatPoint FrameView::clientToLayoutViewportPoint(FloatPoint p) const
+{
+    ASSERT(frame().settings().visualViewportEnabled());
+    return p.scaled(frame().pageZoomFactor());
+}
+
 void FrameView::setTracksRepaints(bool trackRepaints)
 {
     if (trackRepaints == m_isTrackingRepaints)
index ac0e707..c9fe16f 100644 (file)
@@ -454,7 +454,10 @@ public:
     // "Client"
     //    Relative to the visible part of the document (or, more strictly, the layout viewport rect), and with the same scaling
     //    as Document coordinates, i.e. matching CSS pixels. Affected by scroll origin.
-    //    
+    //
+    // "LayoutViewport"
+    //    Similar to client coordinates, but affected by page zoom (but not page scale).
+    //
 
     // Methods to convert points and rects between the coordinate space of the renderer, and this view.
     WEBCORE_EXPORT IntRect convertFromRendererToContainingView(const RenderElement*, const IntRect&) const;
@@ -478,6 +481,12 @@ public:
     FloatRect documentToClientRect(FloatRect) const;
     FloatPoint documentToClientPoint(FloatPoint) const;
 
+    FloatRect layoutViewportToAbsoluteRect(FloatRect) const;
+    FloatPoint layoutViewportToAbsolutePoint(FloatPoint) const;
+
+    // Unlike client coordinates, layout viewport coordinates are affected by page zoom.
+    FloatPoint clientToLayoutViewportPoint(FloatPoint) const;
+
     bool isFrameViewScrollCorner(const RenderScrollbarPart& scrollCorner) const { return m_scrollCorner == &scrollCorner; }
 
     // isScrollable() takes an optional Scrollability parameter that allows the caller to define what they mean by 'scrollable.'
index 96e48c2..8cea9f4 100644 (file)
@@ -4924,9 +4924,17 @@ bool RenderLayer::hitTest(const HitTestRequest& request, const HitTestLocation&
     
     updateLayerListsIfNeeded();
 
-    LayoutRect hitTestArea = isOutOfFlowRenderFlowThread() ? downcast<RenderFlowThread>(renderer()).visualOverflowRect() : renderer().view().documentRect();
-    if (!request.ignoreClipping())
-        hitTestArea.intersect(renderer().view().frameView().visibleContentRect(LegacyIOSDocumentVisibleRect));
+    ASSERT(!isRenderFlowThread());
+    LayoutRect hitTestArea = renderer().view().documentRect();
+    if (!request.ignoreClipping()) {
+        if (renderer().settings().visualViewportEnabled()) {
+            auto& frameView = renderer().view().frameView();
+            LayoutRect layoutViewportBounds({ }, frameView.layoutViewportRect().size());
+            LayoutRect absoluteLayoutViewportRect = LayoutRect(frameView.layoutViewportToAbsoluteRect(layoutViewportBounds));
+            hitTestArea.intersect(absoluteLayoutViewportRect);
+        } else
+            hitTestArea.intersect(renderer().view().frameView().visibleContentRect(LegacyIOSDocumentVisibleRect));
+    }
 
     RenderLayer* insideLayer = hitTestLayer(this, nullptr, request, result, hitTestArea, hitTestLocation, false);
     if (!insideLayer) {