Skip to content

Commit

Permalink
Compute the correct overflow-x and overflow-y values for table elements
Browse files Browse the repository at this point in the history 
https://bugs.webkit.org/show_bug.cgi?id=252422
rdar://problem/105850323

Reviewed by Antti Koivisto.

This patch is to align WebKit with Chromium / Blink and Firefox / Gecko.

Inspired & Test Merge: https://src.chromium.org/viewvc/blink?view=revision&revision=199640

This patch is to align WebKit to respect web-specification[1] & [2], which tells us that
display:table must treat anything other than hidden as visible and if overflow-x and
overflow-y are different, how they must be resolved. This latter part is already taken care
of, we're just allowing it to be applied to tables and table parts now.
Additionally, it allows us to have consolidated logic and simplify our code.

[1] https://drafts.csswg.org/css2/#overflow
[2] https://drafts.csswg.org/css-overflow-3/#propdef-overflow-x

* Source/WebCore/style/StyleAdjuster.cpp:
(isOverflowClipOrVisible): New static 'bool' helper function
(Adjuster::adjust): Update handling of "overflow-x" and "overflow-y" align with spec and
interop with other engines and simplify code with comments
* LayoutTests/fast/table/table-different-overflow-values.html: Add Test Case
* LayoutTests/fast/table/table-different-overflow-values-expected.txt: Add Test Case Expectation
* LayoutTests/fast/table/table-different-overflow-values-2.html: Add Test Case
* LayoutTests/fast/table/table-different-overflow-values-2-expected.txt: Add Test Case Expectation
* LayoutTests/fast/table/overflowScroll.html: Add Test Case
* LayoutTests/fast/table/overflowScroll-expected.html: Add Test Case Expectation
* LayoutTests/fast/table/overflowScroll-display-block.html: Add Test Case
* LayoutTests/fast/table/overflowScroll-display-block-expected.html: Add Test Case Expectation
* LayoutTests/imported/w3c/web-platform-tests/mathml/relations/css-styling/overflow/computed-value-001-expected.txt: Rebaselined

Canonical link: https://commits.webkit.org/266560@main
  • Loading branch information
@Ahmad-S792
Ahmad-S792 authored and Ahmad Saleem committed Aug 4, 2023
1 parent 9809c1a commit e6264ae
Show file tree
Hide file tree
Showing 10 changed files with 263 additions and 29 deletions.
17 changes: 17 additions & 0 deletions LayoutTests/fast/table/overflowScroll-display-block-expected.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!DOCTYPE html>
<style>
div
{
margin-bottom: 35px;
width: 100px;
height: 100px;
background-color: green;
}
</style>
<div style="overflow: scroll;">
</div>
<div style="overflow: scroll;">
</div>
<div style="overflow: scroll;">
</div>
<p> crbug.com/368699: When table elements have display: block, they should be allowed to have overflow:scroll. The exception is the table element, which doesn't permit anything other than visible per http://dev.w3.org/csswg/css2/visufx.html#overflow.</p>
31 changes: 31 additions & 0 deletions LayoutTests/fast/table/overflowScroll-display-block.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<!DOCTYPE html>
<style>
.scrollingElement
{
width: 100px;
height: 100px;
background-color: green;
overflow: scroll;
display: block;
}
table
{
margin-bottom: 35px;
}
</style>
<table class="scrollingElement" cellspacing=0>
<tbody>
<tr><td></td></tr>
</tbody>
</table>
<table cellspacing=0>
<tbody class="scrollingElement">
<tr><td></td></tr>
</tbody>
</table>
<table cellspacing=0>
<tbody>
<tr class="scrollingElement"><td></td></tr>
</tbody>
</table>
<p> crbug.com/368699: When table elements have display: block, they should be allowed to have overflow:scroll. The exception is the table element, which doesn't permit anything other than visible per http://dev.w3.org/csswg/css2/visufx.html#overflow.</p>
29 changes: 29 additions & 0 deletions LayoutTests/fast/table/overflowScroll-expected.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!DOCTYPE html>
<style>
.scrollingElement
{
width: 100px;
height: 100px;
background-color: green;
}
table
{
margin-bottom: 35px;
}
</style>
<table class="scrollingElement" cellspacing=0>
<tbody>
<tr><td> text text text text</td></tr>
</tbody>
</table>
<table cellspacing=0>
<tbody class="scrollingElement">
<tr><td> text text text text</td></tr>
</tbody>
</table>
<table cellspacing=0>
<tbody>
<tr class="scrollingElement"><td>text text text text</td></tr>
</tbody>
</table>
<p> crbug.com/368699: Tables, table rows and table sections don't have scrollbars when they have overflow:scroll. </p>
29 changes: 29 additions & 0 deletions LayoutTests/fast/table/overflowScroll.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!DOCTYPE html>
<style>
.scrollingElement
{
width: 100px;
height: 100px;
background-color: green;
}
table
{
margin-bottom: 35px;
}
</style>
<table class="scrollingElement" cellspacing=0>
<tbody>
<tr><td> text text text text</td></tr>
</tbody>
</table>
<table cellspacing=0>
<tbody class="scrollingElement">
<tr><td> text text text text</td></tr>
</tbody>
</table>
<table cellspacing=0>
<tbody>
<tr class="scrollingElement"><td>text text text text</td></tr>
</tbody>
</table>
<p> crbug.com/368699: Tables, table rows and table sections don't have scrollbars when they have overflow:scroll. </p>
22 changes: 22 additions & 0 deletions LayoutTests/fast/table/table-different-overflow-values-2-expected.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
This test checks for overflow equality. If overflow-x is visible then overflow-y should be visible as well.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS table1: overflow-x is either not visible or both overflow values are visible.
PASS table2: overflow-x is either not visible or both overflow values are visible.
PASS table3: overflow-x is either not visible or both overflow values are visible.
PASS inline_table1: overflow-x is either not visible or both overflow values are visible.
PASS inline_table2: overflow-x is either not visible or both overflow values are visible.
PASS inline_table3: overflow-x is either not visible or both overflow values are visible.
PASS tbody1: overflow-x is either not visible or both overflow values are visible.
PASS tbody2: overflow-x is either not visible or both overflow values are visible.
PASS tbody3: overflow-x is either not visible or both overflow values are visible.
PASS tr1: overflow-x is either not visible or both overflow values are visible.
PASS tr2: overflow-x is either not visible or both overflow values are visible.
PASS tr3: overflow-x is either not visible or both overflow values are visible.
PASS successfullyParsed is true

