Skip to content

Commit 50a0471

Browse files
Ahmad-S792Ahmad Saleem
Ahmad-S792
authored and
Ahmad Saleem
committed
Distribute table height to tbody sections instead of first section only
https://bugs.webkit.org/show_bug.cgi?id=306498 rdar://169154677 Reviewed by Alan Baradlay. When a table has a specified height greater than its content, the extra height should be distributed among tbody sections, not thead or tfoot. Previously, all extra height went to the first body section only, which caused incorrect rendering when tables had multiple tbody elements or when thead/tfoot were present. This change implements proper height distribution: 1. Extra height is distributed only to tbody sections, allowing thead and tfoot to maintain their intrinsic content-based heights 2. When multiple tbody sections exist, height is distributed either proportionally based on their intrinsic heights, or equally if all are empty 3. Empty sections can now receive height distribution (removed early return that prevented this) * Source/WebCore/rendering/RenderTable.cpp: (WebCore::RenderTable::distributeExtraLogicalHeight): * Source/WebCore/rendering/RenderTableSection.cpp: (WebCore::RenderTableSection::distributeExtraLogicalHeightToRows): > Progressions: * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/height-distribution/extra-height-given-to-all-row-groups-003-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-tables/tentative/table-height-redistribution-expected.txt: Canonical link: https://commits.webkit.org/306457@main
1 parent 941a729 commit 50a0471

File tree

5 files changed

+82
-61
lines changed

5 files changed

+82
-61
lines changed

‎LayoutTests/TestExpectations‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6754,8 +6754,6 @@ imported/w3c/web-platform-tests/css/css-tables/fixup-dynamic-anonymous-inline-ta
67546754
imported/w3c/web-platform-tests/css/css-tables/fixup-dynamic-anonymous-inline-table-002.html [ ImageOnlyFailure ]
67556755
imported/w3c/web-platform-tests/css/css-tables/fixup-dynamic-anonymous-inline-table-003.html [ ImageOnlyFailure ]
67566756
imported/w3c/web-platform-tests/css/css-tables/fixup-dynamic-anonymous-table-001.html [ ImageOnlyFailure ]
6757-
imported/w3c/web-platform-tests/css/css-tables/height-distribution/extra-height-given-to-all-row-groups-001.html [ ImageOnlyFailure ]
6758-
imported/w3c/web-platform-tests/css/css-tables/height-distribution/extra-height-given-to-all-row-groups-005.html [ ImageOnlyFailure ]
67596757
imported/w3c/web-platform-tests/css/css-tables/height-distribution/percentage-sizing-of-table-cell-children-005.html [ ImageOnlyFailure ]
67606758
imported/w3c/web-platform-tests/css/css-tables/height-distribution/percentage-sizing-of-table-cell-children-006.html [ ImageOnlyFailure ]
67616759
imported/w3c/web-platform-tests/css/css-tables/min-max-size-table-content-box.html [ ImageOnlyFailure ]
@@ -6767,8 +6765,6 @@ imported/w3c/web-platform-tests/css/css-tables/tentative/paint/background-image-
67676765
imported/w3c/web-platform-tests/css/css-tables/tentative/paint/background-image-row.html [ ImageOnlyFailure ]
67686766
imported/w3c/web-platform-tests/css/css-tables/tentative/paint/collapsed-border-large-cell.html [ ImageOnlyFailure ]
67696767
imported/w3c/web-platform-tests/css/css-tables/tentative/paint/overflow-hidden-table.html [ ImageOnlyFailure ]
6770-
imported/w3c/web-platform-tests/css/css-tables/tentative/section-no-tbody-fixed-distribution.html [ ImageOnlyFailure ]
6771-
imported/w3c/web-platform-tests/css/css-tables/tentative/section-no-tbody-percent-distribution.html [ ImageOnlyFailure ]
67726768
imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-colspan-003.html [ ImageOnlyFailure ]
67736769
imported/w3c/web-platform-tests/css/css-tables/visibility-collapse-rowspan-005.html [ ImageOnlyFailure ]
67746770
imported/w3c/web-platform-tests/css/css-tables/whitespace-001.html [ ImageOnlyFailure ]

‎LayoutTests/imported/w3c/web-platform-tests/css/css-tables/height-distribution/extra-height-given-to-all-row-groups-003-expected.txt‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,3 @@
11

2-
FAIL #theTable 1 assert_equals:
3-
<table id="theTable" style="height:100px">
4-
<tbody data-expected-height="50">
5-
<tr>
6-
<td><div></div></td>
7-
</tr>
8-
</tbody>
9-
<tbody data-expected-height="50">
10-
<tr>
11-
<td><div></div></td>
12-
</tr>
13-
</tbody>
14-
</table>
15-
height expected 50 but got 90
2+
PASS #theTable 1
163

