Changeset 274464 in webkit

Timestamp:

Mar 15, 2021 9:33:30 PM (16 months ago)

Author:

Patrick Angle

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• inspector/InspectorOverlay.cpp (modified) (3 diffs)
• page/FrameView.cpp (modified) (2 diffs)
• page/FrameView.h (modified) (2 diffs)
• platform/ScrollView.cpp (modified) (1 diff)
• platform/ScrollView.h (modified) (1 diff)
• platform/Widget.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-03-15  Patrick Angle  <pangle@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.fraser@apple.com>
 

trunk/Source/WebCore/inspector/InspectorOverlay.cpp

@@ -100 +100 @@
 static FloatPoint localPointToRootPoint(const FrameView* view, const FloatPoint& point)
 {
-    return view->contentsToRootView(roundedIntPoint(point));
+    return view->contentsToRootView(point);
 }
 
@@ -1401 +1401 @@
     constexpr auto translucentLabelBackgroundColor = Color::white.colorWithAlphaByte(153);
     
-    IntPoint scrollOffset;
     FrameView* pageView = m_page.mainFrame().view();
-    if (!pageView->delegatesScrolling())
-        scrollOffset = pageView->visibleContentRect().location();
-    
-    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() };
+    if (!pageView)
+        return;
+    FloatRect viewportBounds = { { 0, 0 }, pageView->sizeForVisibleContent() };
     
     auto& renderGrid = *downcast<RenderGrid>(renderer);
@@ -1424 +1416 @@
     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);

trunk/Source/WebCore/page/FrameView.cpp

@@ -4786 +4786 @@
 }
 
+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
 {
@@ -4884 +4889 @@
 }
 
+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
 {

trunk/Source/WebCore/page/FrameView.h

@@ -482 +482 @@
     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;
 
@@ -489 +490 @@
     FloatRect convertFromContainingView(const FloatRect&) const final;
     IntPoint convertToContainingView(const IntPoint&) const final;
+    FloatPoint convertToContainingView(const FloatPoint&) const final;
     IntPoint convertFromContainingView(const IntPoint&) const final;
 

trunk/Source/WebCore/platform/ScrollView.cpp

@@ -891 +891 @@
     if (delegatesScrolling())
         return point;
-
-    return contentsToView(IntPoint(point));
+    return point - toFloatSize(documentScrollPositionRelativeToViewOrigin());
 }
 

trunk/Source/WebCore/platform/ScrollView.h

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

trunk/Source/WebCore/platform/Widget.cpp

@@ -264 +264 @@
 FloatPoint Widget::convertToContainingView(const FloatPoint& localPoint) const
 {
-    return convertToContainingView(IntPoint(localPoint));
+    if (const ScrollView* parentScrollView = parent())
+        return parentScrollView->convertChildToSelf(this, localPoint);
+
+    return localPoint;
 }