TEST COMPLETE


36 changes: 36 additions & 0 deletions LayoutTests/fast/table/table-different-overflow-values-2.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<!-- For tables. -->
<table id="table1" class="test" style="overflow-y: visible; overflow-x: auto"/>
<table id="table2" class="test" style="overflow-y: visible; overflow-x: scroll"/>
<table id="table3" class="test" style="overflow-y: visible; overflow-x: hidden"/>
<!-- For inline tables. -->
<table id="inline_table1" class="test" style="overflow-y: visible; overflow-x: auto; display: inline-table;"/>
<table id="inline_table2" class="test" style="overflow-y: visible; overflow-x: scroll; display: inline-table;"/>
<table id="inline_table3" class="test" style="overflow-y: visible; overflow-x: hidden; display: inline-table;"/>
<!-- For table row groups. -->
<table>
<tbody id="tbody1" class="test" style="overflow-y: visible; overflow-x: auto"/>
<tbody id="tbody2" class="test" style="overflow-y: visible; overflow-x: scroll"/>
<tbody id="tbody3" class="test" style="overflow-y: visible; overflow-x: hidden"/>
</table>
<!-- For table rows. -->
<table>
<tr id="tr1" class="test" style="overflow-y: visible; overflow-x: auto"/>
<tr id="tr2" class="test" style="overflow-y: visible; overflow-x: scroll"/>
<tr id="tr3" class="test" style="overflow-y: visible; overflow-x: hidden"/>
</table>
<script>
description("This test checks for overflow equality. If overflow-x is visible then overflow-y should be visible as well.");
var elements = document.getElementsByClassName("test");
for (i = 0; i < elements.length; ++i)
{
computedStyle = getComputedStyle(elements[i]);
overflowX = computedStyle.getPropertyValue('overflow-x');
overflowY = computedStyle.getPropertyValue('overflow-y');
if (overflowX == "visible" && overflowX != overflowY)
testFailed(elements[i].id + ": overflow-y should be visible. Was " + overflowY + ".");
else
testPassed(elements[i].id + ": overflow-x is either not visible or both overflow values are visible.");
}
</script>
22 changes: 22 additions & 0 deletions LayoutTests/fast/table/table-different-overflow-values-expected.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
This test checks for overflow equality. If overflow-x is visible then overflow-y should be visible as well.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