‎LayoutTests/imported/w3c/web-platform-tests/css/css-tables/tentative/table-height-redistribution-expected.txt‎

Lines changed: 20 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ b:100
5151
non-empty tbody
5252
b:100
5353
non-empty thead
54-
h:0
54+
h:100
5555
THEAD TBODY
5656

5757
empty thead, empty tbody
@@ -85,33 +85,33 @@ Multiple TBODY
8585
Legacy does not distribute any heights when tr is empty.
8686
Legacy always distributes everything to 1st tbody.
8787
2 tbody
88-
b:0
89-
b:0
88+
b:50
89+
b:50
9090
2 tbody, with non-empty tds
9191
Legacy distributes everything to 1st tbody
92-
b:82
93-
b:18
92+
b:50
93+
b:50
9494
x
9595
x
9696
2 tbody, 40%, auto, no td
9797
FF: distributes everything to auto when empty. Legacy does not distribute
98-
b:0
99-
b:0
98+
b:50
99+
b:50
100100
2 tbody, 40%, auto, non-empty td
101-
b:82
102-
b:18
101+
b:50
102+
b:50
103103
x
104104
x
105105
2 tbody, 40px, auto, non-empty td
106106
FF gets confused with 2nd body placement
107-
b:82
108-
b:18
107+
b:50
108+
b:50
109109
x
110110
x
111111
2 tbody, 40%, 40px, non-empty td
112112
FF splits evenly
113-
b:82
114-
b:18
113+
b:50
114+
b:50
115115
x
116116
x
117117
Sized THEAD/TBODY
@@ -160,53 +160,39 @@ PASS table 12
160160
PASS table 13
161161
PASS table 14
162162
PASS table 15
163-
FAIL table 16 assert_equals:
164-
<table data-expected-height="100">
165-
<thead data-expected-height="100"><tr></tr></thead>
166-
</table>
167-
height expected 100 but got 0
163+
PASS table 16
168164
PASS table 17
169165
PASS table 18
170166
PASS table 19
171167
PASS table 20
172168
PASS table 21
173169
PASS table 22
174-
FAIL table 23 assert_equals:
175-
<table data-expected-height="100">
176-
<tbody data-expected-height="50"><tr></tr></tbody>
177-
<tbody data-expected-height="50"><tr></tr></tbody>
178-
</table>
179-
height expected 100 but got 0
180-
FAIL table 24 assert_equals:
181-
<table data-expected-height="100">
182-
<tbody data-expected-height="50"><tr><td>x</td></tr></tbody>
183-
<tbody data-expected-height="50"><tr><td>x</td></tr></tbody>
184-
</table>
185-
height expected 50 but got 82
170+
PASS table 23
171+
PASS table 24
186172
FAIL table 25 assert_equals:
187173
<table data-expected-height="100">
188174
<tbody style="height:40%" data-expected-height="40"><tr></tr></tbody>
189175
<tbody data-expected-height="60"><tr></tr></tbody>
190176
</table>
191-
height expected 100 but got 0
177+
height expected 40 but got 50
192178
FAIL table 26 assert_equals:
193179
<table data-expected-height="100">
194180
<tbody style="height:40%" data-expected-height="40"><tr><td>x</td></tr></tbody>
195181
<tbody data-expected-height="60"><tr><td>x</td></tr></tbody>
196182
</table>
197-
height expected 40 but got 82
183+
height expected 40 but got 50
198184
FAIL table 27 assert_equals:
199185
<table data-expected-height="100">
200186
<tbody style="height:40px" data-expected-height="40"><tr><td>x</td></tr></tbody>
201187
<tbody data-expected-height="60" data-offset-y="40"><tr><td>x</td></tr></tbody>
202188
</table>
203-
height expected 40 but got 82
189+
height expected 40 but got 50
204190
FAIL table 28 assert_equals:
205191
<table data-expected-height="100">
206192
<tbody style="height:40%" data-expected-height="40"><tr><td>x</td></tr></tbody>
207193
<tbody style="height:40px" data-expected-height="60" data-offset-y="40"><tr><td>x</td></tr></tbody>
208194
</table>
209-
height expected 40 but got 82
195+
height expected 40 but got 50
210196
FAIL table 29 assert_equals:
211197
<table data-expected-height="100">
212198
<thead data-expected-height="20" style="height:20px">

‎Source/WebCore/rendering/RenderTable.cpp‎

Lines changed: 54 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -448,13 +448,61 @@ void RenderTable::distributeExtraLogicalHeight(LayoutUnit extraLogicalHeight)
448448
if (extraLogicalHeight <= 0)
449449
return;
450450

