Skip to content

Commit

Permalink
[content-visibility] Property should not apply to tables
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=264163

Reviewed by Tim Nguyen.

The content-visibility property applied to elements for which size containment can apply [1].
However, size containment does not apply to tables [2], so change the code to make content-visibility
not have a direct effect on tables.

[1] https://drafts.csswg.org/css-contain/#content-visibility
[2] https://drafts.csswg.org/css-contain/#containment-size

* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-094-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-094.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-095-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-contain/content-visibility/content-visibility-095.html: Added.
* Source/WebCore/rendering/RenderElementInlines.h:
(WebCore::RenderElement::shouldApplyLayoutOrPaintContainment const):
(WebCore::RenderElement::shouldApplyPaintContainment const):
* Source/WebCore/rendering/style/RenderStyleInlines.h:
(WebCore::isSkippedContentRoot):

Canonical link: https://commits.webkit.org/270888@main
  • Loading branch information
rwlbuis committed Nov 17, 2023
1 parent f2c812f commit eea0af3
Show file tree
Hide file tree
Showing 6 changed files with 149 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!doctype HTML>
<html>
<meta charset="utf8">
<title>Content Visibility: hidden table types</title>
<link rel="author" title="Rob Buis" href="mailto:rbuis@igalia.com">

<style>
table {
width: 150px;
height: 150px;
background: lightblue;
}
</style>

<table id=table>
<tr id=tr>
<td id=td>
<div>Test passes if this text is visible.</div>
</td>
</tr>
</table>

</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<!doctype HTML>
<html class="reftest-wait">
<meta charset="utf8">
<title>Content Visibility: hidden table types</title>
<link rel="author" title="Rob Buis" href="mailto:rbuis@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
<link rel="match" href="container-ref.html">
<meta name="assert" content="content-visibility effect on hidden table types">

<script src="/common/reftest-wait.js"></script>

<style>
table {
width: 150px;
height: 150px;
background: lightblue;
}
.hidden {
content-visibility: hidden;
}
</style>

<table id=table>
<caption id=caption>
<div>Test fails if this text is visible.</div>
</caption>
<tr id=tr>
<td id=td>
<div>Test passes if this text is visible.</div>
</td>
</tr>
</table>

<script>
function runTest() {
document.getElementById("table").classList.add("hidden");
document.getElementById("tr").classList.add("hidden");
document.getElementById("caption").classList.add("hidden");
requestAnimationFrame(takeScreenshot);
}

window.onload = () => requestAnimationFrame(runTest);
</script>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<!doctype HTML>
<html>
<meta charset="utf8">
<title>CSS Content Visibility: container (reference)</title>
<link rel="author" title="Vladimir Levin" href="mailto:vmpstr@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">

<style>
#table {
width: 150px;
height: 150px;
background: lightblue;
}
#positioned {
position: absolute;
}
</style>

<table id=table>
<td>
<div id=positioned>Test passes if this text is visible.</div>
</td>
</table>

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<!doctype HTML>
<html class="reftest-wait">
<meta charset="utf8">
<title>Content Visibility: hidden table with positioned child</title>
<link rel="author" title="Rob Buis" href="mailto:rbuis@igalia.com">
<link rel="help" href="https://drafts.csswg.org/css-contain/#content-visibility">
<link rel="match" href="container-ref.html">
<meta name="assert" content="content-visibility hidden table does paint the subtree">

<script src="/common/reftest-wait.js"></script>

<style>
table {
width: 150px;
height: 150px;
background: lightblue;
}
#positioned {
position: absolute;
}
.hidden {
content-visibility: hidden;
}
</style>

<table id=table>
<caption id=caption>
<div id=positioned>Test fails if this text is visible.</div>
</caption>
<tr id=tr>
<td id=td>
<div id=positioned>Test passes if this text is visible.</div>
</td>
</tr>
</table>

<script>
function runTest() {
document.getElementById("table").classList.add("hidden");
document.getElementById("tr").classList.add("hidden");
document.getElementById("caption").classList.add("hidden");
requestAnimationFrame(takeScreenshot);
}

window.onload = () => requestAnimationFrame(runTest);
</script>
</html>
5 changes: 3 additions & 2 deletions Source/WebCore/rendering/RenderElementInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,12 @@ inline bool RenderElement::shouldApplyLayoutOrPaintContainment(bool containsAcco

inline bool RenderElement::shouldApplyLayoutOrPaintContainment() const
{
return shouldApplyLayoutOrPaintContainment(style().containsLayoutOrPaint() || style().contentVisibility() != ContentVisibility::Visible);
return shouldApplyLayoutOrPaintContainment(style().containsLayoutOrPaint()) || shouldApplySizeOrStyleContainment(style().contentVisibility() != ContentVisibility::Visible);
}

inline bool RenderElement::shouldApplyPaintContainment() const
{
return shouldApplyLayoutOrPaintContainment(style().containsPaint() || style().contentVisibility() != ContentVisibility::Visible);
return shouldApplyLayoutOrPaintContainment(style().containsPaint()) || shouldApplySizeOrStyleContainment(style().contentVisibility() != ContentVisibility::Visible);
}

inline bool RenderElement::shouldApplySizeContainment() const
Expand All @@ -108,6 +108,7 @@ inline bool RenderElement::shouldApplySizeOrInlineSizeContainment() const
return isSkippedContentRoot() || shouldApplySizeOrStyleContainment(style().containsSizeOrInlineSize());
}

// FIXME: try to avoid duplication with isSkippedContentRoot.
inline bool RenderElement::shouldApplySizeOrStyleContainment(bool containsAccordingToStyle) const
{
return containsAccordingToStyle && (!isInline() || isAtomicInlineLevelBox()) && !isRenderRubyText() && (!isTablePart() || isRenderTableCaption()) && !isRenderTable();
Expand Down
17 changes: 8 additions & 9 deletions Source/WebCore/rendering/style/RenderStyleInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -937,17 +937,16 @@ inline TypographicMode RenderStyle::typographicMode() const

inline bool isSkippedContentRoot(const RenderStyle& style, const Element* element)
{
switch (style.contentVisibility()) {
case ContentVisibility::Visible:
if (style.contentVisibility() == ContentVisibility::Visible)
return false;
case ContentVisibility::Hidden:
// FIXME (https://bugs.webkit.org/show_bug.cgi?id=265020): check more display types.
// FIXME: try to avoid duplication with shouldApplySizeOrStyleContainment.
if (style.isDisplayTableOrTablePart() && style.display() != DisplayType::TableCaption)
return false;
if (style.contentVisibility() == ContentVisibility::Hidden)
return true;
case ContentVisibility::Auto:
return element && !element->isRelevantToUser();
};

ASSERT_NOT_REACHED();
return false;
ASSERT(style.contentVisibility() == ContentVisibility::Auto);
return element && !element->isRelevantToUser();
}

inline float adjustFloatForAbsoluteZoom(float value, const RenderStyle& style)
Expand Down

0 comments on commit eea0af3

Please sign in to comment.