Changeset 227974 in webkit

Timestamp:

Feb 1, 2018 10:59:38 AM (6 years ago)

Author:

dino@apple.com

Message:

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.

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt (modified) (1 diff)
• LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/FrameView.cpp (modified) (1 diff)
• Source/WebCore/page/FrameView.h (modified) (1 diff)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled-expected.txt

@@ -15 +15 @@
 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
 

trunk/LayoutTests/fast/dom/elementFromPoint-scaled-scrolled.html

@@ -53 +53 @@
     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();
 }

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/page/FrameView.cpp

@@ -4609 +4609 @@
 }
 
-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
 {

trunk/Source/WebCore/page/FrameView.h

@@ -476 +476 @@
     WEBCORE_EXPORT FloatPoint clientToDocumentPoint(FloatPoint) const;
 
-    FloatRect layoutViewportToAbsoluteRect(FloatRect) const;
     FloatPoint layoutViewportToAbsolutePoint(FloatPoint) const;
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -4934 +4934 @@
         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