Changeset 261774 in webkit

Timestamp:

May 15, 2020 7:07:00 PM (4 years ago)

Author:

Simon Fraser

Message:

REGRESSION (r249091): Can't click on a video in the second column of a paginated web view
​https://bugs.webkit.org/show_bug.cgi?id=211973
<rdar://problem/61418775>

Reviewed by Zalan Bujtas.

Source/WebCore:

In r249091 I made clip layer computation use offsetFromAncestor() by default, but this turns
out to give different behavior from mapping via renderers in columns.

The bug was that accumulateOffsetTowardsAncestor() would map through the
RenderMultiColumnFlow columns if the ancestorLayer was the one that was using columns,
but mapping via renderers only maps through columns if converting to some ancestor of
the columnated renderer.

I did not investigate why this only affects video.

Test: fast/multicol/clipped-video-in-second-column.html

• rendering/RenderLayer.cpp:

(WebCore::accumulateOffsetTowardsAncestor):
(WebCore::RenderLayer::calculateClipRects const):

LayoutTests:

• fast/multicol/clipped-video-in-second-column-expected.html: Added.
• fast/multicol/clipped-video-in-second-column.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/multicol/clipped-video-in-second-column-expected.html (added)
• LayoutTests/fast/multicol/clipped-video-in-second-column.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-05-15  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r249091): Can't click on a video in the second column of a paginated web view
+        https://bugs.webkit.org/show_bug.cgi?id=211973
+        <rdar://problem/61418775>
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/multicol/clipped-video-in-second-column-expected.html: Added.
+        * fast/multicol/clipped-video-in-second-column.html: Added.
+
 2020-05-15  Devin Rousso  <drousso@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-05-15  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r249091): Can't click on a video in the second column of a paginated web view
+        https://bugs.webkit.org/show_bug.cgi?id=211973
+        <rdar://problem/61418775>
+
+        Reviewed by Zalan Bujtas.
+
+        In r249091 I made clip layer computation use offsetFromAncestor() by default, but this turns
+        out to give different behavior from mapping via renderers in columns.
+
+        The bug was that accumulateOffsetTowardsAncestor() would map through the
+        RenderMultiColumnFlow columns if the ancestorLayer was the one that was using columns,
+        but mapping via renderers only maps through columns if converting to some ancestor of
+        the columnated renderer.
+
+        I did not investigate why this only affects video.
+
+        Test: fast/multicol/clipped-video-in-second-column.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::accumulateOffsetTowardsAncestor):
+        (WebCore::RenderLayer::calculateClipRects const):
+
 2020-05-15  Kenneth Russell  <kbr@chromium.org>
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -2385 +2385 @@
 
     if (adjustForColumns == RenderLayer::AdjustForColumns) {
-        if (RenderLayer* parentLayer = layer->parent()) {
+        auto* parentLayer = layer->parent();
+        if (parentLayer && parentLayer != ancestorLayer) {
             if (is<RenderMultiColumnFlow>(parentLayer->renderer())) {
-                RenderFragmentContainer* fragment = downcast<RenderMultiColumnFlow>(parentLayer->renderer()).physicalTranslationFromFlowToFragment(location);
-                if (fragment)
+                if (auto* fragment = downcast<RenderMultiColumnFlow>(parentLayer->renderer()).physicalTranslationFromFlowToFragment(location))
                     location.moveBy(fragment->topLeftLocation() + -parentLayer->renderBox()->topLeftLocation());
             }
@@ -5784 +5784 @@
         if (clipRects.fixed() && &clipRectsContext.rootLayer->renderer() == &renderer().view())
             offset -= toLayoutSize(renderer().view().frameView().scrollPositionForFixedPosition());
-        
+
         if (renderer().hasOverflowClip()) {
             ClipRect newOverflowClip = downcast<RenderBox>(renderer()).overflowClipRectForChildLayers(offset, nullptr, clipRectsContext.overlayScrollbarSizeRelevancy);
@@ -5800 +5800 @@
     }
 
-    LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " calculateClipRects " << clipRects);
+    LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " calculateClipRects " << clipRectsContext << " computed " << clipRects);
 }