Title: [274464] trunk/Source/WebCore
Revision
274464
Author
pan...@apple.com
Date
2021-03-15 21:33:30 -0700 (Mon, 15 Mar 2021)

Log Message

Web Inspector: Grid overlay does not adjust for element inside iframes
https://bugs.webkit.org/show_bug.cgi?id=222920

Reviewed by Simon Fraser.

Resolves an issue when overlays are applied to grids within iframes, which need to account for the
position/transform of their containing frame, and any other parent frames, to appear correctly in relation to
the root frame. This patch also has the side effect of changing how drawing the grid overlay while the page is
scrolled by allowing `localPointToRootPoint` to account for the scroll translation instead of explicitly
offsetting drawing by the scroll offset.

`Widget`, `ScrollView`, and `FrameView` are updated to retain floating-point precision where necessary.

* inspector/InspectorOverlay.cpp:
(WebCore::localPointToRootPoint):
- Use floating-point precision instead of integer precision.
(WebCore::InspectorOverlay::drawGridOverlay):
- Use `localPointToRootPoint` to calculate absolute points relative to a `renderGrid`'s parent hierarchy.
- Remove graphics context translation, as `localPointToRootPoint` will account for this translation.
* page/FrameView.cpp:
(WebCore::FrameView::convertFromRendererToContainingView const):
(WebCore::FrameView::convertToContainingView const):
* page/FrameView.h:
* platform/ScrollView.cpp:
(WebCore::ScrollView::contentsToView const):
* platform/ScrollView.h:
(WebCore::ScrollView::convertChildToSelf const):
* platform/Widget.cpp:
(WebCore::Widget::convertToContainingView const):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (274463 => 274464)


--- trunk/Source/WebCore/ChangeLog	2021-03-16 03:54:53 UTC (rev 274463)
+++ trunk/Source/WebCore/ChangeLog	2021-03-16 04:33:30 UTC (rev 274464)
@@ -1,3 +1,35 @@
+2021-03-15  Patrick Angle  <pan...@apple.com>
+
+        Web Inspector: Grid overlay does not adjust for element inside iframes
+        https://bugs.webkit.org/show_bug.cgi?id=222920
+
+        Reviewed by Simon Fraser.
+
+        Resolves an issue when overlays are applied to grids within iframes, which need to account for the
+        position/transform of their containing frame, and any other parent frames, to appear correctly in relation to
+        the root frame. This patch also has the side effect of changing how drawing the grid overlay while the page is
+        scrolled by allowing `localPointToRootPoint` to account for the scroll translation instead of explicitly
+        offsetting drawing by the scroll offset.
+
+        `Widget`, `ScrollView`, and `FrameView` are updated to retain floating-point precision where necessary.
+
+        * inspector/InspectorOverlay.cpp:
+        (WebCore::localPointToRootPoint):
+        - Use floating-point precision instead of integer precision.
+        (WebCore::InspectorOverlay::drawGridOverlay):
+        - Use `localPointToRootPoint` to calculate absolute points relative to a `renderGrid`'s parent hierarchy.
+        - Remove graphics context translation, as `localPointToRootPoint` will account for this translation.
+        * page/FrameView.cpp:
+        (WebCore::FrameView::convertFromRendererToContainingView const):
+        (WebCore::FrameView::convertToContainingView const):
+        * page/FrameView.h:
+        * platform/ScrollView.cpp:
+        (WebCore::ScrollView::contentsToView const):
+        * platform/ScrollView.h:
+        (WebCore::ScrollView::convertChildToSelf const):
+        * platform/Widget.cpp:
+        (WebCore::Widget::convertToContainingView const):
+
 2021-03-15  Simon Fraser  <simon.fra...@apple.com>
 
         Optimize canvas repaints

Modified: trunk/Source/WebCore/inspector/InspectorOverlay.cpp (274463 => 274464)


--- trunk/Source/WebCore/inspector/InspectorOverlay.cpp	2021-03-16 03:54:53 UTC (rev 274463)
+++ trunk/Source/WebCore/inspector/InspectorOverlay.cpp	2021-03-16 04:33:30 UTC (rev 274464)
@@ -99,7 +99,7 @@
 
 static FloatPoint localPointToRootPoint(const FrameView* view, const FloatPoint& point)
 {
-    return view->contentsToRootView(roundedIntPoint(point));
+    return view->contentsToRootView(point);
 }
 
 static void contentsQuadToCoordinateSystem(const FrameView* mainView, const FrameView* view, FloatQuad& quad, InspectorOverlay::CoordinateSystem coordinateSystem)
