Skip to content

Commit b50a4d9

Browse files
felipeeriaswebkit-commit-queue
felipeerias
authored and
webkit-commit-queue
committed
[css-flexbox] Table layout disregards overriding width
https://bugs.webkit.org/show_bug.cgi?id=224934 Patch by Felipe Erias <felipeerias@igalia.com> on 2021-04-25 Reviewed by Darin Adler. LayoutTests/imported/w3c: Update expectations. * web-platform-tests/css/css-flexbox/table-with-percent-intrinsic-width-expected.txt: This test now passes. Source/WebCore: If a table's parent has set an overriding width on it, that value will get preference over the one from the table's "width" property. This fixes several tests where the width assigned by the flexbox algorithm did not match the width of the table after layout. * rendering/AutoTableLayout.cpp: (WebCore::AutoTableLayout::applyPreferredLogicalWidthQuirks const): Give precedence to the element's overriding width if it has been set. * rendering/RenderTable.cpp: (WebCore::RenderTable::updateLogicalWidth): Give precedence to the element's overriding width if it has been set, instead of discarding it if it is smaller than the style width. (WebCore::RenderTable::computePreferredLogicalWidths): Take the overriding width into account when computing the preferred min/max values for the element. LayoutTests: * TestExpectations: Remove three WPT tests that now pass. * platform/glib/TestExpectations: Remove one WPT test that now passes. * platform/gtk/imported/w3c/web-platform-tests/css/css-flexbox/flex-item-contains-strict-expected.txt: Added. Canonical link: https://commits.webkit.org/237008@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276572 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 9db87f7 commit b50a4d9

9 files changed

Lines changed: 100 additions & 39 deletions

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
2021-04-25 Felipe Erias <felipeerias@igalia.com>
2+
3+
[css-flexbox] Table layout disregards overriding width
4+
https://bugs.webkit.org/show_bug.cgi?id=224934
5+
6+
Reviewed by Darin Adler.
7+
8+
* TestExpectations: Remove three WPT tests that now pass.
9+
* platform/glib/TestExpectations: Remove one WPT test that now passes.
10+
* platform/gtk/imported/w3c/web-platform-tests/css/css-flexbox/flex-item-contains-strict-expected.txt: Added.
11+
112
2021-04-24 Tim Horton <timothy_horton@apple.com>
213

314
Changing the source of a model element with clipping applied does not update the model

‎LayoutTests/TestExpectations‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3964,13 +3964,10 @@ webkit.org/b/221472 imported/w3c/web-platform-tests/css/css-flexbox/abspos/flex-
39643964

39653965
# Tables as flex items.
39663966
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-fixed-min-width-3.html [ ImageOnlyFailure ]
3967-
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-percent-width-cell-001.html [ ImageOnlyFailure ]
3968-
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-specified-width.html [ ImageOnlyFailure ]
39693967
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-column-1.html [ ImageOnlyFailure ]
39703968
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-column-2.html [ ImageOnlyFailure ]
39713969
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-inflexible-in-row-2.html [ ImageOnlyFailure ]
39723970
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-as-item-specified-height.html [ ImageOnlyFailure ]
3973-
webkit.org/b/221473 imported/w3c/web-platform-tests/css/css-flexbox/table-item-flex-percentage-width.html [ ImageOnlyFailure ]
39743971

39753972
# SVGs as flex items.
39763973
webkit.org/b/221474 imported/w3c/web-platform-tests/css/css-flexbox/svg-root-as-flex-item-002.html [ ImageOnlyFailure ]

‎LayoutTests/imported/w3c/ChangeLog‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
2021-04-25 Felipe Erias <felipeerias@igalia.com>
2+
3+
[css-flexbox] Table layout disregards overriding width
4+
https://bugs.webkit.org/show_bug.cgi?id=224934
5+
6+
Reviewed by Darin Adler.
7+
8+
Update expectations.
9+
10+
* web-platform-tests/css/css-flexbox/table-with-percent-intrinsic-width-expected.txt: This test now passes.
11+
112
2021-04-24 Antoine Quint <graouts@webkit.org>
213

314
Improve parsing and computed style of the rotate CSS property

‎LayoutTests/imported/w3c/web-platform-tests/css/css-flexbox/table-with-percent-intrinsic-width-expected.txt‎

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,4 @@
11

