Content-Length: 605951 | pFad | http://github.com/servo/servo/commit/#start-of-content

68B5BA5A layout: Lay out collapsed table rows and columns, but don't paint the… · servo/servo@fc30a26 · GitHub
Skip to content

Commit fc30a26

Browse files
mrobinsonmrobinson
authored
layout: Lay out collapsed table rows and columns, but don't paint them (#39027)
It's expected that script queries be able to interact with collapsed table rows and columns, so this change starts laying them out. They still do not affect table dimensions, nor are they painted. This does not fix all interaction with collapsed rows and columns. For instance, setting scroll offsets of contained scrolling nodes does not work properly. It does fix the panic though, which is the most important thing. Testing: this change includes a new WPT crash test. Fixes: #37421. Signed-off-by: Martin Robinson <mrobinson@igalia.com>
1 parent 236e28a commit fc30a26

File tree

6 files changed

+59
-8
lines changed

6 files changed

+59
-8
lines changed

‎components/layout/display_list/stacking_context.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,13 @@ impl Fragment {
849849
mode: StackingContextBuildMode,
850850
text_decorations: &Arc<Vec<FragmentTextDecoration>>,
851851
) {
852+
if self
853+
.base()
854+
.is_some_and(|base| base.flags.contains(FragmentFlags::IS_COLLAPSED))
855+
{
856+
return;
857+
}
858+
852859
let containing_block = containing_block_info.get_containing_block_for_fragment(self);
853860
let fragment_clone = self.clone();
854861
match self {

‎components/layout/fragment_tree/base_fragment.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,9 @@ bitflags! {
178178
const IS_ROOT_ELEMENT = 1 << 9;
179179
//github.com/ If element has propagated the overflow value to viewport.
180180
const PROPAGATED_OVERFLOW_TO_VIEWPORT = 1 << 10;
181+
//github.com/ Whether or not this is a table cell that is part of a collapsed row or column.
182+
//github.com/ In that case it should not be painted.
183+
const IS_COLLAPSED = 1 << 11;
181184

182185
}
183186
}

‎components/layout/fragment_tree/box_fragment.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use app_units::{Au, MAX_AU, MIN_AU};
66
use atomic_refcell::AtomicRefCell;
77
use base::id::ScrollTreeNodeId;
88
use base::print_tree::PrintTree;
9+
use euclid::Rect;
910
use malloc_size_of_derive::MallocSizeOf;
1011
use servo_arc::Arc as ServoArc;
1112
use servo_geometry::f32_rect_to_au_rect;
@@ -291,6 +292,15 @@ impl BoxFragment {
291292
acc.union(&scrollable_overflow_from_child)
292293
});
293294

295+
// Fragments with `IS_COLLAPSED` (currently only table cells that are part of
296+
// table tracks with `visibility: collapse`) should not contribute to scrollable
297+
// overflow. This behavior matches Chrome, but not Firefox.
298+
// See https://github.com/w3c/csswg-drafts/issues/12689
299+
if self.base.flags.contains(FragmentFlags::IS_COLLAPSED) {
300+
self.scrollable_overflow = Some(Rect::zero());
301+
return;
302+
}
303+
294304
self.scrollable_overflow = Some(scrollable_overflow)
295305
}
296306

‎components/layout/table/layout.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1817,10 +1817,7 @@ impl<'a> TableLayout<'a> {
18171817
baselines.last = Some(row_end);
18181818
}
18191819

