From b4e41d872789206a618ff729dc6f9fa8065cabd0 Mon Sep 17 00:00:00 2001
From: Oriol Brufau <obrufau@igalia.com>
Date: Tue, 11 Jun 2024 20:59:09 +0200
Subject: [PATCH] Fix table track offsets when there is `visibility: collapse`
 (#32469)

Each non-collapsed track used to increase the offset by the subsequent
border spacing. Now they will take care of their preceding spacing
instead.

This way, if a cell spans two rows, and the second is collapsed, the
cell won't be forced to be at least as tall as the border spacing.
This matches Gecko and Blink (WebKit lacks `visibility: collapse`).

This makes visibility-collapse-border-spacing-001.html fail because we
generate outlines in a different way than Blink. Gecko also fails it
in a similar (but different) way.
---
 components/layout_2020/table/layout.rs        | 36 ++++++++++---------
 tests/wpt/meta/MANIFEST.json                  | 15 +++++++-
 ...ility-collapse-border-spacing-001.html.ini |  2 ++
 ...sibility-collapse-border-spacing-001.html} |  0
 ...isibility-collapse-border-spacing-002.html | 32 +++++++++++++++++
 5 files changed, 68 insertions(+), 17 deletions(-)
 create mode 100644 tests/wpt/meta/css/css-tables/visibility-collapse-border-spacing-001.html.ini
 rename tests/wpt/tests/css/css-tables/{visibility-collapse-border-spacing.html => visibility-collapse-border-spacing-001.html} (100%)
 create mode 100644 tests/wpt/tests/css/css-tables/visibility-collapse-border-spacing-002.html

diff --git a/components/layout_2020/table/layout.rs b/components/layout_2020/table/layout.rs
index 7395eff0d45..3cabdf7727d 100644
--- a/components/layout_2020/table/layout.rs
+++ b/components/layout_2020/table/layout.rs
@@ -1944,36 +1944,40 @@ impl TableAndTrackDimensions {
         let fallback_block_size = table_layout.final_table_height;
 
         let mut column_dimensions = Vec::new();
-        let mut column_offset = if table_layout.table.size.width == 0 {
-            fallback_inline_size
-        } else {
-            border_spacing.inline
-        };
+        let mut column_offset = Au::zero();
         for column_index in 0..table_layout.table.size.width {
             if table_layout.is_column_collapsed(column_index) {
                 column_dimensions.push((column_offset, column_offset));
                 continue;
             }
-            let column_size = table_layout.distributed_column_widths[column_index];
-            column_dimensions.push((column_offset, column_offset + column_size));
-            column_offset += column_size + border_spacing.inline;
+            let start_offset = column_offset + border_spacing.inline;
+            let end_offset = start_offset + table_layout.distributed_column_widths[column_index];
+            column_dimensions.push((start_offset, end_offset));
+            column_offset = end_offset;
         }
+        column_offset += if table_layout.table.size.width == 0 {
+            fallback_inline_size
+        } else {
+            border_spacing.inline
+        };
 
         let mut row_dimensions = Vec::new();
-        let mut row_offset = if table_layout.table.size.height == 0 {
-            fallback_block_size
-        } else {
-            border_spacing.block
-        };
+        let mut row_offset = Au::zero();
         for row_index in 0..table_layout.table.size.height {
             if table_layout.is_row_collapsed(row_index) {
                 row_dimensions.push((row_offset, row_offset));
                 continue;
             }
-            let row_size = table_layout.row_sizes[row_index];
-            row_dimensions.push((row_offset, row_offset + row_size));
-            row_offset += row_size + border_spacing.block;
+            let start_offset = row_offset + border_spacing.block;
+            let end_offset = start_offset + table_layout.row_sizes[row_index];
+            row_dimensions.push((start_offset, end_offset));
+            row_offset = end_offset;
         }
+        row_offset += if table_layout.table.size.height == 0 {
+            fallback_block_size
+        } else {
+            border_spacing.block
+        };
 
         let table_start_corner = LogicalVec2 {
             inline: column_dimensions.first().map_or_else(Au::zero, |v| v.0),
diff --git a/tests/wpt/meta/MANIFEST.json b/tests/wpt/meta/MANIFEST.json
index 0ef1b7c42e1..81817bf2aba 100644
--- a/tests/wpt/meta/MANIFEST.json
+++ b/tests/wpt/meta/MANIFEST.json
@@ -236531,7 +236531,7 @@
        {}
       ]
      ],
-     "visibility-collapse-border-spacing.html": [
+     "visibility-collapse-border-spacing-001.html": [
       "738cea16d5d0d41a2990fead77e8fd1c8bc246ec",
       [
        null,
@@ -236544,6 +236544,19 @@
        {}
       ]
      ],
+     "visibility-collapse-border-spacing-002.html": [
+      "95e2fb063594dee23cec40e6faab6388a35231bd",
+      [
+       null,
+       [
+        [
+         "/css/reference/ref-filled-green-200px-square.html",
+         "=="
+        ]
+       ],
+       {}
+      ]
+     ],
      "visibility-collapse-colspan-003.html": [
       "37092818f62c8beff7666f88910a5e803292bb47",
       [
diff --git a/tests/wpt/meta/css/css-tables/visibility-collapse-border-spacing-001.html.ini b/tests/wpt/meta/css/css-tables/visibility-collapse-border-spacing-001.html.ini
new file mode 100644
index 00000000000..68f0fa04c80
--- /dev/null
+++ b/tests/wpt/meta/css/css-tables/visibility-collapse-border-spacing-001.html.ini
@@ -0,0 +1,2 @@
+[visibility-collapse-border-spacing-001.html]
+  expected: FAIL
diff --git a/tests/wpt/tests/css/css-tables/visibility-collapse-border-spacing.html b/tests/wpt/tests/css/css-tables/visibility-collapse-border-spacing-001.html
similarity index 100%
rename from tests/wpt/tests/css/css-tables/visibility-collapse-border-spacing.html
rename to tests/wpt/tests/css/css-tables/visibility-collapse-border-spacing-001.html
diff --git a/tests/wpt/tests/css/css-tables/visibility-collapse-border-spacing-002.html b/tests/wpt/tests/css/css-tables/visibility-collapse-border-spacing-002.html
new file mode 100644
index 00000000000..95e2fb06359
--- /dev/null
+++ b/tests/wpt/tests/css/css-tables/visibility-collapse-border-spacing-002.html
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<link rel="author" title="Oriol Brufau" href="obrufau@igalia.com">
+<link rel="help" href="https://drafts.csswg.org/css-tables/#visibility-collapse-rendering">
+<link rel="help" href="https://drafts.csswg.org/css-tables/#border-spacing-property">
+<link rel="match" href="../reference/ref-filled-green-200px-square.html">
+<meta name="assert" content="
+  A cell spanning two rows is typically at least as tall as the vertical border-spacing.
+  However, in this case the 2nd row is collapsed, so the cell should be 0px tall.
+  This matches Gecko and Blink.">
+
+<style>
+table {
+  border-spacing: 50px 100px;
+  background: green;
+}
+td {
+  width: 100px;
+  padding: 0;
+  background: red;
+}
+tr + tr {
+  visibility: collapse;
+}
+</style>
+<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
+<table>
+  <tr>
+    <td rowspan="2"></td>
+  </tr>
+  <tr></tr>
+</table>