fix(table): span zero row spans to section end

moli-layout/src/table.rs

@@ -731,7 +731,7 @@ fn collect_rows<N>(
                 }
                 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);
                     }

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;

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::<u32>().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", ""));