getBoundingClientRect returns wrong value for combination of page zoom and scroll
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Jun 2017 02:09:09 +0000 (02:09 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 30 Jun 2017 02:09:09 +0000 (02:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173841
rdar://problem/32983841

Reviewed by Dean Jackson.

Source/WebCore:

The layout viewport returned by FrameView::layoutViewportRect() is affected by page (Command-+) zooming,
since it's computed using scroll positions, so when we use its origin to convert into client coordinates
(which are zoom-agnostic), we need to account for page zoom, so fix FrameView::documentToClientOffset()
to do this.

Callers of documentToClientOffset() were checked, revealing that event client coordinates were also
wrong with page zoom and are fixed in the same way. It was found that SimulatedClick was using an
entirely wrong rect to compute its location: Element::clientRect() is NOT in client coordinates,
so change this code to use getBoundingClientRect() instead.

Minor refactoring in MouseRelatedEvent to make getting to the FrameView cleaner.

Some geometry types enhanced to have non-mutating scale functions.

Tests: fast/events/simulated-click-zoomed.html
       fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed.html

* dom/MouseRelatedEvent.cpp:
(WebCore::MouseRelatedEvent::init):
(WebCore::MouseRelatedEvent::initCoordinates):
(WebCore::MouseRelatedEvent::frameView):
(WebCore::MouseRelatedEvent::documentToAbsoluteScaleFactor):
(WebCore::MouseRelatedEvent::computePageLocation):
(WebCore::MouseRelatedEvent::computeRelativePosition):
(WebCore::pageZoomFactor): Deleted.
(WebCore::frameScaleFactor): Deleted.
* dom/MouseRelatedEvent.h:
(WebCore::MouseRelatedEvent::absoluteLocation):
(WebCore::MouseRelatedEvent::setAbsoluteLocation): Deleted.
* dom/SimulatedClick.cpp:
* page/FrameView.cpp:
(WebCore::FrameView::layoutViewportRect): baseLayoutViewportSize() is the same as the old code.
(WebCore::FrameView::documentToAbsoluteScaleFactor):
(WebCore::FrameView::absoluteToDocumentScaleFactor):
(WebCore::FrameView::absoluteToDocumentPoint):
(WebCore::FrameView::documentToClientOffset):
* page/FrameView.h:
* platform/graphics/FloatPoint.h:
(WebCore::FloatPoint::scale):
(WebCore::FloatPoint::scaled):
* platform/graphics/FloatSize.h:
(WebCore::FloatSize::scaled):
* platform/graphics/LayoutPoint.h:
(WebCore::LayoutPoint::scaled):

Tools:

Make "Zoom In" and "Zoom Out" work correctly in the WebKit1 window. Previously they
always did text zooming.

* MiniBrowser/mac/WK1BrowserWindowController.m:
(-[WK1BrowserWindowController zoomIn:]):
(-[WK1BrowserWindowController zoomOut:]):
(-[WK1BrowserWindowController canResetZoom]):
(-[WK1BrowserWindowController resetZoom:]):

LayoutTests:

* fast/events/clientXY-in-zoom-and-scroll.html: New baseline for progressed behavior.
* fast/events/simulated-click-zoomed-expected.txt: Added.
* fast/events/simulated-click-zoomed.html: Added.
* fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed-expected.txt: Added.
* fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed.html: Added.
* platform/ios/TestExpectations:
* platform/mac/fast/events/clientXY-in-zoom-and-scroll-expected.txt:

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

21 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/events/clientXY-in-zoom-and-scroll.html
LayoutTests/fast/events/simulated-click-zoomed-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/simulated-click-zoomed.html [new file with mode: 0644]
LayoutTests/fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed-expected.txt [new file with mode: 0644]
LayoutTests/fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed.html [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
LayoutTests/platform/mac/fast/events/clientXY-in-zoom-and-scroll-expected.txt
Source/WebCore/ChangeLog
Source/WebCore/dom/Element.cpp
Source/WebCore/dom/Element.h
Source/WebCore/dom/MouseRelatedEvent.cpp
Source/WebCore/dom/MouseRelatedEvent.h
Source/WebCore/dom/SimulatedClick.cpp
Source/WebCore/page/FrameView.cpp
Source/WebCore/page/FrameView.h
Source/WebCore/platform/graphics/FloatPoint.h
Source/WebCore/platform/graphics/FloatSize.h
Source/WebCore/platform/graphics/LayoutPoint.h
Tools/ChangeLog
Tools/MiniBrowser/mac/WK1BrowserWindowController.m

index 2fabe2b..c64bdc2 100644 (file)
@@ -1,3 +1,19 @@
+2017-06-28  Simon Fraser  <simon.fraser@apple.com>
+
+        getBoundingClientRect returns wrong value for combination of page zoom and scroll
+        https://bugs.webkit.org/show_bug.cgi?id=173841
+        rdar://problem/32983841
+
+        Reviewed by Dean Jackson.
+
+        * fast/events/clientXY-in-zoom-and-scroll.html: New baseline for progressed behavior.
+        * fast/events/simulated-click-zoomed-expected.txt: Added.
+        * fast/events/simulated-click-zoomed.html: Added.
+        * fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed-expected.txt: Added.
+        * fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed.html: Added.
+        * platform/ios/TestExpectations:
+        * platform/mac/fast/events/clientXY-in-zoom-and-scroll-expected.txt:
+
 2017-06-29  John Wilander  <wilander@apple.com>
 
         Fix for intermittent Layout Test fail http/tests/loading/resourceLoadStatistics/telemetry-generation.html
index 877a5cd..29d20ad 100644 (file)
     {
         event = e;
         debug("\nZoomed and scrolled");
-        shouldBe("event.clientX", hasSubpixelSupport ? "73" : "74");
-        shouldBe("event.clientY", hasSubpixelSupport ? "73" : "74");
+        shouldBe("event.clientX", hasSubpixelSupport ? "83" : "84");
+        shouldBe("event.clientY", hasSubpixelSupport ? "83" : "84");
         shouldBe("event.pageX", "133");
         shouldBe("event.pageY", "133");
     }
diff --git a/LayoutTests/fast/events/simulated-click-zoomed-expected.txt b/LayoutTests/fast/events/simulated-click-zoomed-expected.txt
new file mode 100644 (file)
index 0000000..c50b3d2
--- /dev/null
@@ -0,0 +1,6 @@
+PASS event.clientX is 120
+PASS event.clientY is 200
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Click me
diff --git a/LayoutTests/fast/events/simulated-click-zoomed.html b/LayoutTests/fast/events/simulated-click-zoomed.html
new file mode 100644 (file)
index 0000000..0eda120
--- /dev/null
@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="../../resources/js-test-pre.js"></script>
+    
+    <style>
+        body {
+            height: 2000px;
+        }
+
+        button {
+            position: absolute;
+            top: 400px;
+            left: 20px;
+            height: 100px;
+            width: 200px;
+        }
+    </style>
+    <script>
+        function buttonClicked(event)
+        {
+            shouldBe('event.clientX', '120');
+            shouldBe('event.clientY', '200');
+        }
+    </script>
+</head>
+<body>
+
+<button onclick="buttonClicked(event)">Click me</button>
+<script>
+    if (window.eventSender)
+        eventSender.zoomPageIn();
+
+    window.scrollTo(0, 250);
+
+    var button = document.getElementsByTagName('button')[0];
+    button.focus();
+    
+    if (window.eventSender)
+        eventSender.keyDown(" "); // Activate the button
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed-expected.txt b/LayoutTests/fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed-expected.txt
new file mode 100644 (file)
index 0000000..aef23f1
--- /dev/null
@@ -0,0 +1,23 @@
+This test zooms and scrolls the page and checks that client rects are correct.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+layoutViewport: 0.00, 0.00 - 785.00 x 585.00
+visualViewportRect: 0.00, 0.00 - 785.00 x 585.00
+fixed client bounds: 11.99, 10.00 - 30.00 x 20.00
+fixed client rect: 11.99, 10.00 - 30.00 x 20.00
+absolute client bounds: 120.00, 100.00 - 50.00 x 25.00
+absolute client rect: 120.00, 100.00 - 50.00 x 25.00
+
+Scrolled to 119, 99
+layoutViewport: 144.00, 120.00 - 785.00 x 585.00
+visualViewportRect: 144.00, 120.00 - 785.00 x 585.00
+fixed client bounds: 11.99, 10.00 - 30.00 x 20.00
+fixed client rect: 11.99, 10.00 - 30.00 x 20.00
+absolute client bounds: 0.00, 0.00 - 50.00 x 25.00
+absolute client rect: 0.00, 0.00 - 50.00 x 25.00
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed.html b/LayoutTests/fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed.html
new file mode 100644 (file)
index 0000000..7e52e9b
--- /dev/null
@@ -0,0 +1,85 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+    <style>
+        body {
+            height: 3000px;
+            width: 2000px;
+        }
+        #fixed {
+            position: fixed;
+            top: 10px;
+            left: 12px;
+            width: 30px;
+            height: 20px;
+            background-color: silver;
+        }
+        #absolute {
+            position: absolute;
+            top: 100px;
+            left: 120px;
+            width: 50px;
+            height: 25px;
+            background-color: blue;
+        }
+    </style>
+    <script src="../../resources/js-test-pre.js"></script>
+    <script>
+        description("This test zooms and scrolls the page and checks that client rects are correct.");
+
+        if (window.internals)
+            internals.settings.setVisualViewportEnabled(true);
+
+        window.jsTestIsAsync = true;
+
+        function stringFromRect(domRect)
+        {
+            return `${domRect.x.toFixed(2)}, ${domRect.y.toFixed(2)} - ${domRect.width.toFixed(2)} x ${domRect.height.toFixed(2)}`;
+        }
+
+        function dumpRects()
+        {
+            var fixed = document.getElementById('fixed');
+            var absolute = document.getElementById('absolute');
+
+            debug('layoutViewport: ' + stringFromRect(internals.layoutViewportRect()));
+            debug('visualViewportRect: ' + stringFromRect(internals.visualViewportRect()));
+
+            debug('fixed client bounds: ' + stringFromRect(fixed.getBoundingClientRect()));
+            debug('fixed client rect: ' + stringFromRect(fixed.getClientRects()[0]));
+
+            debug('absolute client bounds: ' + stringFromRect(absolute.getBoundingClientRect()));
+            debug('absolute client rect: ' + stringFromRect(absolute.getClientRects()[0]));
+        }
+
+        function doTest()
+        {
+            if (!window.testRunner)
+                return;
+
+            if (window.eventSender)
+                eventSender.zoomPageIn();
+
+            dumpRects();
+
+            debug('');
+            window.scrollTo(120, 100);
+
+            debug('Scrolled to ' + window.scrollX + ', ' + window.scrollY);
+            dumpRects();
+
+            window.scrollTo(0, 0);
+
+            finishJSTest();
+        }
+        
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div id="fixed"></div>
+    <div id="absolute"></div>
+    <script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
index 07e62f9..3ea8eb1 100644 (file)
@@ -327,6 +327,7 @@ fast/scrolling/scroll-animator-overlay-scrollbars-clicked.html [ Skip ]
 fast/scrolling/scroll-animator-overlay-scrollbars-hovered.html [ Skip ]
 fast/scrolling/scroll-animator-select-list-events.html [ Skip ]
 fast/events/prevent-default-prevents-interaction-with-scrollbars.html [ Skip ]
+fast/events/simulated-click-zoomed.html [ Skip ]
 fast/text/text-disappear-on-deselect.html [ ImageOnlyFailure ]
 fast/css/pseudo-active-on-labeled-control-without-renderer.html [ Skip ]
 fast/css/pseudo-active-on-labeled-element-not-canceled-by-focus.html [ Skip ]
@@ -2726,6 +2727,7 @@ fast/visual-viewport/zoomed-scroll-to-anchor-in-position-fixed.html [ Skip ]
 fast/visual-viewport/zoomed-scroll-into-view-fixed.html [ Skip ]
 fast/scrolling/scroll-to-anchor-zoomed-header.html [ Skip ]
 fast/visual-viewport/client-coordinates-relative-to-layout-viewport.html [ Skip ]
+fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed.html [ Skip ]
 
 # On iOS we do not visually highlight a programmatic selection
 fast/selectors/040.html
index 534a3c3..586881e 100644 (file)
@@ -17,8 +17,8 @@ PASS event.pageX is 150
 PASS event.pageY is 150
 
 Zoomed and scrolled
-PASS event.clientX is 73
-PASS event.clientY is 73
+PASS event.clientX is 83
+PASS event.clientY is 83
 PASS event.pageX is 133
 PASS event.pageY is 133
 PASS successfullyParsed is true
index b904938..fdf05c7 100644 (file)
@@ -1,3 +1,56 @@
+2017-06-28  Simon Fraser  <simon.fraser@apple.com>
+
+        getBoundingClientRect returns wrong value for combination of page zoom and scroll
+        https://bugs.webkit.org/show_bug.cgi?id=173841
+        rdar://problem/32983841
+
+        Reviewed by Dean Jackson.
+
+        The layout viewport returned by FrameView::layoutViewportRect() is affected by page (Command-+) zooming,
+        since it's computed using scroll positions, so when we use its origin to convert into client coordinates
+        (which are zoom-agnostic), we need to account for page zoom, so fix FrameView::documentToClientOffset()
+        to do this.
+
+        Callers of documentToClientOffset() were checked, revealing that event client coordinates were also
+        wrong with page zoom and are fixed in the same way. It was found that SimulatedClick was using an
+        entirely wrong rect to compute its location: Element::clientRect() is NOT in client coordinates,
+        so change this code to use getBoundingClientRect() instead.
+
+        Minor refactoring in MouseRelatedEvent to make getting to the FrameView cleaner.
+
+        Some geometry types enhanced to have non-mutating scale functions.
+
+        Tests: fast/events/simulated-click-zoomed.html
+               fast/visual-viewport/client-rects-relative-to-layout-viewport-zoomed.html
+
+        * dom/MouseRelatedEvent.cpp:
+        (WebCore::MouseRelatedEvent::init):
+        (WebCore::MouseRelatedEvent::initCoordinates):
+        (WebCore::MouseRelatedEvent::frameView):
+        (WebCore::MouseRelatedEvent::documentToAbsoluteScaleFactor):
+        (WebCore::MouseRelatedEvent::computePageLocation):
+        (WebCore::MouseRelatedEvent::computeRelativePosition):
+        (WebCore::pageZoomFactor): Deleted.
+        (WebCore::frameScaleFactor): Deleted.
+        * dom/MouseRelatedEvent.h:
+        (WebCore::MouseRelatedEvent::absoluteLocation):
+        (WebCore::MouseRelatedEvent::setAbsoluteLocation): Deleted.
+        * dom/SimulatedClick.cpp:
+        * page/FrameView.cpp:
+        (WebCore::FrameView::layoutViewportRect): baseLayoutViewportSize() is the same as the old code.
+        (WebCore::FrameView::documentToAbsoluteScaleFactor):
+        (WebCore::FrameView::absoluteToDocumentScaleFactor):
+        (WebCore::FrameView::absoluteToDocumentPoint):
+        (WebCore::FrameView::documentToClientOffset):
+        * page/FrameView.h:
+        * platform/graphics/FloatPoint.h:
+        (WebCore::FloatPoint::scale):
+        (WebCore::FloatPoint::scaled):
+        * platform/graphics/FloatSize.h:
+        (WebCore::FloatSize::scaled):
+        * platform/graphics/LayoutPoint.h:
+        (WebCore::LayoutPoint::scaled):
+
 2017-06-29  Megan Gardner  <megan_gardner@apple.com>
 
         Unreviewed, fixing Window's build after r218976
index a81ab08..4290cc5 100644 (file)
@@ -1162,7 +1162,7 @@ Ref<DOMRectList> Element::getClientRects()
     return DOMRectList::create(quads);
 }
 
