REGRESSION (r219342): Scaled HTML widget is not responding to a clicks outside the...

https://bugs.webkit.org/show_bug.cgi?id=182394
<rdar://problem/34840816>

Reviewed by Simon Fraser.

Source/WebCore:

If a scale < 1 is applied to the page, then the visual viewport will be bigger
than the layout viewport. Our hit testing code would then ignore any hits
that were outside the layout viewport.

The fix is to only apply a hit testing clip if the page is scaling up, not down.

Update the existing fast/dom/elementFromPoint-scaled-scrolled.html test.

* page/FrameView.cpp:
(WebCore::FrameView::layoutViewportToAbsoluteRect const): Deleted. This helper is
no longer used, and it would have probably been more confusing to have it accept
a flag to ignore the scale if it is less than 1.
* page/FrameView.h:
* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::hitTest): No need to take the layout rect, remove the origin,
and pass it to a helper that added the origin back. The only thing the helper was
doing for us was applying a scale factor, which we only want to do if it was
scaling up.

LayoutTests:

Add a test for a scaled down page.

* fast/dom/elementFromPoint-scaled-scrolled-expected.txt:
* fast/dom/elementFromPoint-scaled-scrolled.html:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 7c27ba1..c1e1ac3 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2018-02-01  Dean Jackson  <dino@apple.com>
+
+        REGRESSION (r219342): Scaled HTML widget is not responding to a clicks outside the body
+        https://bugs.webkit.org/show_bug.cgi?id=182394
+        <rdar://problem/34840816>
+
+        Reviewed by Simon Fraser.
+
+        Add a test for a scaled down page.
+
+        * fast/dom/elementFromPoint-scaled-scrolled-expected.txt:
+        * fast/dom/elementFromPoint-scaled-scrolled.html:
+
 2018-02-01  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r227958 and r227972.

diff --git a/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt b/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt
index d929d4d..83a7a15 100644 (file)
--- a/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt
+++ b/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt
@@ -14,6 +14,7 @@ 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

diff --git a/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html b/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html
index 22ba207..66461ec 100644 (file)
--- a/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html
+++ b/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html
@@ -52,6 +52,14 @@ function runTest() {
     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();
 }
 </script>

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index e07c5df..e7981ea 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2018-02-01  Dean Jackson  <dino@apple.com>
+
+        REGRESSION (r219342): Scaled HTML widget is not responding to a clicks outside the body
+        https://bugs.webkit.org/show_bug.cgi?id=182394
+        <rdar://problem/34840816>
+
+        Reviewed by Simon Fraser.
+
+        If a scale < 1 is applied to the page, then the visual viewport will be bigger
+        than the layout viewport. Our hit testing code would then ignore any hits
+        that were outside the layout viewport.
+
+        The fix is to only apply a hit testing clip if the page is scaling up, not down.
+
+        Update the existing fast/dom/elementFromPoint-scaled-scrolled.html test.
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layoutViewportToAbsoluteRect const): Deleted. This helper is
+        no longer used, and it would have probably been more confusing to have it accept
+        a flag to ignore the scale if it is less than 1.
+        * page/FrameView.h:
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::hitTest): No need to take the layout rect, remove the origin,
+        and pass it to a helper that added the origin back. The only thing the helper was
+        doing for us was applying a scale factor, which we only want to do if it was
+        scaling up.
+
 2018-02-01  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Structured cloning a Symbol should throw

diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp
index 38986b2..cbeb3df 100644 (file)
--- a/Source/WebCore/page/FrameView.cpp
+++ b/Source/WebCore/page/FrameView.cpp
@@ -4608,14 +4608,6 @@ FloatPoint FrameView::clientToDocumentPoint(FloatPoint point) const
     return point;
 }
 
-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());

diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h
index c7a5785..f6ffc99 100644 (file)
--- a/Source/WebCore/page/FrameView.h
+++ b/Source/WebCore/page/FrameView.h
@@ -475,7 +475,6 @@ public:
     WEBCORE_EXPORT FloatRect clientToDocumentRect(FloatRect) const;
     WEBCORE_EXPORT FloatPoint clientToDocumentPoint(FloatPoint) const;
 
-    FloatRect layoutViewportToAbsoluteRect(FloatRect) const;
     FloatPoint layoutViewportToAbsolutePoint(FloatPoint) const;
 
     // Unlike client coordinates, layout viewport coordinates are affected by page zoom.

diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
index 193e4f5..29adf99 100644 (file)
--- a/Source/WebCore/rendering/RenderLayer.cpp
+++ b/Source/WebCore/rendering/RenderLayer.cpp
@@ -4933,8 +4933,10 @@ bool RenderLayer::hitTest(const HitTestRequest& request, const HitTestLocation&
     if (!request.ignoreClipping()) {
         if (renderer().settings().visualViewportEnabled()) {
             auto& frameView = renderer().view().frameView();
-            LayoutRect layoutViewportBounds({ }, frameView.layoutViewportRect().size());
-            LayoutRect absoluteLayoutViewportRect = LayoutRect(frameView.layoutViewportToAbsoluteRect(layoutViewportBounds));
+            LayoutRect absoluteLayoutViewportRect = frameView.layoutViewportRect();
+            auto scaleFactor = frameView.frame().frameScaleFactor();
+            if (scaleFactor > 1)
+                absoluteLayoutViewportRect.scale(scaleFactor);
             hitTestArea.intersect(absoluteLayoutViewportRect);
         } else
             hitTestArea.intersect(renderer().view().frameView().visibleContentRect(LegacyIOSDocumentVisibleRect));