451-
// FIXME: Distribute the extra logical height between all table sections instead of giving it all to the first one.
452-
if (RenderTableSection* section = firstBody())
453-
extraLogicalHeight -= section->distributeExtraLogicalHeightToRows(extraLogicalHeight);
451+
// Collect tbody sections separately from thead/tfoot
452+
Vector<RenderTableSection*> tbodySections;
453+
for (CheckedPtr section = topSection(); section; section = sectionBelow(section)) {
454+
// A section is a tbody if it's not the header or footer.
455+
if (section != m_head.get() && section != m_foot.get())
456+
tbodySections.append(section);
457+
}
458+
459+
// If we have tbody sections, distribute all extra height to them only.
460+
// This matches the expected behavior where thead/tfoot keep their intrinsic size.
461+
if (!tbodySections.isEmpty()) {
462+
LayoutUnit totalTBodyHeight = 0;
463+
for (auto& section : tbodySections)
464+
totalTBodyHeight += section->logicalHeight();
465+
466+
if (totalTBodyHeight > 0) {
467+
// Distribute proportionally among tbodies based on their intrinsic heights
468+
LayoutUnit remainingHeight = extraLogicalHeight;
469+
for (size_t sectionIndex = 0; sectionIndex < tbodySections.size(); ++sectionIndex) {
470+
CheckedPtr section = tbodySections[sectionIndex];
471+
LayoutUnit sectionHeight = section->logicalHeight();
472+
473+
LayoutUnit extraHeightForSection;
474+
if (sectionIndex == tbodySections.size() - 1)
475+
extraHeightForSection = remainingHeight;
476+
else {
477+
extraHeightForSection = (extraLogicalHeight * sectionHeight) / totalTBodyHeight;
478+
remainingHeight -= extraHeightForSection;
479+
}
480+
481+
section->distributeExtraLogicalHeightToRows(extraHeightForSection);
482+
}
483+
} else {
484+
// All tbodies are empty - distribute equally among them
485+
LayoutUnit remainingHeight = extraLogicalHeight;
486+
LayoutUnit heightPerSection = extraLogicalHeight / tbodySections.size();
487+
488+
for (size_t sectionIndex = 0; sectionIndex < tbodySections.size(); ++sectionIndex) {
489+
LayoutUnit extraHeightForSection;
490+
if (sectionIndex == tbodySections.size() - 1)
491+
extraHeightForSection = remainingHeight;
492+
else {
493+
extraHeightForSection = heightPerSection;
494+
remainingHeight -= extraHeightForSection;
495+
}
496+
497+
tbodySections[sectionIndex]->distributeExtraLogicalHeightToRows(extraHeightForSection);
498+
}
499+
}
500+
return;
501+
}
454502

455-
// FIXME: We really would like to enable this ASSERT to ensure that all the extra space has been distributed.
456-
// However our current distribution algorithm does not round properly and thus we can have some remaining height.
457-
// ASSERT(!topSection() || !extraLogicalHeight);
503+
// No tbody sections - fall back to distributing to thead or tfoot.
504+
if (CheckedPtr section = topSection())
505+
section->distributeExtraLogicalHeightToRows(extraLogicalHeight);
458506
}
459507

460508
void RenderTable::simplifiedNormalFlowLayout()

‎Source/WebCore/rendering/RenderTableSection.cpp‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -483,9 +483,6 @@ LayoutUnit RenderTableSection::distributeExtraLogicalHeightToRows(LayoutUnit ext
483483
if (!totalRows)
484484
return extraLogicalHeight;
485485

486-
if (!m_rowPos[totalRows] && nextSibling())
487-
return extraLogicalHeight;
488-
489486
unsigned autoRowsCount = 0;
490487
int totalPercent = 0;
491488
for (unsigned r = 0; r < totalRows; r++) {
@@ -495,6 +492,13 @@ LayoutUnit RenderTableSection::distributeExtraLogicalHeightToRows(LayoutUnit ext
495492
totalPercent += percentageLogicalHeight->value;
496493
}
497494

495+
// If this section has no intrinsic height and there are other sections,
496+
// distribute based on whether we have percentage/auto rows that can grow.
497+
if (!m_rowPos[totalRows] && nextSibling()) {
498+
if (!autoRowsCount && !totalPercent)
499+
return extraLogicalHeight;
500+
}
501+
498502
LayoutUnit remainingExtraLogicalHeight = extraLogicalHeight;
499503
distributeExtraLogicalHeightToPercentRows(remainingExtraLogicalHeight, totalPercent);
500504
distributeExtraLogicalHeightToAutoRows(remainingExtraLogicalHeight, autoRowsCount);

0 commit comments

Comments
 (0)