PASS table1: overflow-x is either not visible or both overflow values are visible.
PASS table2: overflow-x is either not visible or both overflow values are visible.
PASS table3: overflow-x is either not visible or both overflow values are visible.
PASS inline_table1: overflow-x is either not visible or both overflow values are visible.
PASS inline_table2: overflow-x is either not visible or both overflow values are visible.
PASS inline_table3: overflow-x is either not visible or both overflow values are visible.
PASS tbody1: overflow-x is either not visible or both overflow values are visible.
PASS tbody2: overflow-x is either not visible or both overflow values are visible.
PASS tbody3: overflow-x is either not visible or both overflow values are visible.
PASS tr1: overflow-x is either not visible or both overflow values are visible.
PASS tr2: overflow-x is either not visible or both overflow values are visible.
PASS tr3: overflow-x is either not visible or both overflow values are visible.
PASS successfullyParsed is true

TEST COMPLETE


36 changes: 36 additions & 0 deletions LayoutTests/fast/table/table-different-overflow-values.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<!DOCTYPE html>
<script src="../../resources/js-test.js"></script>
<!-- For tables. -->
<table id="table1" class="test" style="overflow-x: visible; overflow-y: auto"/>
<table id="table2" class="test" style="overflow-x: visible; overflow-y: scroll"/>
<table id="table3" class="test" style="overflow-x: visible; overflow-y: hidden"/>
<!-- For inline tables. -->
<table id="inline_table1" class="test" style="overflow-x: visible; overflow-y: auto; display: inline-table;"/>
<table id="inline_table2" class="test" style="overflow-x: visible; overflow-y: scroll; display: inline-table;"/>
<table id="inline_table3" class="test" style="overflow-x: visible; overflow-y: hidden; display: inline-table;"/>
<!-- For table row groups. -->
<table>
<tbody id="tbody1" class="test" style="overflow-x: visible; overflow-y: auto"/>
<tbody id="tbody2" class="test" style="overflow-x: visible; overflow-y: scroll"/>
<tbody id="tbody3" class="test" style="overflow-x: visible; overflow-y: hidden"/>
</table>
<!-- For table rows. -->
<table>
<tr id="tr1" class="test" style="overflow-x: visible; overflow-y: auto"/>
<tr id="tr2" class="test" style="overflow-x: visible; overflow-y: scroll"/>
<tr id="tr3" class="test" style="overflow-x: visible; overflow-y: hidden"/>
</table>
<script>
description("This test checks for overflow equality. If overflow-x is visible then overflow-y should be visible as well.");
var elements = document.getElementsByClassName("test");
for (i = 0; i < elements.length; ++i)
{
computedStyle = getComputedStyle(elements[i]);
overflowX = computedStyle.getPropertyValue('overflow-x');
overflowY = computedStyle.getPropertyValue('overflow-y');
if (overflowX == "visible" && overflowX != overflowY)
testFailed(elements[i].id + ": overflow-y should be visible. Was " + overflowY + ".");
else
testPassed(elements[i].id + ": overflow-x is either not visible or both overflow values are visible.");
}
</script>
2 changes: 1 addition & 1 deletion .../web-platform-tests/mathml/relations/css-styling/overflow/computed-value-001-expected.txt
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

FAIL overflow on MathML elements assert_equals: overflow on mtr expected "scroll" but got "visible"
PASS overflow on MathML elements



Expand Down
68 changes: 40 additions & 28 deletions Source/WebCore/style/StyleAdjuster.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,11 @@ OptionSet<EventListenerRegionType> Adjuster::computeEventListenerRegionTypes(con
return types;
}

static bool isOverflowClipOrVisible(Overflow overflow)
{
return overflow == Overflow::Clip || overflow == Overflow::Visible;
}