2-
FAIL table 1 assert_equals:
3-
<div class="container">
4-
<div class="left">
5-
</div>
6-
<table style="width:100%" data-expected-width="100">
7-
<tbody><tr><td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
8-
<td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
9-
<td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
10-
<td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
11-
<td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
12-
</tr></tbody></table>
13-
</div>
14-
width expected 100 but got 200
15-
FAIL table 2 assert_equals:
16-
<div class="container">
17-
<div class="left">
18-
</div>
19-
<table style="width:70%; flex-basis: 200px" data-expected-width="100">
20-
<tbody><tr><td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
21-
<td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
22-
<td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
23-
<td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
24-
<td data-expected-width="20"><div class="spacer"></div><div class="spacer"></div></td>
25-
</tr></tbody></table>
26-
</div>
27-
width expected 100 but got 140
2+
PASS table 1
3+
PASS table 2
284

‎LayoutTests/platform/glib/TestExpectations‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -435,8 +435,6 @@ webkit.org/b/169988 css3/filters/backdrop/backdrop-filter-with-border-radius-and
435435

436436
webkit.org/b/214682 imported/w3c/web-platform-tests/css/cssom/stylesheet-same-origin.sub.html [ Pass Failure ]
437437

438-
webkit.org/b/169007 imported/w3c/web-platform-tests/css/css-flexbox/flex-item-contains-strict.html [ Failure ]
439-
440438
webkit.org/b/216767 css3/font-feature-settings-stylistic-set.html [ ImageOnlyFailure ]
441439

442440
webkit.org/b/216853 css3/font-synthesis-small-caps.html [ ImageOnlyFailure ]

‎LayoutTests/platform/gtk/imported/w3c/web-platform-tests/css/css-flexbox/flex-item-contains-strict-expected.txt‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
Stretched:
2+
3+
Column
4+
5+
Row
6+
7+
Column
8+
9+
Row
10+
Flex-start:
11+
12+
Column
13+
14+
Row
15+
16+
Column
17+
18+
Row
19+
20+
FAIL .inline-flex 1 assert_equals:
21+
<div class="inline-flex" style="display: inline-flex; flex-direction: column;" data-expected-width="0" data-expected-height="0">
22+
<div style="contain: strict;" data-expected-width="0" data-expected-height="0">Column</div>
23+
</div>
24+
width expected 0 but got 51
25+
FAIL .inline-flex 2 assert_equals:
26+
<div class="inline-flex" data-expected-width="0" data-expected-height="0">
27+
<div style="contain: strict;" data-expected-width="0" data-expected-height="0">Row</div>
28+
</div>
29+
width expected 0 but got 31
30+
PASS .inline-flex 3
31+
PASS .inline-flex 4
32+
FAIL .inline-flex 5 assert_equals:
33+
<div class="inline-flex" style="flex-direction: column; align-items: flex-start;" data-expected-width="0" data-expected-height="0">
34+
<div style="contain: strict;" data-expected-width="0" data-expected-height="0">Column</div>
35+
</div>
36+
width expected 0 but got 51
37+
FAIL .inline-flex 6 assert_equals:
38+
<div class="inline-flex" style="align-items: flex-start;" data-expected-width="0" data-expected-height="0">
39+
<div style="contain: strict;" data-expected-width="0" data-expected-height="0">Row</div>
40+
</div>
41+
width expected 0 but got 31
42+
PASS .inline-flex 7
43+
PASS .inline-flex 8
44+

‎Source/WebCore/ChangeLog‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,24 @@
1+
2021-04-25 Felipe Erias <felipeerias@igalia.com>
2+
3+
[css-flexbox] Table layout disregards overriding width
4+
https://bugs.webkit.org/show_bug.cgi?id=224934
5+
6+
Reviewed by Darin Adler.
7+
8+
If a table's parent has set an overriding width on it, that value will get preference over the one from
9+
the table's "width" property. This fixes several tests where the width assigned by the flexbox algorithm
10+
did not match the width of the table after layout.
11+
12+
* rendering/AutoTableLayout.cpp:
13+
(WebCore::AutoTableLayout::applyPreferredLogicalWidthQuirks const): Give precedence to the element's
14+
overriding width if it has been set.
15+
16+
* rendering/RenderTable.cpp:
17+
(WebCore::RenderTable::updateLogicalWidth): Give precedence to the element's overriding width if it
18+
has been set, instead of discarding it if it is smaller than the style width.
19+
(WebCore::RenderTable::computePreferredLogicalWidths): Take the overriding width into account when
20+
computing the preferred min/max values for the element.
21+
122
2021-04-25 Tyler Wilcock <twilco.o@protonmail.com>
223