-Ref<DOMRect> Element::getBoundingClientRect()
+FloatRect Element::boundingClientRect()
 {
     document().updateLayoutIgnorePendingStylesheets();
 
@@ -1180,14 +1180,19 @@ Ref<DOMRect> Element::getBoundingClientRect()
     }
 
     if (quads.isEmpty())
-        return DOMRect::create();
+        return { };
 
     FloatRect result = quads[0].boundingBox();
     for (size_t i = 1; i < quads.size(); ++i)
         result.unite(quads[i].boundingBox());
 
     document().convertAbsoluteToClientRect(result, renderer()->style());
-    return DOMRect::create(result);
+    return result;
+}
+
+Ref<DOMRect> Element::getBoundingClientRect()
+{
+    return DOMRect::create(boundingClientRect());
 }
 
 // Note that this is not web-exposed, and does not use the same coordinate system as getBoundingClientRect() and friends.
index 81b6459..012bbd8 100644 (file)
@@ -173,6 +173,8 @@ public:
 
     WEBCORE_EXPORT IntRect boundsInRootViewSpace();
 
+    FloatRect boundingClientRect();
+
     Ref<DOMRectList> getClientRects();
     Ref<DOMRect> getBoundingClientRect();
 
index 7eb8453..12cb767 100644 (file)
@@ -59,9 +59,8 @@ MouseRelatedEvent::MouseRelatedEvent(const AtomicString& eventType, const MouseR
 
 void MouseRelatedEvent::init(bool isSimulated, const IntPoint& windowLocation)
 {
-    auto* frame = view() ? view()->frame() : nullptr;
-    if (frame && !isSimulated) {
-        if (FrameView* frameView = frame->view()) {
+    if (!isSimulated) {
+        if (auto* frameView = this->frameView()) {
             FloatPoint absolutePoint = frameView->windowToContents(windowLocation);
             FloatPoint documentPoint = frameView->absoluteToDocumentPoint(absolutePoint);
             m_pageLocation = flooredLayoutPoint(documentPoint);
@@ -88,11 +87,8 @@ void MouseRelatedEvent::initCoordinates(const LayoutPoint& clientLocation)
     // Set up initial values for coordinates.
     // Correct values are computed lazily, see computeRelativePosition.
     FloatSize documentToClientOffset;
-    auto* frame = view() ? view()->frame() : nullptr;
-    if (frame) {
-        if (FrameView* frameView = frame->view())
-            documentToClientOffset = frameView->documentToClientOffset();
-    }
+    if (auto* frameView = this->frameView())
+        documentToClientOffset = frameView->documentToClientOffset();
 
     m_clientLocation = clientLocation;
     m_pageLocation = clientLocation - LayoutSize(documentToClientOffset);
@@ -104,32 +100,26 @@ void MouseRelatedEvent::initCoordinates(const LayoutPoint& clientLocation)
     m_hasCachedRelativePosition = false;
 }
 
-static float pageZoomFactor(const UIEvent* event)
+FrameView* MouseRelatedEvent::frameView() const
 {
-    DOMWindow* window = event->view();
-    if (!window)
-        return 1;
-    Frame* frame = window->frame();
+    auto* frame = view() ? view()->frame() : nullptr;
     if (!frame)
-        return 1;
-    return frame->pageZoomFactor();
+        return nullptr;
+
+    return frame->view();
 }
 
-static float frameScaleFactor(const UIEvent* event)
+float MouseRelatedEvent::documentToAbsoluteScaleFactor() const
 {
-    DOMWindow* window = event->view();
-    if (!window)
-        return 1;
-    Frame* frame = window->frame();
-    if (!frame)
-        return 1;
-    return frame->frameScaleFactor();
+    if (auto* frameView = this->frameView())
+        return frameView->documentToAbsoluteScaleFactor();
+
+    return 1;
 }
 
 void MouseRelatedEvent::computePageLocation()
 {
-    float scaleFactor = pageZoomFactor(this) * frameScaleFactor(this);
-    setAbsoluteLocation(LayoutPoint(pageX() * scaleFactor, pageY() * scaleFactor));
+    m_absoluteLocation = m_pageLocation.scaled(documentToAbsoluteScaleFactor());
 }
 
 void MouseRelatedEvent::receivedTarget()
@@ -153,7 +143,7 @@ void MouseRelatedEvent::computeRelativePosition()
     // Adjust offsetLocation to be relative to the target's position.
     if (RenderObject* r = targetNode->renderer()) {
         m_offsetLocation = LayoutPoint(r->absoluteToLocal(absoluteLocation(), UseTransforms));
-        float scaleFactor = 1 / (pageZoomFactor(this) * frameScaleFactor(this));
+        float scaleFactor = 1 / documentToAbsoluteScaleFactor();
         if (scaleFactor != 1.0f)
             m_offsetLocation.scale(scaleFactor);
     }
index e50f7cd..e260be2 100644 (file)
@@ -28,6 +28,8 @@
 
 namespace WebCore {
 
+class FrameView;
+
 struct MouseRelatedEventInit : public EventModifierInit {
     int screenX { 0 };
     int screenY { 0 };
@@ -62,7 +64,6 @@ public:
     // Page point in "absolute" coordinates (i.e. post-zoomed, page-relative coords,
     // usable with RenderObject::absoluteToLocal).
     const LayoutPoint& absoluteLocation() const { return m_absoluteLocation; }
-    void setAbsoluteLocation(const LayoutPoint& p) { m_absoluteLocation = p; }
 
 protected:
     MouseRelatedEvent() = default;
@@ -81,6 +82,9 @@ protected:
     void computePageLocation();
     void computeRelativePosition();
 
+    float documentToAbsoluteScaleFactor() const;
+    FrameView* frameView() const;
+
     // Expose these so MouseEvent::initMouseEvent can set them.
     IntPoint m_screenLocation;
     LayoutPoint m_clientLocation;
index 427ac52..6fb3b4d 100644 (file)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "SimulatedClick.h"
 
+#include "DOMRect.h"
 #include "DataTransfer.h"
 #include "Element.h"
 #include "EventDispatcher.h"
@@ -72,7 +73,7 @@ private:
             // (element.click()), the coordinates will be 0, similarly to Firefox and Chrome.
             // Note that the call to screenRect() causes a synchronous IPC with the UI process.
             m_screenLocation = target.screenRect().center();
-            initCoordinates(LayoutPoint(target.clientRect().center()));
+            initCoordinates(LayoutPoint(target.boundingClientRect().center()));
         }
     }
 
index fea2a2b..a692d04 100644 (file)
@@ -1978,7 +1978,7 @@ LayoutRect FrameView::layoutViewportRect() const
         return m_layoutViewportOverrideRect.value();
 
     // Size of initial containing block, anchored at scroll position, in document coordinates (unchanged by scale factor).
-    return LayoutRect(m_layoutViewportOrigin, renderView() ? renderView()->size() : size());
+    return LayoutRect(m_layoutViewportOrigin, baseLayoutViewportSize());
 }
 
 // visibleContentRect is in the bounds of the scroll view content. That consists of an
@@ -4882,11 +4882,16 @@ IntPoint FrameView::convertFromContainingView(const IntPoint& parentPoint) const
     return parentPoint;
 }
 
+float FrameView::documentToAbsoluteScaleFactor(std::optional<float> effectiveZoom) const
+{
+    // If effectiveZoom is passed, it already factors in pageZoomFactor(). 
+    return effectiveZoom.value_or(frame().pageZoomFactor()) * frame().frameScaleFactor();
+}
+
 float FrameView::absoluteToDocumentScaleFactor(std::optional<float> effectiveZoom) const
 {
     // If effectiveZoom is passed, it already factors in pageZoomFactor(). 
-    float cssZoom = effectiveZoom.value_or(frame().pageZoomFactor()); 
-    return 1 / (cssZoom * frame().frameScaleFactor());
+    return 1 / documentToAbsoluteScaleFactor(effectiveZoom);
 }
 
 FloatRect FrameView::absoluteToDocumentRect(FloatRect rect, std::optional<float> effectiveZoom) const
@@ -4897,13 +4902,15 @@ FloatRect FrameView::absoluteToDocumentRect(FloatRect rect, std::optional<float>
 
 FloatPoint FrameView::absoluteToDocumentPoint(FloatPoint p, std::optional<float> effectiveZoom) const
 {
-    p.scale(absoluteToDocumentScaleFactor(effectiveZoom));
-    return p;
+    return p.scaled(absoluteToDocumentScaleFactor(effectiveZoom));
 }
 
 FloatSize FrameView::documentToClientOffset() const
 {
-    return frame().settings().visualViewportEnabled() ? -toFloatSize(layoutViewportRect().location()) : -toFloatSize(visibleContentRect().location());
+    FloatSize clientOrigin = frame().settings().visualViewportEnabled() ? -toFloatSize(layoutViewportRect().location()) : -toFloatSize(visibleContentRect().location());
+
+    // Layout and visual viewports are affected by page zoom, so we need to factor that out.
+    return clientOrigin.scaled(1 / frame().pageZoomFactor());
 }
 
 FloatRect FrameView::documentToClientRect(FloatRect rect) const
index 13da292..1c46714 100644 (file)
@@ -259,7 +259,7 @@ public:
     // Used with delegated scrolling (i.e. iOS).
     WEBCORE_EXPORT void setLayoutViewportOverrideRect(std::optional<LayoutRect>);
 
-    // These are in document coordinates, unaffected by zooming.
+    // These are in document coordinates, unaffected by page scale (but affected by zooming).
     WEBCORE_EXPORT LayoutRect layoutViewportRect() const;
     WEBCORE_EXPORT LayoutRect visualViewportRect() const;
     
@@ -445,6 +445,7 @@ public:
     //
     // "Document"
     //    Relative to the document's scroll origin, but not affected by page zoom or page scale. Size is equivalent to CSS pixel dimensions.
+    //    FIXME: some uses are affected by page zoom (e.g. layout and visual viewports).
     //
     // "Client"
     //    Relative to the visible part of the document (or, more strictly, the layout viewport rect), and with the same scaling
@@ -463,7 +464,9 @@ public:
     IntPoint convertToContainingView(const IntPoint&) const final;
     IntPoint convertFromContainingView(const IntPoint&) const final;
 
+    float documentToAbsoluteScaleFactor(std::optional<float> effectiveZoom = std::nullopt) const;
     float absoluteToDocumentScaleFactor(std::optional<float> effectiveZoom = std::nullopt) const;
+
     FloatRect absoluteToDocumentRect(FloatRect, std::optional<float> effectiveZoom = std::nullopt) const;
     FloatPoint absoluteToDocumentPoint(FloatPoint, std::optional<float> effectiveZoom = std::nullopt) const;
 
index bd722b1..6ed4d21 100644 (file)
@@ -118,10 +118,20 @@ public:
         m_y *= scale;
     }
 
-    void scale(float sx, float sy)
+    void scale(float scaleX, float scaleY)
     {
-        m_x *= sx;
-        m_y *= sy;
+        m_x *= scaleX;
+        m_y *= scaleY;
+    }
+
+    FloatPoint scaled(float scale)
+    {
+        return { m_x * scale, m_y * scale };
+    }
+
+    FloatPoint scaled(float scaleX, float scaleY)
+    {
+        return { m_x * scaleX, m_y * scaleY };
     }
 
     WEBCORE_EXPORT void normalize();
index 83580ff..51e52ac 100644 (file)
@@ -94,6 +94,16 @@ public:
         m_height *= scaleY;
     }
 
+    FloatSize scaled(float s) const
+    {
+        return { m_width * s, m_height * s };
+    }
+
+    FloatSize scaled(float scaleX, float scaleY) const
+    {
+        return { m_width * scaleX, m_height * scaleY };
+    }
+
     WEBCORE_EXPORT FloatSize constrainedBetween(const FloatSize& min, const FloatSize& max) const;
 
     FloatSize expandedTo(const FloatSize& other) const
index f7d7d06..51262d2 100644 (file)
@@ -67,6 +67,16 @@ public:
         m_y *= sy;
     }
 
+    LayoutPoint scaled(float s) const
+    {
+        return { m_x * s, m_y * s };
+    }
+
+    LayoutPoint scaled(float sx, float sy) const
+    {
+        return { m_x * sx, m_y * sy };
+    }
+
     LayoutPoint constrainedBetween(const LayoutPoint& min, const LayoutPoint& max) const;
 
     LayoutPoint expandedTo(const LayoutPoint& other) const
index c3b8ddb..fee85a6 100644 (file)
@@ -1,3 +1,20 @@
+2017-06-28  Simon Fraser  <simon.fraser@apple.com>
+
+        getBoundingClientRect returns wrong value for combination of page zoom and scroll
+        https://bugs.webkit.org/show_bug.cgi?id=173841
+        rdar://problem/32983841
+
+        Reviewed by Dean Jackson.
+
+        Make "Zoom In" and "Zoom Out" work correctly in the WebKit1 window. Previously they
+        always did text zooming.
+
+        * MiniBrowser/mac/WK1BrowserWindowController.m:
+        (-[WK1BrowserWindowController zoomIn:]):
+        (-[WK1BrowserWindowController zoomOut:]):
+        (-[WK1BrowserWindowController canResetZoom]):
+        (-[WK1BrowserWindowController resetZoom:]):
+
 2017-06-29  John Wilander  <wilander@apple.com>
 
         Fix for intermittent Layout Test fail http/tests/loading/resourceLoadStatistics/telemetry-generation.html
index c782718..0b5d3db 100644 (file)
@@ -199,7 +199,11 @@ static BOOL areEssentiallyEqual(double a, double b)
     if (![self canZoomIn])
         return;
 
-    [_webView makeTextLarger:sender];
+    if (_zoomTextOnly)
+        [_webView makeTextLarger:sender];
+    else
+        [_webView zoomPageIn:sender];
+
 }
 
 - (BOOL)canZoomOut
@@ -212,12 +216,15 @@ static BOOL areEssentiallyEqual(double a, double b)
     if (![self canZoomIn])
         return;
 
-    [_webView makeTextSmaller:sender];
+    if (_zoomTextOnly)
+        [_webView makeTextSmaller:sender];
+    else
+        [_webView zoomPageOut:sender];
 }
 
 - (BOOL)canResetZoom
 {
-    return [_webView canMakeTextStandardSize];
+    return _zoomTextOnly ? [_webView canMakeTextStandardSize] : [_webView canResetPageZoom];
 }
 
 - (void)resetZoom:(id)sender
@@ -225,7 +232,10 @@ static BOOL areEssentiallyEqual(double a, double b)
     if (![self canResetZoom])
         return;
 
-    [_webView makeTextStandardSize:sender];
+    if (_zoomTextOnly)
+        [_webView makeTextStandardSize:sender];
+    else
+        [_webView resetPageZoom:sender];
 }
 
 - (IBAction)toggleZoomMode:(id)sender