@@ -1400,19 +1400,11 @@
     
     constexpr auto translucentLabelBackgroundColor = Color::white.colorWithAlphaByte(153);
     
-    IntPoint scrollOffset;
     FrameView* pageView = m_page.mainFrame().view();
-    if (!pageView->delegatesScrolling())
-        scrollOffset = pageView->visibleContentRect().location();
+    if (!pageView)
+        return;
+    FloatRect viewportBounds = { { 0, 0 }, pageView->sizeForVisibleContent() };
     
-    FloatSize contentInset(0, pageView->topContentInset(ScrollView::TopContentInsetType::WebCoreOrPlatformContentInset));
-    float pageScaleFactor = m_page.pageScaleFactor();
-
-    float scrollX = scrollOffset.x() * pageScaleFactor;
-    float scrollY = scrollOffset.y() * pageScaleFactor;
-    
-    FloatRect viewportBounds = { FloatPoint(scrollX, scrollY), pageView->sizeForVisibleContent() };
-    
     auto& renderGrid = *downcast<RenderGrid>(renderer);
     auto columnPositions = renderGrid.columnPositions();
     auto rowPositions = renderGrid.rowPositions();
@@ -1423,25 +1415,26 @@
     float gridEndX = columnPositions[columnPositions.size() - 1];
     float gridStartY = rowPositions[0];
     float gridEndY = rowPositions[rowPositions.size() - 1];
-    
-    // FIXME: <webkit.org/b/222920> Grid overlay does not adjust for element inside iframes.
+
+    Frame* containingFrame = node->document().frame();
+    if (!containingFrame)
+        return;
+    FrameView* containingView = containingFrame->view();
+
     auto columnLineAt = [&](int x) -> FloatLine {
         return {
-            renderGrid.localToContainerPoint(FloatPoint(x, gridStartY), nullptr),
-            renderGrid.localToContainerPoint(FloatPoint(x, gridEndY), nullptr),
+            localPointToRootPoint(containingView, renderGrid.localToContainerPoint(FloatPoint(x, gridStartY), nullptr)),
+            localPointToRootPoint(containingView, renderGrid.localToContainerPoint(FloatPoint(x, gridEndY), nullptr)),
         };
     };
     auto rowLineAt = [&](int y) -> FloatLine {
         return {
-            renderGrid.localToContainerPoint(FloatPoint(gridStartX, y), nullptr),
-            renderGrid.localToContainerPoint(FloatPoint(gridEndX, y), nullptr),
+            localPointToRootPoint(containingView, renderGrid.localToContainerPoint(FloatPoint(gridStartX, y), nullptr)),
+            localPointToRootPoint(containingView, renderGrid.localToContainerPoint(FloatPoint(gridEndX, y), nullptr)),
         };
     };
     
     GraphicsContextStateSaver saver(context);
-
-    // Drawing code is relative to the visible viewport area.
-    context.translate(0 - scrollX, contentInset.height() - scrollY);
     context.setStrokeThickness(1);
     context.setStrokeColor(gridOverlay.config.gridColor);
 

Modified: trunk/Source/WebCore/page/FrameView.cpp (274463 => 274464)


--- trunk/Source/WebCore/page/FrameView.cpp	2021-03-16 03:54:53 UTC (rev 274463)
+++ trunk/Source/WebCore/page/FrameView.cpp	2021-03-16 04:33:30 UTC (rev 274464)
@@ -4785,6 +4785,11 @@
     return contentsToView(point);
 }
 
+FloatPoint FrameView::convertFromRendererToContainingView(const RenderElement* renderer, const FloatPoint& rendererPoint) const
+{
+    return contentsToView(renderer->localToAbsolute(rendererPoint, UseTransforms));
+}
+
 IntPoint FrameView::convertFromContainingViewToRenderer(const RenderElement* renderer, const IntPoint& viewPoint) const
 {
     IntPoint point = viewPoint;
@@ -4883,6 +4888,28 @@
     return localPoint;
 }
 