324
css-contain category missing from CSSProperties.json

‎Source/WebCore/rendering/AutoTableLayout.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,9 @@ void AutoTableLayout::computeIntrinsicLogicalWidths(LayoutUnit& minWidth, Layout
280280

281281
void AutoTableLayout::applyPreferredLogicalWidthQuirks(LayoutUnit& minWidth, LayoutUnit& maxWidth) const
282282
{
283-
Length tableLogicalWidth = m_table->style().logicalWidth();
284-
if (tableLogicalWidth.isFixed() && tableLogicalWidth.isPositive())
283+
if (m_table->hasOverridingLogicalWidth())
284+
minWidth = maxWidth = std::max(minWidth, m_table->overridingLogicalWidth());
285+
else if (auto tableLogicalWidth = m_table->style().logicalWidth(); tableLogicalWidth.isFixed() && tableLogicalWidth.isPositive())
285286
minWidth = maxWidth = std::max(minWidth, LayoutUnit(tableLogicalWidth.value()));
286287
}
287288

‎Source/WebCore/rendering/RenderTable.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,9 @@ void RenderTable::updateLogicalWidth()
261261
LayoutUnit containerWidthInInlineDirection = hasPerpendicularContainingBlock ? perpendicularContainingBlockLogicalHeight() : availableLogicalWidth;
262262

263263
Length styleLogicalWidth = style().logicalWidth();
264-
if ((styleLogicalWidth.isSpecified() && styleLogicalWidth.isPositive()) || styleLogicalWidth.isIntrinsic())
264+
if (hasOverridingLogicalWidth())
265+
setLogicalWidth(overridingLogicalWidth());
266+
else if ((styleLogicalWidth.isSpecified() && styleLogicalWidth.isPositive()) || styleLogicalWidth.isIntrinsic())
265267
setLogicalWidth(convertStyleLogicalWidthToComputedWidth(styleLogicalWidth, containerWidthInInlineDirection));
266268
else {
267269
// Subtract out any fixed margins from our available width for auto width tables.
@@ -287,11 +289,6 @@ void RenderTable::updateLogicalWidth()
287289
setLogicalWidth(std::min(availableContentLogicalWidth, maxWidth));
288290
}
289291

290-
// Our parent might have set an override content logical width on us, so we must respect it. This
291-
// is how flexbox containers flex or stretch us.
292-
if (hasOverridingLogicalWidth())
293-
setLogicalWidth(std::max(logicalWidth(), overridingLogicalWidth()));
294-
295292
// Ensure we aren't bigger than our max-width style.
296293
Length styleMaxLogicalWidth = style().logicalMaxWidth();
297294
if ((styleMaxLogicalWidth.isSpecified() && !styleMaxLogicalWidth.isNegative()) || styleMaxLogicalWidth.isIntrinsic()) {
@@ -845,6 +842,11 @@ void RenderTable::computePreferredLogicalWidths()
845842
for (unsigned i = 0; i < m_captions.size(); i++)
846843
m_minPreferredLogicalWidth = std::max(m_minPreferredLogicalWidth, m_captions[i]->minPreferredLogicalWidth());
847844

845+
if (hasOverridingLogicalWidth()) {
846+
m_minPreferredLogicalWidth = std::max(m_minPreferredLogicalWidth, overridingLogicalWidth());
847+
m_maxPreferredLogicalWidth = std::max(m_maxPreferredLogicalWidth, overridingLogicalWidth());
848+
}
849+
848850
auto& styleToUse = style();
849851
// FIXME: This should probably be checking for isSpecified since you should be able to use percentage or calc values for min-width.
850852
if (styleToUse.logicalMinWidth().isFixed() && styleToUse.logicalMinWidth().value() > 0) {

0 commit comments

Comments
 (0)