void Adjuster::adjust(RenderStyle& style, const RenderStyle* userAgentAppearanceStyle) const
{
if (style.display() == DisplayType::Contents)
Expand Down Expand Up @@ -468,42 +473,49 @@ void Adjuster::adjust(RenderStyle& style, const RenderStyle* userAgentAppearance
else
style.setTextDecorationsInEffect(style.textDecorationLine());

auto overflowReplacement = [] (Overflow overflow, Overflow overflowInOtherDimension) -> std::optional<Overflow> {
if (overflow != Overflow::Visible && overflow != Overflow::Clip) {
if (overflowInOtherDimension == Overflow::Visible)
return Overflow::Auto;
if (overflowInOtherDimension == Overflow::Clip)
return Overflow::Hidden;
}
return std::nullopt;
};
bool overflowIsClipOrVisible = isOverflowClipOrVisible(style.overflowX()) && isOverflowClipOrVisible(style.overflowX());

// If either overflow value is not visible, change to auto. Similarly if either overflow
// value is not clip, change to hidden.
// FIXME: Once we implement pagination controls, overflow-x should default to hidden
// if overflow-y is set to -webkit-paged-x or -webkit-page-y. For now, we'll let it
// default to auto so we can at least scroll through the pages.
if (auto replacement = overflowReplacement(style.overflowY(), style.overflowX()))
style.setOverflowX(*replacement);
else if (auto replacement = overflowReplacement(style.overflowX(), style.overflowY()))
style.setOverflowY(*replacement);
if (!overflowIsClipOrVisible && (style.display() == DisplayType::Table || style.display() == DisplayType::InlineTable)) {
// Tables only support overflow:hidden and overflow:visible and ignore anything else,
// see https://drafts.csswg.org/css2/#overflow. As a table is not a block
// container box the rules for resolving conflicting x and y values in CSS Overflow Module
// Level 3 do not apply. Arguably overflow-x and overflow-y aren't allowed on tables but
// all UAs allow it.
if (style.overflowX() != Overflow::Hidden)
style.setOverflowX(Overflow::Visible);
if (style.overflowY() != Overflow::Hidden)
style.setOverflowY(Overflow::Visible);
// If we are left with conflicting overflow values for the x and y axes on a table then resolve
// both to Overflow::Visible. This is interoperable behaviour but is not specced anywhere.
if (style.overflowX() == Overflow::Visible)
style.setOverflowY(Overflow::Visible);
else if (style.overflowY() == Overflow::Visible)
style.setOverflowX(Overflow::Visible);
} else if (!isOverflowClipOrVisible(style.overflowY())) {
// FIXME: Once we implement pagination controls, overflow-x should default to hidden
// if overflow-y is set to -webkit-paged-x or -webkit-page-y. For now, we'll let it
// default to auto so we can at least scroll through the pages.
// Values of 'clip' and 'visible' can only be used with 'clip' and 'visible'.
// If they aren't, 'clip' and 'visible' is reset.
if (style.overflowX() == Overflow::Visible)
style.setOverflowX(Overflow::Auto);
else if (style.overflowX() == Overflow::Clip)
style.setOverflowX(Overflow::Hidden);
} else if (!isOverflowClipOrVisible(style.overflowX())) {
// Values of 'clip' and 'visible' can only be used with 'clip' and 'visible'.
// If they aren't, 'clip' and 'visible' is reset.
if (style.overflowY() == Overflow::Visible)
style.setOverflowY(Overflow::Auto);
else if (style.overflowY() == Overflow::Clip)
style.setOverflowY(Overflow::Hidden);
}

// Call setStylesForPaginationMode() if a pagination mode is set for any non-root elements. If these
// styles are specified on a root element, then they will be incorporated in
// Style::createForm_document.
if ((style.overflowY() == Overflow::PagedX || style.overflowY() == Overflow::PagedY) && !(m_element && (m_element->hasTagName(htmlTag) || m_element->hasTagName(bodyTag))))
style.setColumnStylesFromPaginationMode(WebCore::paginationModeForRenderStyle(style));

// Table rows, sections and the table itself will support overflow:hidden and will ignore scroll/auto.
// FIXME: Eventually table sections will support auto and scroll.
if (style.display() == DisplayType::Table || style.display() == DisplayType::InlineTable
|| style.display() == DisplayType::TableRowGroup || style.display() == DisplayType::TableRow) {
if (style.overflowX() != Overflow::Visible && style.overflowX() != Overflow::Hidden && style.overflowX() != Overflow::Clip)
style.setOverflowX(Overflow::Visible);
if (style.overflowY() != Overflow::Visible && style.overflowY() != Overflow::Hidden && style.overflowY() != Overflow::Clip)
style.setOverflowY(Overflow::Visible);
}

#if ENABLE(OVERFLOW_SCROLLING_TOUCH)
// Touch overflow scrolling creates a stacking context.
if (style.hasAutoUsedZIndex() && style.useTouchOverflowScrolling() && (isScrollableOverflow(style.overflowX()) || isScrollableOverflow(style.overflowY())))
Expand Down

0 comments on commit e6264ae

Please sign in to comment.