[content-visibility] Column spanner stays visible in skipped subtree

alanbaradlay · alanbaradlay · commit 910863b50bd8 · 2025-05-23T08:15:52.000-07:00
https://bugs.webkit.org/show_bug.cgi?id=293470 Reviewed by Antti Koivisto. 1. In WebKit, column spanners are moved out of their original insertion position and get reparented under the column container. 2. "is this renderer inside a skipped subtree" logic consults parent state. Now the parent of a column spanner (#1) may be completely outside of 'c-v: h' tricking us into believing the spanner is not hidden. * LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner.html: Added. * Source/WebCore/rendering/RenderBlock.cpp: (WebCore::RenderBlock::paintChild): * Source/WebCore/rendering/RenderBlockFlow.cpp: (WebCore::RenderBlockFlow::layoutBlockChildren): * Source/WebCore/rendering/RenderBox.h: * Source/WebCore/rendering/RenderBoxInlines.h: (WebCore::RenderBox::isColumnSpanner const): * Source/WebCore/rendering/RenderObject.cpp: (WebCore::RenderObject::isSkippedContent const): Canonical link: https://commits.webkit.org/295335@main
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner-expected.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner-expected.html
@@ -0,0 +1,12 @@
+<!doctype HTML>
+<html>
+<meta charset="utf8">
+<title>CSS Content Visibility: hidden column spanner</title>
+<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
+<style>
+body {
+  width: 600px;
+  overflow: hidden;
+}
+</style>
+Test fails if you can see text below.
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner-ref.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner-ref.html
@@ -0,0 +1,12 @@
+<!doctype HTML>
+<html>
+<meta charset="utf8">
+<title>CSS Content Visibility: hidden column spanner</title>
+<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
+<style>
+body {
+  width: 600px;
+  overflow: hidden;
+}
+</style>
+Test fails if you can see text below.
diff --git a/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner.html b/LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-hidden-with-column-spanner.html
@@ -0,0 +1,33 @@
+<!doctype HTML>
+<html>
+<meta charset="utf8">
+<title>CSS Content Visibility: hidden column spanner</title>
+<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
+<link rel="match" href="content-visibility-hidden-with-spanner-ref.html">
+<meta name="assert" content="content-visibility hidden column spanner should stay hidden">
+<style>
+body {
+  width: 600px;
+  overflow: hidden;
+}
+.container {
+  column-count: 2;
+  width: 200px;
+  height: 100px;
+}
+.spanner {
+  column-span: all;
+}
+</style>
+Test fails if you can see text below.
+<div class=container>
+  <div id=hide_this>
+    XXX
+    <div class=spanner>PASS if not visible</div>
+    XXX
+  </div>
+</div>
+<script>
+document.body.offsetHeight;
+hide_this.style.contentVisibility = "hidden";
+</script>
diff --git a/Source/WebCore/rendering/RenderBlock.cpp b/Source/WebCore/rendering/RenderBlock.cpp
@@ -1141,6 +1141,11 @@ bool RenderBlock::paintChild(RenderBox& child, PaintInfo& paintInfo, const Layou
     if (child.isExcludedAndPlacedInBorder())
         return true;
 
+    if (child.isSkippedContent()) {
+        ASSERT(child.isColumnSpanner());
+        return true;
+    }
+
     // Check for page-break-before: always, and if it's set, break and bail.
     bool checkBeforeAlways = !childrenInline() && (usePrintRect && alwaysPageBreak(child.style().breakBefore()));
     LayoutUnit absoluteChildY = paintOffset.y() + child.y();
diff --git a/Source/WebCore/rendering/RenderBlockFlow.cpp b/Source/WebCore/rendering/RenderBlockFlow.cpp
@@ -833,14 +833,20 @@ void RenderBlockFlow::layoutBlockChildren(RelayoutChildren relayoutChildren, Lay
     RenderBox* next = firstChildBox();
 
     while (next) {
-        ASSERT(!layoutContext().isSkippedContentForLayout(*next));
-
         RenderBox& child = *next;
         next = child.nextSiblingBox();
 
         if (child.isExcludedFromNormalLayout())
             continue; // Skip this child, since it will be positioned by the specialized subclass (fieldsets and ruby runs).
 
+        if (layoutContext().isSkippedContentForLayout(child)) {
+            ASSERT(child.isColumnSpanner());
+
+            child.clearNeedsLayout();
+            child.clearNeedsLayoutForSkippedContent();
+            continue;
+        }
+
         updateBlockChildDirtyBitsBeforeLayout(relayoutChildren, child);
 
         if (child.isOutOfFlowPositioned()) {
diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
@@ -594,6 +594,8 @@ class RenderBox : public RenderBoxModelObject {
     bool isGridItem() const { return parent() && parent()->isRenderGrid() && !isExcludedFromNormalLayout(); }
     bool isFlexItem() const { return parent() && parent()->isRenderFlexibleBox() && !isExcludedFromNormalLayout(); }
 
+    inline bool isColumnSpanner() const;
+
     virtual void adjustBorderBoxRectForPainting(LayoutRect&) { };
 
     bool shouldComputeLogicalHeightFromAspectRatio() const;
diff --git a/Source/WebCore/rendering/RenderBoxInlines.h b/Source/WebCore/rendering/RenderBoxInlines.h
@@ -68,6 +68,7 @@ inline void RenderBox::setLogicalLocation(LayoutPoint location) { setLocation(wr
 inline void RenderBox::setLogicalSize(LayoutSize size) { setSize(writingMode().isHorizontal() ? size : size.transposedSize()); }
 inline bool RenderBox::shouldTrimChildMargin(MarginTrimType type, const RenderBox& child) const { return style().marginTrim().contains(type) && isChildEligibleForMarginTrim(type, child); }
 inline bool RenderBox::stretchesToViewport() const { return document().inQuirksMode() && style().logicalHeight().isAuto() && !isFloatingOrOutOfFlowPositioned() && (isDocumentElementRenderer() || isBody()) && !shouldComputeLogicalHeightFromAspectRatio() && !isInline(); }
+inline bool RenderBox::isColumnSpanner() const { return style().columnSpan() == ColumnSpan::All; }
 
 inline LayoutPoint RenderBox::topLeftLocation() const
 {
diff --git a/Source/WebCore/rendering/RenderObject.cpp b/Source/WebCore/rendering/RenderObject.cpp
@@ -2981,6 +2981,14 @@ String RenderObject::debugDescription() const
 
 bool RenderObject::isSkippedContent() const
 {
+    if (is<RenderText>(*this))
+        return style().isSkippedRootOrSkippedContent();
+
+    if (CheckedPtr renderBox = dynamicDowncast<RenderBox>(*this); renderBox && renderBox->isColumnSpanner()) {
+        // Checking if parent is root or part of a skipped tree does not work in cases when the renderer is
+        // moved out of its original position (e.g. column spanners).
+        return renderBox->style().isSkippedRootOrSkippedContent() && !isSkippedContentRoot(*renderBox);
+    }
     return parent() && parent()->style().isSkippedRootOrSkippedContent();
 }
 

Original file line number	Diff line number	Diff line change
`@@ -68,6 +68,7 @@ inline void RenderBox::setLogicalLocation(LayoutPoint location) { setLocation(wr`
`68`	`68`	`inline void RenderBox::setLogicalSize(LayoutSize size) { setSize(writingMode().isHorizontal() ? size : size.transposedSize()); }`
`69`	`69`	`inline bool RenderBox::shouldTrimChildMargin(MarginTrimType type, const RenderBox& child) const { return style().marginTrim().contains(type) && isChildEligibleForMarginTrim(type, child); }`
`70`	`70`	`inline bool RenderBox::stretchesToViewport() const { return document().inQuirksMode() && style().logicalHeight().isAuto() && !isFloatingOrOutOfFlowPositioned() && (isDocumentElementRenderer() \|\| isBody()) && !shouldComputeLogicalHeightFromAspectRatio() && !isInline(); }`
	`71`	`+inline bool RenderBox::isColumnSpanner() const { return style().columnSpan() == ColumnSpan::All; }`
`71`	`72`
`72`	`73`	`inline LayoutPoint RenderBox::topLeftLocation() const`
`73`	`74`	`{`
Original file line number	Diff line number	Diff line change
`@@ -2981,6 +2981,14 @@ String RenderObject::debugDescription() const`
`2981`	`2981`
`2982`	`2982`	`bool RenderObject::isSkippedContent() const`
`2983`	`2983`	`{`
	`2984`	`+ if (is<RenderText>(*this))`
	`2985`	`+ return style().isSkippedRootOrSkippedContent();`
	`2986`	`+`
	`2987`	`+ if (CheckedPtr renderBox = dynamicDowncast<RenderBox>(*this); renderBox && renderBox->isColumnSpanner()) {`
	`2988`	`+ // Checking if parent is root or part of a skipped tree does not work in cases when the renderer is`
	`2989`	`+ // moved out of its original position (e.g. column spanners).`
	`2990`	`+ return renderBox->style().isSkippedRootOrSkippedContent() && !isSkippedContentRoot(*renderBox);`
	`2991`	`+ }`
`2984`	`2992`	`return parent() && parent()->style().isSkippedRootOrSkippedContent();`
`2985`	`2993`	`}`
`2986`	`2994`