mirror of
https://github.com/servo/servo.git
synced 2025-09-16 09:58:23 +01:00
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>
This commit is contained in:
parent
236e28aeab
commit
fc30a26005
6 changed files with 59 additions and 8 deletions
|
@ -849,6 +849,13 @@ impl Fragment {
|
|||
mode: StackingContextBuildMode,
|
||||
text_decorations: &Arc<Vec<FragmentTextDecoration>>,
|
||||
) {
|
||||
if self
|
||||
.base()
|
||||
.is_some_and(|base| base.flags.contains(FragmentFlags::IS_COLLAPSED))
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let containing_block = containing_block_info.get_containing_block_for_fragment(self);
|
||||
let fragment_clone = self.clone();
|
||||
match self {
|
||||
|
|
|
@ -178,6 +178,9 @@ bitflags! {
|
|||
const IS_ROOT_ELEMENT = 1 << 9;
|
||||
/// If element has propagated the overflow value to viewport.
|
||||
const PROPAGATED_OVERFLOW_TO_VIEWPORT = 1 << 10;
|
||||
/// Whether or not this is a table cell that is part of a collapsed row or column.
|
||||
/// In that case it should not be painted.
|
||||
const IS_COLLAPSED = 1 << 11;
|
||||
|
||||
}
|
||||
}
|
||||
|
|
|
@ -6,6 +6,7 @@ use app_units::{Au, MAX_AU, MIN_AU};
|
|||
use atomic_refcell::AtomicRefCell;
|
||||
use base::id::ScrollTreeNodeId;
|
||||
use base::print_tree::PrintTree;
|
||||
use euclid::Rect;
|
||||
use malloc_size_of_derive::MallocSizeOf;
|
||||
use servo_arc::Arc as ServoArc;
|
||||
use servo_geometry::f32_rect_to_au_rect;
|
||||
|
@ -291,6 +292,15 @@ impl BoxFragment {
|
|||
acc.union(&scrollable_overflow_from_child)
|
||||
});
|
||||
|
||||
// Fragments with `IS_COLLAPSED` (currently only table cells that are part of
|
||||
// table tracks with `visibility: collapse`) should not contribute to scrollable
|
||||
// overflow. This behavior matches Chrome, but not Firefox.
|
||||
// See https://github.com/w3c/csswg-drafts/issues/12689
|
||||
if self.base.flags.contains(FragmentFlags::IS_COLLAPSED) {
|
||||
self.scrollable_overflow = Some(Rect::zero());
|
||||
return;
|
||||
}
|
||||
|
||||
self.scrollable_overflow = Some(scrollable_overflow)
|
||||
}
|
||||
|
||||
|
|
|
@ -1817,10 +1817,7 @@ impl<'a> TableLayout<'a> {
|
|||
baselines.last = Some(row_end);
|
||||
}
|
||||
|
||||
if self.is_row_collapsed(row_index) {
|
||||
continue;
|
||||
}
|
||||
|
||||
let row_is_collapsed = self.is_row_collapsed(row_index);
|
||||
let table_row = self.table.rows[row_index].borrow();
|
||||
let mut row_fragment_layout = RowFragmentLayout::new(
|
||||
&table_row,
|
||||
|
@ -1856,10 +1853,6 @@ impl<'a> TableLayout<'a> {
|
|||
let column_indices = 0..self.table.size.width;
|
||||
row_fragment_layout.fragments.reserve(self.table.size.width);
|
||||
for column_index in column_indices {
|
||||
if self.is_column_collapsed(column_index) {
|
||||
continue;
|
||||
}
|
||||
|
||||
self.do_final_cell_layout(
|
||||
row_index,
|
||||
column_index,
|
||||
|
@ -1868,6 +1861,7 @@ impl<'a> TableLayout<'a> {
|
|||
&mut row_fragment_layout,
|
||||
row_group_fragment_layout.as_mut(),
|
||||
positioning_context,
|
||||
self.is_column_collapsed(column_index) || row_is_collapsed,
|
||||
);
|
||||
}
|
||||
|
||||
|
@ -1997,6 +1991,7 @@ impl<'a> TableLayout<'a> {
|
|||
row_fragment_layout: &mut RowFragmentLayout,
|
||||
row_group_fragment_layout: Option<&mut RowGroupFragmentLayout>,
|
||||
positioning_context_for_table: &mut PositioningContext,
|
||||
is_collapsed: bool,
|
||||
) {
|
||||
// The PositioningContext for cells is, in order or preference, the PositioningContext of the row,
|
||||
// the PositioningContext of the row group, or the PositioningContext of the table.
|
||||
|
@ -2046,6 +2041,7 @@ impl<'a> TableLayout<'a> {
|
|||
positioning_context,
|
||||
&self.table.style,
|
||||
&row_fragment_layout.containing_block,
|
||||
is_collapsed,
|
||||
);
|
||||
|
||||
// Make a table part rectangle relative to the row fragment for the purposes of
|
||||
|
@ -2829,6 +2825,7 @@ impl TableSlotCell {
|
|||
.sizes
|
||||
}
|
||||
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn create_fragment(
|
||||
&self,
|
||||
mut layout: CellLayout,
|
||||
|
@ -2837,6 +2834,7 @@ impl TableSlotCell {
|
|||
positioning_context: &mut PositioningContext,
|
||||
table_style: &ComputedValues,
|
||||
containing_block: &ContainingBlock,
|
||||
is_collapsed: bool,
|
||||
) -> BoxFragment {
|
||||
// This must be scoped to this function because it conflicts with euclid's Zero.
|
||||
use style::Zero as StyleZero;
|
||||
|
@ -2864,6 +2862,10 @@ impl TableSlotCell {
|
|||
base_fragment_info.flags.insert(FragmentFlags::DO_NOT_PAINT);
|
||||
}
|
||||
|
||||
if is_collapsed {
|
||||
base_fragment_info.flags.insert(FragmentFlags::IS_COLLAPSED);
|
||||
}
|
||||
|
||||
// Create an `AnonymousFragment` to move the cell contents to the cell baseline.
|
||||
let mut vertical_align_fragment_rect = cell_content_rect;
|
||||
vertical_align_fragment_rect.start_corner = LogicalVec2 {
|
||||
|
|
7
tests/wpt/meta/MANIFEST.json
vendored
7
tests/wpt/meta/MANIFEST.json
vendored
|
@ -5414,6 +5414,13 @@
|
|||
{}
|
||||
]
|
||||
],
|
||||
"table-collapsed-row-or-column-crash.html": [
|
||||
"5a8bb7e5479e93b00d7d02731aa00a48a45269be",
|
||||
[
|
||||
null,
|
||||
{}
|
||||
]
|
||||
],
|
||||
"visibility-collapse-colspan-crash.html": [
|
||||
"591fbd9a9941648f1456b41aeee1e23ed660a1ed",
|
||||
[
|
||||
|
|
22
tests/wpt/tests/css/css-tables/table-collapsed-row-or-column-crash.html
vendored
Normal file
22
tests/wpt/tests/css/css-tables/table-collapsed-row-or-column-crash.html
vendored
Normal file
|
@ -0,0 +1,22 @@
|
|||
<!DOCTYPE html>
|
||||
|
||||
<link rel="help" href="https://github.com/servo/servo/issues/37421">
|
||||
|
||||
<div style="display: table; visibility: collapse">
|
||||
<div id="scroller" style="overflow: scroll"></div>
|
||||
</div>
|
||||
|
||||
<table>
|
||||
<colgroup>
|
||||
<col style="background: green; visibility: collapse">
|
||||
</colgroup>
|
||||
<tr>
|
||||
<td><div id="scroller2" style="overflow: scroll"></div></td>
|
||||
</tr>
|
||||
</table>
|
||||
|
||||
<script>
|
||||
scroller.scrollTo();
|
||||
scroller2.scrollTo();
|
||||
</script>
|
||||
|
Loading…
Add table
Add a link
Reference in a new issue