+FloatPoint FrameView::convertToContainingView(const FloatPoint& localPoint) const
+{
+    if (const ScrollView* parentScrollView = parent()) {
+        if (is<FrameView>(*parentScrollView)) {
+            const FrameView& parentView = downcast<FrameView>(*parentScrollView);
+
+            // Get our renderer in the parent view
+            RenderWidget* renderer = frame().ownerRenderer();
+            if (!renderer)
+                return localPoint;
+
+            auto point = localPoint;
+            point.moveBy(renderer->contentBoxLocation());
+            return parentView.convertFromRendererToContainingView(renderer, point);
+        }
+
+        return Widget::convertToContainingView(localPoint);
+    }
+
+    return localPoint;
+}
+
 IntPoint FrameView::convertFromContainingView(const IntPoint& parentPoint) const
 {
     if (const ScrollView* parentScrollView = parent()) {

Modified: trunk/Source/WebCore/page/FrameView.h (274463 => 274464)


--- trunk/Source/WebCore/page/FrameView.h	2021-03-16 03:54:53 UTC (rev 274463)
+++ trunk/Source/WebCore/page/FrameView.h	2021-03-16 04:33:30 UTC (rev 274464)
@@ -481,6 +481,7 @@
     WEBCORE_EXPORT IntRect convertFromContainingViewToRenderer(const RenderElement*, const IntRect&) const;
     WEBCORE_EXPORT FloatRect convertFromContainingViewToRenderer(const RenderElement*, const FloatRect&) const;
     WEBCORE_EXPORT IntPoint convertFromRendererToContainingView(const RenderElement*, const IntPoint&) const;
+    WEBCORE_EXPORT FloatPoint convertFromRendererToContainingView(const RenderElement*, const FloatPoint&) const;
     WEBCORE_EXPORT IntPoint convertFromContainingViewToRenderer(const RenderElement*, const IntPoint&) const;
 
     // Override ScrollView methods to do point conversion via renderers, in order to take transforms into account.
@@ -488,6 +489,7 @@
     IntRect convertFromContainingView(const IntRect&) const final;
     FloatRect convertFromContainingView(const FloatRect&) const final;
     IntPoint convertToContainingView(const IntPoint&) const final;
+    FloatPoint convertToContainingView(const FloatPoint&) const final;
     IntPoint convertFromContainingView(const IntPoint&) const final;
 
     float documentToAbsoluteScaleFactor(Optional<float> effectiveZoom = WTF::nullopt) const;

Modified: trunk/Source/WebCore/platform/ScrollView.cpp (274463 => 274464)


--- trunk/Source/WebCore/platform/ScrollView.cpp	2021-03-16 03:54:53 UTC (rev 274463)
+++ trunk/Source/WebCore/platform/ScrollView.cpp	2021-03-16 04:33:30 UTC (rev 274464)
@@ -890,8 +890,7 @@
 {
     if (delegatesScrolling())
         return point;
-
-    return contentsToView(IntPoint(point));
+    return point - toFloatSize(documentScrollPositionRelativeToViewOrigin());
 }
 
 IntRect ScrollView::viewToContents(IntRect rect) const

Modified: trunk/Source/WebCore/platform/ScrollView.h (274463 => 274464)


--- trunk/Source/WebCore/platform/ScrollView.h	2021-03-16 03:54:53 UTC (rev 274463)
+++ trunk/Source/WebCore/platform/ScrollView.h	2021-03-16 04:33:30 UTC (rev 274464)
@@ -354,6 +354,15 @@
         return newPoint;
     }
 
+    FloatPoint convertChildToSelf(const Widget* child, const FloatPoint& point) const
+    {
+        FloatPoint newPoint = point;
+        if (!isScrollViewScrollbar(child))
+            newPoint -= toFloatSize(scrollPosition());
+        newPoint.moveBy(child->location());
+        return newPoint;
+    }
+
     IntPoint convertSelfToChild(const Widget* child, const IntPoint& point) const
     {
         IntPoint newPoint = point;

Modified: trunk/Source/WebCore/platform/Widget.cpp (274463 => 274464)


--- trunk/Source/WebCore/platform/Widget.cpp	2021-03-16 03:54:53 UTC (rev 274463)
+++ trunk/Source/WebCore/platform/Widget.cpp	2021-03-16 04:33:30 UTC (rev 274464)
@@ -263,7 +263,10 @@
 
 FloatPoint Widget::convertToContainingView(const FloatPoint& localPoint) const
 {
-    return convertToContainingView(IntPoint(localPoint));
+    if (const ScrollView* parentScrollView = parent())
+        return parentScrollView->convertChildToSelf(this, localPoint);
+
+    return localPoint;
 }
 
 FloatPoint Widget::convertFromContainingView(const FloatPoint& parentPoint) const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to