From eb70c39474351fbaf5dc8907a7f82f36a337a061 Mon Sep 17 00:00:00 2001 From: Nico Burns Date: Mon, 24 Aug 2026 22:03:16 +0000 Subject: [PATCH 1/2] =?UTF-8?q?Implement=20anonymous=20table=20box=20const?= =?UTF-8?q?ruction=20fixups=20(CSS=202.2=20=C2=A717.2.1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Generalize anonymous box creation to AnonKind (block/table/row/cell) styled via stylo's precomputed ServoAnonymous* pseudo-elements, with UA rules in default.css. - Run-based fixups inside tables: consecutive misplaced children share a single anonymous cell; cells outside a row share an anonymous row. - Missing-parent fixups: runs of table-internal boxes outside a table are wrapped in an anonymous table box, routed through normal table construction. Whitespace between table-internal boxes is discarded. --- packages/blitz-dom/src/layout/construct.rs | 89 +++- packages/blitz-dom/src/layout/table.rs | 434 +++++++++++--------- tests/blitz-tests/tests/table_box_fixups.rs | 272 ++++++++++++ 3 files changed, 596 insertions(+), 199 deletions(-) create mode 100644 tests/blitz-tests/tests/table_box_fixups.rs diff --git a/packages/blitz-dom/src/layout/construct.rs b/packages/blitz-dom/src/layout/construct.rs index 2e6e8a1dd..ce9f02a63 100644 --- a/packages/blitz-dom/src/layout/construct.rs +++ b/packages/blitz-dom/src/layout/construct.rs @@ -41,7 +41,6 @@ const DUMMY_NAME: QualName = qual_name!("div", html); /// The kind of anonymous box to generate, determining which precomputed /// pseudo-element supplies its style (and hence its `display`). #[derive(Debug, Copy, Clone, PartialEq, Eq)] -#[allow(dead_code, reason = "table variants are used by upcoming table fixups")] pub(crate) enum AnonKind { Block, Table, @@ -136,6 +135,10 @@ pub(crate) enum ConstructionTaskResultData { pub(crate) struct LayoutChildren { pub(crate) children: ThinVec, pub(crate) anonymous_block_id: Option, + /// The currently-open anonymous table wrapping a run of table-internal + /// children occurring outside a table (missing-parent fixup, CSS 2.2 + /// §17.2.1). Closed by any non-table-internal sibling. + pub(crate) anonymous_table_id: Option, /// All anonymous blocks created while collecting these layout children. /// /// These are recorded on the container node so they can be deallocated the @@ -182,6 +185,7 @@ impl LayoutChildren { } self.anonymous_block_id = None; + self.anonymous_table_id = None; } fn push_wrapped( @@ -190,6 +194,15 @@ impl LayoutChildren { child_id: NodeId, doc: &mut BaseDocument, ) { + // A wrapped (text/inline) child terminates any open run of + // table-internal children, except that whitespace between + // table-internal boxes is discarded (CSS 2.2 §17.2.1). + if self.anonymous_table_id.is_some() { + if doc.nodes[child_id].is_whitespace_node() { + return; + } + self.anonymous_table_id = None; + } if self.anonymous_block_id.is_none() { self.create_anonymous_block(container_node_id, doc); } @@ -198,6 +211,27 @@ impl LayoutChildren { .push(child_id); } + /// Append a table-internal child (row, row group, cell, ...) occurring + /// outside a table to the currently-open anonymous table, opening one + /// first if needed. Consecutive runs share a single anonymous table. + fn push_wrapped_in_table( + &mut self, + container_node_id: NodeId, + child_id: NodeId, + doc: &mut BaseDocument, + ) { + if self.anonymous_table_id.is_none() { + self.maybe_push_anon_block(doc); + let node_id = create_anonymous_node(doc, container_node_id, AnonKind::Table); + self.children.push(node_id); + self.anonymous_blocks.push(node_id); + self.anonymous_table_id = Some(node_id); + } + doc.nodes[self.anonymous_table_id.unwrap()] + .children + .push(child_id); + } + fn create_anonymous_block(&mut self, container_node_id: NodeId, doc: &mut BaseDocument) { let node_id = create_anonymous_node(doc, container_node_id, AnonKind::Block); self.children.push(node_id); @@ -256,6 +290,12 @@ fn push_hoisted_child( collect_layout_children_with_wrap(doc, child_id, out, wrap); return; } + if child_display.outside() == DisplayOutside::InternalTable { + if let Some(wrap_ctx) = wrap { + out.push_wrapped_in_table(wrap_ctx.container_node_id, child_id, doc); + return; + } + } if let Some(wrap_ctx) = wrap { let child_node_kind = child.data.kind(); let display_outside = if child.is_or_contains_block() { @@ -327,6 +367,10 @@ struct FlowClassification { all_inline: bool, all_out_of_flow: bool, has_contents: bool, + /// The container has table-internal children (rows, row groups, cells, + /// ...) which, occurring outside a table, must be wrapped in an + /// anonymous table box. + has_stray_table_internal: bool, } impl Default for FlowClassification { @@ -336,6 +380,7 @@ impl Default for FlowClassification { all_inline: true, all_out_of_flow: true, has_contents: false, + has_stray_table_internal: false, } } } @@ -421,9 +466,13 @@ fn classify_flow_children( classification.all_out_of_flow = false; match display.outside() { DisplayOutside::None => {} - DisplayOutside::Block - | DisplayOutside::TableCaption - | DisplayOutside::InternalTable => classification.all_inline = false, + DisplayOutside::InternalTable => { + classification.all_inline = false; + classification.has_stray_table_internal = true; + } + DisplayOutside::Block | DisplayOutside::TableCaption => { + classification.all_inline = false + } DisplayOutside::Inline => { classification.all_block = false; @@ -568,7 +617,7 @@ fn collect_layout_children_with_wrap( // ::before/::after pseudos must be checked too: a pseudo with // display:contents hoists its text content into the container. let container = &doc.nodes[container_node_id]; - let has_text_node_or_contents = container + let needs_complex = container .children .iter() .copied() @@ -578,10 +627,12 @@ fn collect_layout_children_with_wrap( .any(|child| { let display = child.display_style().unwrap_or(Display::inline()); let node_kind = child.data.kind(); - display.inside() == DisplayInside::Contents || node_kind == NodeKind::Text + display.inside() == DisplayInside::Contents + || display.outside() == DisplayOutside::InternalTable + || node_kind == NodeKind::Text }); - if !has_text_node_or_contents { + if !needs_complex { return push_non_whitespace_children_and_pseudos( &mut out.children, &doc.nodes[container_node_id], @@ -598,7 +649,9 @@ fn collect_layout_children_with_wrap( } DisplayInside::Table => { - let (table_context, tlayout_children) = build_table_context(doc, container_node_id); + let (table_context, tlayout_children, anonymous_nodes) = + build_table_context(doc, container_node_id); + out.anonymous_blocks.extend(anonymous_nodes); #[allow(clippy::arc_with_non_send_sync)] let data = SpecialElementData::TableRoot(Arc::new(table_context)); doc.nodes[container_node_id] @@ -618,13 +671,9 @@ fn collect_layout_children_with_wrap( } } - // Flow, FlowRoot and TableCell, plus internal table displays (row, - // row group, column, ...) occurring outside of a table. Blitz does - // not yet generate anonymous table wrapper boxes for the latter, so - // they are laid out as flow containers, which crucially ensures - // their text children get wrapped rather than being pushed as bare - // layout children (text nodes carry no style and cannot be laid out - // as block/flex/grid items). + // Flow, FlowRoot and TableCell. Table-internal children (rows, row + // groups, cells, ...) occurring outside a table are wrapped in an + // anonymous table box by the classification below. _ => { let mut classification = FlowClassification::default(); classify_flow_children(doc, container_node_id, &mut classification); @@ -663,7 +712,10 @@ fn collect_layout_children_with_wrap( // If the children are either all inline or all block then simply return the regular children // as the layout children - if classification.all_block & !classification.has_contents { + if classification.all_block + & !classification.has_contents + & !classification.has_stray_table_internal + { return push_non_whitespace_children_and_pseudos( &mut out.children, &doc.nodes[container_node_id], @@ -866,6 +918,11 @@ fn collect_complex_layout_children( }); collect_layout_children_with_wrap(doc, child_id, out, wrap) } + // Table-internal children occurring outside a table get wrapped in + // an anonymous table box (missing-parent fixup) + else if child_display.outside() == DisplayOutside::InternalTable { + out.push_wrapped_in_table(container_node_id, child_id, doc); + } // Push nodes that need wrapping into the current "anonymous block container". // If there is not an open one then we create one. else if needs_wrap(child_node_kind, display_outside) { diff --git a/packages/blitz-dom/src/layout/table.rs b/packages/blitz-dom/src/layout/table.rs index 7c90ade80..1a0e5303b 100644 --- a/packages/blitz-dom/src/layout/table.rs +++ b/packages/blitz-dom/src/layout/table.rs @@ -16,6 +16,7 @@ use taffy::{ use crate::BaseDocument; +use super::construct::{AnonKind, create_anonymous_node}; use super::damage::{CONSTRUCT_BOX, CONSTRUCT_DESCENDENT, CONSTRUCT_FC}; use super::resolve_calc_value; @@ -57,12 +58,7 @@ pub struct TableRow { pub(crate) fn build_table_context( doc: &mut BaseDocument, table_root_node_id: NodeId, -) -> (TableContext, Vec) { - let mut cells: Vec = Vec::new(); - let mut rows: Vec = Vec::new(); - let mut row = 0u16; - let mut col = 0u16; - +) -> (TableContext, Vec, Vec) { let root_node = &mut doc.nodes[table_root_node_id]; let children = std::mem::take(&mut root_node.children); @@ -92,25 +88,38 @@ pub(crate) fn build_table_context( drop(stylo_styles); - let mut column_sizes: Vec = Vec::new(); - let mut first_cell_border: Option> = None; + let mut builder = TableBuilder { + table_root_node_id, + is_fixed, + border_collapse, + row: 0, + col: 0, + cells: Vec::new(), + rows: Vec::new(), + columns: Vec::new(), + first_cell_border: None, + anonymous_nodes: Vec::new(), + open_anon_row: None, + open_anon_cell: None, + }; for child_id in children.iter().copied() { - collect_table_cells( - doc, - child_id, - is_fixed, - border_collapse, - &mut row, - &mut col, - &mut cells, - &mut rows, - &mut column_sizes, - &mut first_cell_border, - ); + builder.visit(doc, child_id, false); } - column_sizes.resize(col as usize, style_helpers::auto()); - style.grid_template_columns = column_sizes.into_iter().map(|dim| dim.into()).collect(); + let TableBuilder { + row, + col, + cells, + rows, + mut columns, + first_cell_border, + anonymous_nodes, + .. + } = builder; + + columns.resize(col as usize, style_helpers::auto()); + + style.grid_template_columns = columns.into_iter().map(|dim| dim.into()).collect(); style.grid_template_rows = vec![style_helpers::auto(); row as usize]; style.gap = match border_collapse { @@ -175,192 +184,251 @@ pub(crate) fn build_table_context( border_style: first_cell_border, }, layout_children, + anonymous_nodes, ) } -#[allow(clippy::too_many_arguments)] -pub(crate) fn collect_table_cells( - doc: &mut BaseDocument, - node_id: NodeId, +/// Walks a table's descendants, mapping rows/cells into the table grid and +/// generating anonymous rows/cells around misplaced children per the box +/// fixup rules of CSS 2.2 §17.2.1: consecutive runs of non-table-internal +/// children share a single anonymous cell, and cells (real or anonymous) +/// occurring outside a row share a single anonymous row. +struct TableBuilder { + table_root_node_id: NodeId, is_fixed: bool, border_collapse: BorderCollapse, - row: &mut u16, - col: &mut u16, - cells: &mut Vec, - rows: &mut Vec, - columns: &mut Vec, - first_cell_border: &mut Option>, -) { - let node = &mut doc.nodes[node_id]; - - if !node.is_element() { - return; - } + row: u16, + col: u16, + cells: Vec, + rows: Vec, + columns: Vec, + first_cell_border: Option>, + /// Anonymous row/cell nodes created during this build. Recorded on the + /// table root (via `LayoutChildren::anonymous_blocks`) so they are + /// deallocated the next time it is reconstructed. + anonymous_nodes: Vec, + /// The anonymous row currently accepting cells that occur outside a real + /// row. Closed by any real row or row group. + open_anon_row: Option, + /// The anonymous cell currently accepting misplaced children. Closed by + /// any table-internal sibling. + open_anon_cell: Option, +} - let Some(display) = node.primary_styles().map(|s| s.clone_display()) else { - #[cfg(feature = "tracing")] - tracing::info!("Ignoring table descendent because it has no styles"); - return; - }; +impl TableBuilder { + /// `in_row` is true when `node_id` is a child of a real table-row. + fn visit(&mut self, doc: &mut BaseDocument, node_id: NodeId, in_row: bool) { + let node = &mut doc.nodes[node_id]; + + if !node.is_element() { + // Non-whitespace text gets an anonymous cell. Whitespace-only + // text only joins an already-open anonymous cell. + if node.is_text_node() && (!node.is_whitespace_node() || self.open_anon_cell.is_some()) + { + self.push_into_anon_cell(doc, node_id, in_row); + } + return; + } - if display.outside() == DisplayOutside::None { - node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); - return; - } + let Some(display) = node.primary_styles().map(|s| s.clone_display()) else { + #[cfg(feature = "tracing")] + tracing::info!("Ignoring table descendent because it has no styles"); + return; + }; + + if display.outside() == DisplayOutside::None { + node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); + return; + } + + match display.inside() { + DisplayInside::TableRowGroup + | DisplayInside::TableHeaderGroup + | DisplayInside::TableFooterGroup => { + self.open_anon_cell = None; + self.open_anon_row = None; + self.visit_children(doc, node_id, false); + } + // display:contents is transparent for box generation: its + // children participate as if they were siblings of the contents + // node, so open anonymous runs are neither closed nor reopened. + DisplayInside::Contents => { + doc.nodes[node_id] + .remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); + self.visit_children(doc, node_id, in_row); + } + DisplayInside::TableRow => { + self.open_anon_cell = None; + self.open_anon_row = None; + + doc.nodes[node_id] + .remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); + self.row += 1; + self.col = 0; - match display.inside() { - DisplayInside::TableRowGroup - | DisplayInside::TableHeaderGroup - | DisplayInside::TableFooterGroup - | DisplayInside::Contents => { - let children = std::mem::take(&mut doc.nodes[node_id].children); - for child_id in children.iter().copied() { - doc.nodes[child_id] + self.rows.push(TableRow { + node_id, + height: 0.0, + }); + + self.visit_children(doc, node_id, true); + self.open_anon_cell = None; + } + DisplayInside::TableCell => { + self.open_anon_cell = None; + if !in_row { + self.ensure_anon_row(doc); + } + self.push_cell(doc, node_id, true); + } + // Non-table-internal children generate an anonymous table cell + // around them, with consecutive runs sharing a single cell. + DisplayInside::Flow + | DisplayInside::FlowRoot + | DisplayInside::Flex + | DisplayInside::Grid + | DisplayInside::Table => { + self.push_into_anon_cell(doc, node_id, in_row); + } + DisplayInside::TableColumnGroup | DisplayInside::TableColumn => { + doc.nodes[node_id] + .remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); + //Ignore + } + DisplayInside::None => { + doc.nodes[node_id] .remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); - collect_table_cells( - doc, - child_id, - is_fixed, - border_collapse, - row, - col, - cells, - rows, - columns, - first_cell_border, - ); + // Ignore } - doc.nodes[node_id].children = children; } - DisplayInside::TableRow => { - node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); - *row += 1; - *col = 0; + } + + fn visit_children(&mut self, doc: &mut BaseDocument, node_id: NodeId, in_row: bool) { + let children = std::mem::take(&mut doc.nodes[node_id].children); + for child_id in children.iter().copied() { + self.visit(doc, child_id, in_row); + } + doc.nodes[node_id].children = children; + } - rows.push(TableRow { - node_id, + /// Open an anonymous row to hold cells occurring outside a real row. + fn ensure_anon_row(&mut self, doc: &mut BaseDocument) { + if self.open_anon_row.is_none() { + let anon_id = create_anonymous_node(doc, self.table_root_node_id, AnonKind::TableRow); + // Anonymous rows are not layout children, so construction damage + // would never be cleared by the resolve pass. + doc.nodes[anon_id].remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); + self.anonymous_nodes.push(anon_id); + + self.row += 1; + self.col = 0; + self.rows.push(TableRow { + node_id: anon_id, height: 0.0, }); + self.open_anon_row = Some(anon_id); + } + } - let children = std::mem::take(&mut doc.nodes[node_id].children); - for child_id in children.iter().copied() { - collect_table_cells( - doc, - child_id, - is_fixed, - border_collapse, - row, - col, - cells, - rows, - columns, - first_cell_border, - ); + /// Append a misplaced (non-table-internal) child to the currently-open + /// anonymous cell, opening one (and an anonymous row if needed) first. + fn push_into_anon_cell(&mut self, doc: &mut BaseDocument, node_id: NodeId, in_row: bool) { + if self.open_anon_cell.is_none() { + if !in_row { + self.ensure_anon_row(doc); } - doc.nodes[node_id].children = children; + let container_id = doc.nodes[node_id].parent.unwrap_or(self.table_root_node_id); + let anon_id = create_anonymous_node(doc, container_id, AnonKind::TableCell); + self.anonymous_nodes.push(anon_id); + self.push_cell(doc, anon_id, false); + self.open_anon_cell = Some(anon_id); } - // Table-cell children, plus non-table-internal children which, per the - // CSS tables spec, generate an anonymous table cell around them. - DisplayInside::TableCell - | DisplayInside::Flow - | DisplayInside::FlowRoot - | DisplayInside::Flex - | DisplayInside::Grid => { - // node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); - let is_cell = display.inside() == DisplayInside::TableCell; - let stylo_style = &node.primary_styles().unwrap(); - let colspan: u16 = if is_cell { - node.attr(local_name!("colspan")) - .and_then(|val| val.parse().ok()) - .unwrap_or(1) - } else { - 1 - }; - let rowspan: u16 = if is_cell { - node.attr(local_name!("rowspan")) - .and_then(|val| val.parse::().ok()) - .map(|v| v.clamp(1, 65534)) - .unwrap_or(1) - } else { - 1 - }; - let mut style = stylo_taffy::to_taffy_style(stylo_style); + doc.nodes[self.open_anon_cell.unwrap()] + .children + .push(node_id); + } - if is_cell && first_cell_border.is_none() { - *first_cell_border = Some(stylo_style.clone_border()); - } + /// Map a cell (real or anonymous) into the table grid. `is_real_cell` + /// controls whether the cell's border participates in the table's + /// collapsed-border approximation. + fn push_cell(&mut self, doc: &mut BaseDocument, node_id: NodeId, is_real_cell: bool) { + let node = &mut doc.nodes[node_id]; + let stylo_style = &node.primary_styles().unwrap(); + let colspan: u16 = node + .attr(local_name!("colspan")) + .and_then(|val| val.parse().ok()) + .unwrap_or(1); + let rowspan: u16 = node + .attr(local_name!("rowspan")) + .and_then(|val| val.parse::().ok()) + .map(|v| v.clamp(1, 65534)) + .unwrap_or(1); + let mut style = stylo_taffy::to_taffy_style(stylo_style); + + if is_real_cell && self.first_cell_border.is_none() { + self.first_cell_border = Some(stylo_style.clone_border()); + } - // Cells occurring before any row are placed in an anonymous row - if *row == 0 { - *row = 1; - } + // Cells occurring before any row are placed in an anonymous row + if self.row == 0 { + self.row = 1; + } - if *row == 1 { - let column = match style.size.width.tag() { - taffy::CompactLength::LENGTH_TAG => { - let len = style.size.width.value(); - let padding = style.padding.resolve_or_zero(None, resolve_calc_value); - let border = style.border.resolve_or_zero(None, resolve_calc_value); - match style.box_sizing { - taffy::BoxSizing::ContentBox => style_helpers::length( - len + padding.left + padding.right + border.left + border.right, - ), - taffy::BoxSizing::BorderBox => style_helpers::length(len), - } + if self.row == 1 { + let column = match style.size.width.tag() { + taffy::CompactLength::LENGTH_TAG => { + let len = style.size.width.value(); + let padding = style.padding.resolve_or_zero(None, resolve_calc_value); + let border = style.border.resolve_or_zero(None, resolve_calc_value); + match style.box_sizing { + taffy::BoxSizing::ContentBox => style_helpers::length( + len + padding.left + padding.right + border.left + border.right, + ), + taffy::BoxSizing::BorderBox => style_helpers::length(len), } - taffy::CompactLength::PERCENT_TAG => { - if is_fixed { - style_helpers::percent(style.size.width.value()) - } else { - style_helpers::auto() - } + } + taffy::CompactLength::PERCENT_TAG => { + if self.is_fixed { + style_helpers::percent(style.size.width.value()) + } else { + style_helpers::auto() } - taffy::CompactLength::AUTO_TAG => style_helpers::auto(), - // Dimension values are always length, percentage, auto or calc(), - // so any other tag is a calc() value. Pass it through so that - // Taffy resolves it against the table's inner width. - _ => style.size.width.into(), - }; - columns.push(column); - } - - // Zero-out cell borders is BorderCollapse is Collapse - // Borders are handled at the table level in this mode - if is_cell && border_collapse == BorderCollapse::Collapse { - style.border = taffy::Rect::ZERO.map(style_helpers::length); - } - - // The margin properties do not apply to table-internal elements - if is_cell { - style.margin = taffy::Rect::ZERO.map(style_helpers::length); - } - - // Let Taffy auto-place the column. Combined with - // `grid_auto_flow: RowDense` set on the table root, each cell - // scans from the first track in its row for a free position, - // which makes cells automatically skip columns occupied by - // rowspan cells from earlier rows. - style.grid_column = taffy::Line { - start: style_helpers::auto(), - end: style_helpers::span(colspan), - }; - style.grid_row = taffy::Line { - start: style_helpers::line(*row as i16), - end: style_helpers::span(rowspan), + } + taffy::CompactLength::AUTO_TAG => style_helpers::auto(), + // Dimension values are always length, percentage, auto or calc(), + // so any other tag is a calc() value. Pass it through so that + // Taffy resolves it against the table's inner width. + _ => style.size.width.into(), }; - style.size.width = style_helpers::auto(); - cells.push(TableCell { node_id, style }); - - *col += colspan; + self.columns.push(column); } - DisplayInside::TableColumnGroup | DisplayInside::TableColumn | DisplayInside::Table => { - node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); - //Ignore - } - DisplayInside::None => { - node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); - // Ignore + + // Zero-out cell borders is BorderCollapse is Collapse + // Borders are handled at the table level in this mode + if self.border_collapse == BorderCollapse::Collapse { + style.border = taffy::Rect::ZERO.map(style_helpers::length); } + + // The margin properties do not apply to table-internal elements + style.margin = taffy::Rect::ZERO.map(style_helpers::length); + + // Let Taffy auto-place the column. Combined with + // `grid_auto_flow: RowDense` set on the table root, each cell + // scans from the first track in its row for a free position, + // which makes cells automatically skip columns occupied by + // rowspan cells from earlier rows. + style.grid_column = taffy::Line { + start: style_helpers::auto(), + end: style_helpers::span(colspan), + }; + style.grid_row = taffy::Line { + start: style_helpers::line(self.row as i16), + end: style_helpers::span(rowspan), + }; + style.size.width = style_helpers::auto(); + self.cells.push(TableCell { node_id, style }); + + self.col += colspan; } } diff --git a/tests/blitz-tests/tests/table_box_fixups.rs b/tests/blitz-tests/tests/table_box_fixups.rs new file mode 100644 index 000000000..ec4b67b6e --- /dev/null +++ b/tests/blitz-tests/tests/table_box_fixups.rs @@ -0,0 +1,272 @@ +//! Anonymous table box construction / fixups (CSS 2.2 §17.2.1). +//! +//! - Consecutive non-table-internal children of a table share a single +//! anonymous table cell (so they stack vertically in one column). +//! - Table cells occurring directly under a table are wrapped in an +//! anonymous table row. +//! - Table-internal boxes (cells, rows) occurring outside a table are +//! wrapped in an anonymous table box. +//! - The anonymous nodes generated by these fixups must not leak across +//! reconstructions or incremental mutations. + +use blitz_dom::DocumentConfig; +use blitz_html::{HtmlDocument, HtmlProvider}; +use blitz_test_harness::{Harness, HarnessOptions}; +use blitz_traits::shell::{ColorScheme, Viewport}; +use std::sync::Arc; + +fn harness(html: &str) -> Harness { + Harness::from_html_with( + html, + HarnessOptions { + width: 400, + height: 200, + ..Default::default() + }, + ) +} + +fn document(html: &str) -> HtmlDocument { + HtmlDocument::from_html( + html, + DocumentConfig { + viewport: Some(Viewport::new(400, 200, 1.0, ColorScheme::Light)), + html_parser_provider: Some(Arc::new(HtmlProvider) as _), + ..Default::default() + }, + ) +} + +/// Consecutive misplaced (non-table-internal) children share one anonymous +/// cell: they lay out as flow content in a single column, stacking +/// vertically, rather than each becoming its own cell/column. +#[test] +fn consecutive_misplaced_children_share_one_anonymous_cell() { + let harness = harness( + r#" +
+
+
+
+ "#, + ); + + let a = harness.layout_rect("#a"); + let b = harness.layout_rect("#b"); + assert!(a.width > 0.0 && a.height > 0.0, "got {a:?}"); + assert!(b.width > 0.0 && b.height > 0.0, "got {b:?}"); + assert_eq!( + a.x, b.x, + "run-sharing children should be in the same column" + ); + assert!( + b.y >= a.y + a.height, + "run-sharing children should stack vertically: a={a:?} b={b:?}" + ); +} + +/// A real cell terminates the anonymous run: the misplaced children form one +/// cell and the real cell forms another column beside it. +#[test] +fn real_cell_closes_anonymous_run() { + let harness = harness( + r#" +
+
A
+
B
+
+ "#, + ); + + let misplaced = harness.layout_rect("#misplaced"); + let cell = harness.layout_rect("#cell"); + assert!(misplaced.width > 0.0 && misplaced.height > 0.0); + assert!(cell.width > 0.0 && cell.height > 0.0); + assert!( + cell.x >= misplaced.x + misplaced.width, + "real cell should form its own column: misplaced={misplaced:?} cell={cell:?}" + ); +} + +/// Table cells directly under the table (no row) get an anonymous row and +/// lay out side by side. +#[test] +fn cells_directly_under_table_get_anonymous_row() { + let harness = harness( + r#" +
+
A
+
B
+
+ "#, + ); + + let c1 = harness.layout_rect("#c1"); + let c2 = harness.layout_rect("#c2"); + assert!(c1.width > 0.0 && c1.height > 0.0); + assert!(c2.width > 0.0 && c2.height > 0.0); + assert_eq!(c1.y, c2.y, "cells in the anonymous row should share a row"); + assert!( + c2.x >= c1.x + c1.width, + "cells should lay out side by side: c1={c1:?} c2={c2:?}" + ); +} + +/// Bare text directly under a table is wrapped in an anonymous cell and laid +/// out (the table gets a nonzero content height). +#[test] +fn bare_text_under_table_is_laid_out() { + let harness = harness( + r#" +
hello world
+ "#, + ); + + let table = harness.layout_rect("#table"); + assert!( + table.height > 0.0, + "table with bare text content should have height, got {table:?}" + ); +} + +/// Table-internal boxes outside a table (missing parent) are wrapped in an +/// anonymous table: sibling table-cells in a block container lay out side by +/// side as columns of the generated table. +#[test] +fn stray_cells_in_block_container_generate_anonymous_table() { + let harness = harness( + r#" +
+
A
+
B
+
+ "#, + ); + + let s1 = harness.layout_rect("#s1"); + let s2 = harness.layout_rect("#s2"); + assert!(s1.width > 0.0 && s1.height > 0.0, "got {s1:?}"); + assert!(s2.width > 0.0 && s2.height > 0.0, "got {s2:?}"); + assert_eq!(s1.y, s2.y, "stray cells should share an anonymous row"); + assert!( + s2.x >= s1.x + s1.width, + "stray cells should lay out side by side: s1={s1:?} s2={s2:?}" + ); +} + +/// A stray table-row outside a table is likewise wrapped: its cells lay out +/// side by side. +#[test] +fn stray_row_in_block_container_generates_anonymous_table() { + let harness = harness( + r#" +
+
+
A
+
B
+
+
+ "#, + ); + + let r1 = harness.layout_rect("#r1"); + let r2 = harness.layout_rect("#r2"); + assert!(r1.width > 0.0 && r1.height > 0.0, "got {r1:?}"); + assert!(r2.width > 0.0 && r2.height > 0.0, "got {r2:?}"); + assert_eq!(r1.y, r2.y); + assert!( + r2.x >= r1.x + r1.width, + "cells of a stray row should lay out side by side: r1={r1:?} r2={r2:?}" + ); +} + +/// Incremental mutations inside a fixed-up table update geometry and do not +/// leak anonymous nodes. +#[test] +fn incremental_mutations_update_geometry_without_leaking() { + let mut harness = harness( + r#" +
+
+
+
+ "#, + ); + + let a = harness.node("#a"); + let style = blitz_dom::QualName::new(None, blitz_dom::ns!(), blitz_dom::local_name!("style")); + + let node_count_before = harness.base().tree().len(); + + for i in 0..10 { + let height = 30 + 10 * (i % 2); + harness + .base_mut() + .mutate() + .set_attribute(a, style.clone(), &format!("height:{height}px;")); + harness.pump(); + let rect = harness.layout_rect("#a"); + assert_eq!( + rect.height, height as f32, + "mutation {i} should update the misplaced child's geometry" + ); + } + + assert_eq!( + harness.base().tree().len(), + node_count_before, + "repeated incremental mutations must not leak nodes" + ); +} + +/// Full (non-incremental) reconstructions of tables with fixups must not +/// leak anonymous nodes. +#[test] +fn table_fixup_anonymous_nodes_do_not_leak_across_reconstructions() { + let mut doc = document( + r#" +
+
misplaced
+
cell without row
+
+
+
stray cell
+
+ "#, + ); + + doc.set_incremental_layout(false); + doc.resolve(0.0); + + let anon_after_first = doc + .tree() + .iter() + .filter(|(_, node)| node.is_anonymous()) + .count(); + let nodes_after_first = doc.tree().len(); + + assert!( + anon_after_first >= 3, + "expected anonymous table fixup nodes to be generated, got {anon_after_first}" + ); + + for _ in 0..20 { + doc.resolve(0.0); + } + + let anon_after_many = doc + .tree() + .iter() + .filter(|(_, node)| node.is_anonymous()) + .count(); + + assert_eq!( + anon_after_many, anon_after_first, + "anonymous fixup node count grew across reconstructions" + ); + assert_eq!( + doc.tree().len(), + nodes_after_first, + "total node count grew across reconstructions" + ); +} From 640f18dbfb8aab4fef0e654edaf20c57f9ff4a9e Mon Sep 17 00:00:00 2001 From: Nico Burns Date: Mon, 24 Aug 2026 22:32:10 +0000 Subject: [PATCH 2/2] Fix WPT regressions: ignore captions in table grid, treat replaced elements as ordinary table content, rebuild tables on table-property style changes --- packages/blitz-dom/src/layout/construct.rs | 20 +++++++++++++++++--- packages/blitz-dom/src/layout/damage.rs | 9 +++++++++ packages/blitz-dom/src/layout/table.rs | 21 +++++++++++++++++++++ 3 files changed, 47 insertions(+), 3 deletions(-) diff --git a/packages/blitz-dom/src/layout/construct.rs b/packages/blitz-dom/src/layout/construct.rs index ce9f02a63..cb5509c20 100644 --- a/packages/blitz-dom/src/layout/construct.rs +++ b/packages/blitz-dom/src/layout/construct.rs @@ -276,6 +276,14 @@ fn text_item_needs_wrap(child_node_kind: NodeKind, _display_outside: DisplayOuts child_node_kind == NodeKind::Text } +/// Whether a node is a replaced element. Table display values on replaced +/// elements are treated as ordinary content (CSS 2.2 §17.2.1 applies to +/// non-replaced elements only). +fn node_is_replaced(node: &Node) -> bool { + node.element_data() + .is_some_and(|el| is_replaced_element(&el.name.local)) +} + /// Push a single hoisted child, recursing through display:contents nodes and /// wrapping text/inline children per the ancestor container's `WrapContext`. fn push_hoisted_child( @@ -290,7 +298,7 @@ fn push_hoisted_child( collect_layout_children_with_wrap(doc, child_id, out, wrap); return; } - if child_display.outside() == DisplayOutside::InternalTable { + if child_display.outside() == DisplayOutside::InternalTable && !node_is_replaced(child) { if let Some(wrap_ctx) = wrap { out.push_wrapped_in_table(wrap_ctx.container_node_id, child_id, doc); return; @@ -468,7 +476,11 @@ fn classify_flow_children( DisplayOutside::None => {} DisplayOutside::InternalTable => { classification.all_inline = false; - classification.has_stray_table_internal = true; + // Table display values on replaced elements are treated + // as ordinary content, not table-internal boxes. + if !node_is_replaced(child) { + classification.has_stray_table_internal = true; + } } DisplayOutside::Block | DisplayOutside::TableCaption => { classification.all_inline = false @@ -920,7 +932,9 @@ fn collect_complex_layout_children( } // Table-internal children occurring outside a table get wrapped in // an anonymous table box (missing-parent fixup) - else if child_display.outside() == DisplayOutside::InternalTable { + else if child_display.outside() == DisplayOutside::InternalTable + && !node_is_replaced(&doc.nodes[child_id]) + { out.push_wrapped_in_table(container_node_id, child_id, doc); } // Push nodes that need wrapping into the current "anonymous block container". diff --git a/packages/blitz-dom/src/layout/damage.rs b/packages/blitz-dom/src/layout/damage.rs index 9957254f3..e895630fb 100644 --- a/packages/blitz-dom/src/layout/damage.rs +++ b/packages/blitz-dom/src/layout/damage.rs @@ -214,6 +214,15 @@ pub(crate) fn compute_layout_damage(old: &ComputedValues, new: &ComputedValues) return true; } + // Table properties (border-spacing, border-collapse, ...) are baked + // into the TableContext at construction time, so changing them + // requires reconstructing the table's box tree. + if old.get_inherited_table() != new.get_inherited_table() + || old.get_table() != new.get_table() + { + return true; + } + if new_box.display.outside() == DisplayOutside::Block && new_box.display.inside() == DisplayInside::Flow { diff --git a/packages/blitz-dom/src/layout/table.rs b/packages/blitz-dom/src/layout/table.rs index 1a0e5303b..aa66416a1 100644 --- a/packages/blitz-dom/src/layout/table.rs +++ b/packages/blitz-dom/src/layout/table.rs @@ -18,6 +18,7 @@ use crate::BaseDocument; use super::construct::{AnonKind, create_anonymous_node}; use super::damage::{CONSTRUCT_BOX, CONSTRUCT_DESCENDENT, CONSTRUCT_FC}; +use super::replaced::is_replaced_element; use super::resolve_calc_value; pub struct TableTreeWrapper<'doc> { @@ -241,6 +242,26 @@ impl TableBuilder { return; } + // Captions are not part of the table grid. Blitz does not yet + // generate the table wrapper box that would hold them, so they are + // dropped rather than being wrapped in an anonymous cell. They do + // not close an open anonymous run. + if display.outside() == DisplayOutside::TableCaption { + node.remove_damage(CONSTRUCT_DESCENDENT | CONSTRUCT_FC | CONSTRUCT_BOX); + return; + } + + // Table display values on replaced elements are treated as ordinary + // content: per CSS 2.2 §17.2.1 the table box generation rules apply + // to non-replaced elements only. + let is_replaced = node + .element_data() + .is_some_and(|el| is_replaced_element(&el.name.local)); + if is_replaced { + self.push_into_anon_cell(doc, node_id, in_row); + return; + } + match display.inside() { DisplayInside::TableRowGroup | DisplayInside::TableHeaderGroup