1820-
if self.is_row_collapsed(row_index) {
1821-
continue;
1822-
}
1823-
1820+
let row_is_collapsed = self.is_row_collapsed(row_index);
18241821
let table_row = self.table.rows[row_index].borrow();
18251822
let mut row_fragment_layout = RowFragmentLayout::new(
18261823
&table_row,
@@ -1856,10 +1853,6 @@ impl<'a> TableLayout<'a> {
18561853
let column_indices = 0..self.table.size.width;
18571854
row_fragment_layout.fragments.reserve(self.table.size.width);
18581855
for column_index in column_indices {
1859-
if self.is_column_collapsed(column_index) {
1860-
continue;
1861-
}
1862-
18631856
self.do_final_cell_layout(
18641857
row_index,
18651858
column_index,
@@ -1868,6 +1861,7 @@ impl<'a> TableLayout<'a> {
18681861
&mut row_fragment_layout,
18691862
row_group_fragment_layout.as_mut(),
18701863
positioning_context,
1864+
self.is_column_collapsed(column_index) || row_is_collapsed,
18711865
);
18721866
}
18731867

@@ -1997,6 +1991,7 @@ impl<'a> TableLayout<'a> {
19971991
row_fragment_layout: &mut RowFragmentLayout,
19981992
row_group_fragment_layout: Option<&mut RowGroupFragmentLayout>,
19991993
positioning_context_for_table: &mut PositioningContext,
1994+
is_collapsed: bool,
20001995
) {
20011996
// The PositioningContext for cells is, in order or preference, the PositioningContext of the row,
20021997
// the PositioningContext of the row group, or the PositioningContext of the table.
@@ -2046,6 +2041,7 @@ impl<'a> TableLayout<'a> {
20462041
positioning_context,
20472042
&self.table.style,
20482043
&row_fragment_layout.containing_block,
2044+
is_collapsed,
20492045
);
20502046

20512047
// Make a table part rectangle relative to the row fragment for the purposes of
@@ -2829,6 +2825,7 @@ impl TableSlotCell {
28292825
.sizes
28302826
}
28312827

2828+
#[allow(clippy::too_many_arguments)]
28322829
fn create_fragment(
28332830
&self,
28342831
mut layout: CellLayout,
@@ -2837,6 +2834,7 @@ impl TableSlotCell {
28372834
positioning_context: &mut PositioningContext,
28382835
table_style: &ComputedValues,
28392836
containing_block: &ContainingBlock,
2837+
is_collapsed: bool,
28402838
) -> BoxFragment {
28412839
// This must be scoped to this function because it conflicts with euclid's Zero.
28422840
use style::Zero as StyleZero;
@@ -2864,6 +2862,10 @@ impl TableSlotCell {
28642862
base_fragment_info.flags.insert(FragmentFlags::DO_NOT_PAINT);
28652863
}
28662864

2865+
if is_collapsed {
2866+
base_fragment_info.flags.insert(FragmentFlags::IS_COLLAPSED);
2867+
}
2868+
28672869
// Create an `AnonymousFragment` to move the cell contents to the cell baseline.
28682870
let mut vertical_align_fragment_rect = cell_content_rect;
28692871
vertical_align_fragment_rect.start_corner = LogicalVec2 {

‎tests/wpt/meta/MANIFEST.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5414,6 +5414,13 @@
54145414
{}
54155415
]
54165416
],
5417+
"table-collapsed-row-or-column-crash.html": [
5418+
"5a8bb7e5479e93b00d7d02731aa00a48a45269be",
5419+
[
5420+
null,
5421+
{}
5422+
]
5423+
],
54175424
"visibility-collapse-colspan-crash.html": [
54185425
"591fbd9a9941648f1456b41aeee1e23ed660a1ed",
54195426
[

‎tests/wpt/tests/css/css-tables/table-collapsed-row-or-column-crash.html

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<!DOCTYPE html>
2+
3+
<link rel="help" href="https://github.com/servo/servo/issues/37421">
4+
5+
<div style="display: table; visibility: collapse">
6+
<div id="scroller" style="overflow: scroll"></div>
7+
</div>
8+
9+
<table>
10+
<colgroup>
11+
<col style="background: green; visibility: collapse">
12+
</colgroup>
13+
<tr>
14+
<td><div id="scroller2" style="overflow: scroll"></div></td>
15+
</tr>
16+
</table>
17+
18+
<script>
19+
scroller.scrollTo();
20+
scroller2.scrollTo();
21+
</script>
22+

0 commit comments

Comments
 (0)








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/servo/servo/commit/#start-of-content

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy