Title: [261774] trunk
Revision
261774
Author
simon.fra...@apple.com
Date
2020-05-15 19:07:00 -0700 (Fri, 15 May 2020)

Log 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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (261773 => 261774)


--- trunk/LayoutTests/ChangeLog	2020-05-16 01:23:40 UTC (rev 261773)
+++ trunk/LayoutTests/ChangeLog	2020-05-16 02:07:00 UTC (rev 261774)
@@ -1,3 +1,14 @@
+2020-05-15  Simon Fraser  <simon.fra...@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  <drou...@apple.com>
 
         Web Inspector: Fails to pretty-print a particular CSS file

Added: trunk/LayoutTests/fast/multicol/clipped-video-in-second-column-expected.html (0 => 261774)


--- trunk/LayoutTests/fast/multicol/clipped-video-in-second-column-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/multicol/clipped-video-in-second-column-expected.html	2020-05-16 02:07:00 UTC (rev 261774)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .multicol {
+            width: 680px;
+            height: 500px;
+            columns: 2;
+            padding: 10px;
+            border: 1px solid green;
+        }
+
+        img {
+            margin: 10px;
+            width: 300px;
+            height: 200px;
+            border: 5px solid black;
+            background-color: silver;
+        }
+
+        .container {
+            height: 400px;
+            overflow: hidden;
+            border: 1px solid gray;
+        }
+    </style>
+</head>
+<body>
+    <div class="multicol">
+        <div class="container">
+            <img>
+        </div>
+        <div class="container">
+            <img>
+        </div>
+    </div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/multicol/clipped-video-in-second-column.html (0 => 261774)


--- trunk/LayoutTests/fast/multicol/clipped-video-in-second-column.html	                        (rev 0)
+++ trunk/LayoutTests/fast/multicol/clipped-video-in-second-column.html	2020-05-16 02:07:00 UTC (rev 261774)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        .multicol {
+            width: 680px;
+            height: 500px;
+            columns: 2;
+            padding: 10px;
+            border: 1px solid green;
+        }
+
+        video {
+            margin: 10px;
+            width: 300px;
+            height: 200px;
+            border: 5px solid black;
+            background-color: silver;
+        }
+
+        .container {
+            height: 400px;
+            overflow: hidden;
+            border: 1px solid gray;
+        }
+    </style>
+</head>
+<body>
+    <div class="multicol">
+        <div class="container">
+            <video></video>
+        </div>
+        <div class="container">
+            <video></video>
+        </div>
+    </div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (261773 => 261774)


--- trunk/Source/WebCore/ChangeLog	2020-05-16 01:23:40 UTC (rev 261773)
+++ trunk/Source/WebCore/ChangeLog	2020-05-16 02:07:00 UTC (rev 261774)
@@ -1,3 +1,27 @@
+2020-05-15  Simon Fraser  <simon.fra...@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  <k...@chromium.org>
 
         OES_texture_float internal format conversion must depend on WEBGL_color_buffer_float

Modified: trunk/Source/WebCore/rendering/RenderLayer.cpp (261773 => 261774)


--- trunk/Source/WebCore/rendering/RenderLayer.cpp	2020-05-16 01:23:40 UTC (rev 261773)
+++ trunk/Source/WebCore/rendering/RenderLayer.cpp	2020-05-16 02:07:00 UTC (rev 261774)
@@ -2384,10 +2384,10 @@
     location += toLayoutSize(layer->location());
 
     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());
             }
         }
@@ -5783,7 +5783,7 @@
 
         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);
             newOverflowClip.setAffectedByRadius(renderer().style().hasBorderRadius());
@@ -5799,7 +5799,7 @@
         }
     }
 
-    LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " calculateClipRects " << clipRects);
+    LOG_WITH_STREAM(ClipRects, stream << "RenderLayer " << this << " calculateClipRects " << clipRectsContext << " computed " << clipRects);
 }
 
 Ref<ClipRects> RenderLayer::parentClipRects(const ClipRectsContext& clipRectsContext) const
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to