diff --git a/moli-layout/src/table.rs b/moli-layout/src/table.rs index 1b60fb2009..904f08a3cb 100644 --- a/moli-layout/src/table.rs +++ b/moli-layout/src/table.rs @@ -731,7 +731,7 @@ fn collect_rows( } let data = table_data(world, cell); let column_span = usize::from(data.column_span.max(1)); - let row_span = usize::from(data.row_span.max(1)); + let row_span = usize::from(data.row_span); let authored_style = &world.boxes[cell.index()].style; let mut cell_style = authored_style.taffy.clone(); cell_style.margin = Rect::ZERO.map(style_helpers::length); @@ -786,10 +786,12 @@ fn place_table_cells(cells: &mut [TableCell], rows: &[TableRow], max_columns: &m .all(|occupied| *occupied <= row.index) { cell.column = cursor; - cell.row_span = cell - .row_span - .min(section_end.saturating_sub(row.index)) - .max(1); + let row_span = if cell.row_span == 0 { + section_end.saturating_sub(row.index) + } else { + cell.row_span + }; + cell.row_span = row_span.min(section_end.saturating_sub(row.index)).max(1); for occupied in &mut occupied_until[cursor..end] { *occupied = row.index.saturating_add(cell.row_span); } diff --git a/moli-layout/tests/phase4_layout_contract.rs b/moli-layout/tests/phase4_layout_contract.rs index ddc410b46b..64db0de42f 100644 --- a/moli-layout/tests/phase4_layout_contract.rs +++ b/moli-layout/tests/phase4_layout_contract.rs @@ -327,6 +327,77 @@ fn table_caption_tracks_rows_cells_and_common_spans_share_one_wrapper_geometry() assert_close(span.width, 300.0); } +#[test] +fn zero_row_span_reaches_the_end_of_its_row_group() { + use LayoutElementCategory::Table; + use LayoutTableRole::{BodyGroup, Cell, Row, Table as Root}; + + let cell = |label, row_span| { + Node::element(label, "td", Table(Cell), None, Vec::new()).with_metadata( + LayoutElementMetadata { + table: Some(LayoutTableData { + row_span, + ..LayoutTableData::default() + }), + ..LayoutElementMetadata::default() + }, + ) + }; + let source = Source(vec![ + Node::element("root", "div", LayoutElementCategory::Generic, None, vec![1]), + Node::element("table", "table", Table(Root), None, vec![2, 8]), + Node::element("first-body", "tbody", Table(BodyGroup), None, vec![3, 6]), + Node::element("first-row", "tr", Table(Row), None, vec![4, 5]), + cell("zero-span", 0), + cell("first-reference", 1), + Node::element("second-row", "tr", Table(Row), None, vec![7]), + cell("second-reference", 1), + Node::element("second-body", "tbody", Table(BodyGroup), None, vec![9]), + Node::element("third-row", "tr", Table(Row), None, vec![10]), + cell("next-group", 1), + ]); + let mut styles = Styles::default(); + styles.primary.insert( + 0, + sized(LayoutDisplay::Block, 200.0, 100.0, PaintColor::TRANSPARENT), + ); + styles.primary.insert( + 1, + sized(LayoutDisplay::Table, 100.0, 60.0, PaintColor::TRANSPARENT), + ); + for id in [2, 8] { + styles.primary.insert( + id, + style(LayoutDisplay::TableRowGroup, PaintColor::TRANSPARENT), + ); + } + for id in [3, 6, 9] { + styles + .primary + .insert(id, style(LayoutDisplay::TableRow, PaintColor::TRANSPARENT)); + } + styles + .primary + .insert(4, sized(LayoutDisplay::TableCell, 50.0, 20.0, RED)); + styles + .primary + .insert(5, sized(LayoutDisplay::TableCell, 50.0, 20.0, GREEN)); + styles + .primary + .insert(7, sized(LayoutDisplay::TableCell, 50.0, 20.0, BLUE)); + styles + .primary + .insert(10, sized(LayoutDisplay::TableCell, 100.0, 20.0, YELLOW)); + + let snapshot = render(&source, &mut styles, 200, 100); + let zero_span = rect(&snapshot, RED); + let first_reference = rect(&snapshot, GREEN); + let next_group = rect(&snapshot, YELLOW); + assert_close(zero_span.height, 40.0); + assert_close(first_reference.height, 20.0); + assert_close(next_group.y, 40.0); +} + #[test] fn separated_table_parts_ignore_authored_border_padding_and_margin() { use LayoutElementCategory::Table; diff --git a/moli-renderer-v8/src/layout_renderer/source_view.rs b/moli-renderer-v8/src/layout_renderer/source_view.rs index 86b1912166..657278ee4f 100644 --- a/moli-renderer-v8/src/layout_renderer/source_view.rs +++ b/moli-renderer-v8/src/layout_renderer/source_view.rs @@ -482,7 +482,7 @@ fn layout_element_metadata( match role { LayoutTableRole::Cell => { table.column_span = positive_u16_attribute(element, "colspan", 1, 1000); - table.row_span = positive_u16_attribute(element, "rowspan", 1, 65_534); + table.row_span = non_negative_u16_attribute(element, "rowspan", 1, 65_534); } LayoutTableRole::Column | LayoutTableRole::ColumnGroup => { table.span = positive_u16_attribute(element, "span", 1, 1000); @@ -543,6 +543,19 @@ fn positive_u16_attribute( optional_positive_u16_attribute(element, name, 1, maximum).unwrap_or(default) } +fn non_negative_u16_attribute( + element: &crate::dom::native::Element, + name: &str, + default: u16, + maximum: u16, +) -> u16 { + element + .attribute(name) + .and_then(|value| value.trim().parse::().ok()) + .map(|value| value.min(u32::from(maximum)) as u16) + .unwrap_or(default) +} + fn optional_positive_u16_attribute( element: &crate::dom::native::Element, name: &str, @@ -847,6 +860,14 @@ mod tests { } ); + assert!(host.set_attribute(cell, "rowspan", "0")); + let zero_row_span = layout_element_semantics_for_source( + &host, + cell, + host.node(cell).unwrap().as_element().unwrap(), + ); + assert_eq!(zero_row_span.metadata.table.unwrap().row_span, 0); + let list = host.create_element("ol"); assert!(host.set_attribute(list, "start", "-3")); assert!(host.set_attribute(list, "reversed", ""));