Fix reordering of table-header-group and table-footer-group (#33383)

We weren't moving a table-header-group to the front if it was the first
row group. However, there might still be preceding rows that don't
belong to any row group.

And similarly, we weren't moving a table-footer-group to the end if it
was the last row group. However, there might still be following rows
that don't belong to any row group.

This patch fixes the logic, and enables existing tests from Microsoft
that were missing a reference.

Signed-off-by: Oriol Brufau <obrufau@igalia.com>
Co-authored-by: Martin Robinson <mrobinson@igalia.com>

components/layout_2020/table/construct.rs

@@ -370,28 +370,8 @@ impl TableBuilder {
     }
 
     fn move_row_group_to_front(&mut self, index_to_move: usize) {
-        if index_to_move == 0 {
-            return;
-        }
-
-        // Move the slots associated with this group.
-        let row_range = self.table.row_groups[index_to_move].track_range.clone();
-        let removed_slots: Vec<Vec<TableSlot>> = self
-            .table
-            .slots
-            .splice(row_range.clone(), std::iter::empty())
-            .collect();
-        self.table.slots.splice(0..0, removed_slots);
-
-        // Move the rows associated with this group.
-        let removed_rows: Vec<TableTrack> = self
-            .table
-            .rows
-            .splice(row_range, std::iter::empty())
-            .collect();
-        self.table.rows.splice(0..0, removed_rows);
-
         // Move the group itself.
+        if index_to_move > 0 {
             let removed_row_group = self.table.row_groups.remove(index_to_move);
             self.table.row_groups.insert(0, removed_row_group);
 
@@ -402,36 +382,37 @@ impl TableBuilder {
                     _ => {},
                 }
             }
-
-        // Do this now, rather than after possibly moving a `<tfoot>` row group to the end,
-        // because moving row groups depends on an accurate `track_range` in every group.
-        self.regenerate_track_ranges();
         }
 
-    fn move_row_group_to_end(&mut self, index_to_move: usize) {
+        let row_range = self.table.row_groups[0].track_range.clone();
-        let last_row_group_index = self.table.row_groups.len() - 1;
+        if row_range.start > 0 {
-        if index_to_move == last_row_group_index {
+            // Move the slots associated with the moved group.
-            return;
-        }
-
-        // Move the slots associated with this group.
-        let row_range = self.table.row_groups[index_to_move].track_range.clone();
             let removed_slots: Vec<Vec<TableSlot>> = self
                 .table
                 .slots
                 .splice(row_range.clone(), std::iter::empty())
                 .collect();
-        self.table.slots.extend(removed_slots);
+            self.table.slots.splice(0..0, removed_slots);
 
-        // Move the rows associated with this group.
+            // Move the rows associated with the moved group.
             let removed_rows: Vec<TableTrack> = self
                 .table
                 .rows
                 .splice(row_range, std::iter::empty())
                 .collect();
-        self.table.rows.extend(removed_rows);
+            self.table.rows.splice(0..0, removed_rows);
+
+            // Do this now, rather than after possibly moving a `<tfoot>` row group to the end,
+            // because moving row groups depends on an accurate `track_range` in every group.
+            self.regenerate_track_ranges();
+        }
+    }
+
+    fn move_row_group_to_end(&mut self, index_to_move: usize) {
+        let last_row_group_index = self.table.row_groups.len() - 1;
 
         // Move the group itself.
+        if index_to_move < last_row_group_index {
             let removed_row_group = self.table.row_groups.remove(index_to_move);
             self.table.row_groups.push(removed_row_group);
 
@@ -444,9 +425,31 @@ impl TableBuilder {
                     _ => {},
                 }
             }
+        }
+
+        let row_range = self.table.row_groups[last_row_group_index]
+            .track_range
+            .clone();
+        if row_range.end < self.table.rows.len() {
+            // Move the slots associated with the moved group.
+            let removed_slots: Vec<Vec<TableSlot>> = self
+                .table
+                .slots
+                .splice(row_range.clone(), std::iter::empty())
+                .collect();
+            self.table.slots.extend(removed_slots);
+
+            // Move the rows associated with the moved group.
+            let removed_rows: Vec<TableTrack> = self
+                .table
+                .rows
+                .splice(row_range, std::iter::empty())
+                .collect();
+            self.table.rows.extend(removed_rows);
 
             self.regenerate_track_ranges();
         }
+    }
 
     /// Turn all rowspan=0 rows into the real value to avoid having to make the calculation
     /// continually during layout. In addition, make sure that there are no rowspans that extend

tests/wpt/meta/MANIFEST.json

@@ -108798,6 +108798,32 @@
         {}
        ]
       ],
+      "table-footer-group-001.xht": [
+       "e32e2dac22df85b038237fbd59a9fb8d85708ff6",
+       [
+        null,
+        [
+         [
+          "/css/CSS2/tables/table-row-group-001-ref.xht",
+          "=="
+         ]
+        ],
+        {}
+       ]
+      ],
+      "table-header-group-001.xht": [
+       "0d8f4798052d5ff68fc2fd05e7d5ec29d107b3ec",
+       [
+        null,
+        [
+         [
+          "/css/CSS2/tables/table-row-group-001-ref.xht",
+          "=="
+         ]
+        ],
+        {}
+       ]
+      ],
       "table-height-algorithm-008a.xht": [
        "eaaf04f759be858bbe0d8b103930748bfd4bf1e7",
        [
@@ -108863,6 +108889,19 @@
         {}
        ]
       ],
+      "table-row-group-001.xht": [
+       "a0f7966e35e68b0fea10309dcdc83b51e2e15372",
+       [
+        null,
+        [
+         [
+          "/css/CSS2/tables/table-row-group-001-ref.xht",
+          "=="
+         ]
+        ],
+        {}
+       ]
+      ],
       "table-vertical-align-baseline-001.xht": [
        "1862eb41f7f14cb6f77320f6e1e3fa7986c9ff00",
        [
@@ -391423,6 +391462,10 @@
        "8ea849424688a0f3a3b9a345b32e53b6ad1d76c5",
        []
       ],
+      "table-row-group-001-ref.xht": [
+       "c58134ae3594a80e3512b1aeac8957ec0d3a1ef9",
+       []
+      ],
       "table-vertical-align-baseline-001-ref.xht": [
        "9d4b753a6dd0c1cba08d83d80180459e4f2ad593",
        []
@@ -849035,13 +849078,6 @@
         {}
        ]
       ],
-      "table-footer-group-001.xht": [
-       "fb2338caf47a312e9580e04333ada107762c4a9b",
-       [
-        null,
-        {}
-       ]
-      ],
       "table-footer-group-002.xht": [
        "a041b45789125f283f5f033417ec5d397b73f29c",
        [
@@ -849063,13 +849099,6 @@
         {}
        ]
       ],
-      "table-header-group-001.xht": [
-       "0b3a595cb77ba8f90ab87c2ec34cbdf29ddf0a06",
-       [
-        null,
-        {}
-       ]
-      ],
       "table-header-group-002.xht": [
        "0e03b0a045f8ff3bee300288f0fac4f4c3469021",
        [
@@ -849609,13 +849638,6 @@
         {}
        ]
       ],
-      "table-row-group-001.xht": [
-       "d5a85400ae64a21080c424aeae2b7f919b818426",
-       [
-        null,
-        {}
-       ]
-      ],
       "table-valign-001.xht": [
        "021a3e76e04eee628237b1a8473ab0d7b5e9c282",
        [

tests/wpt/tests/css/CSS2/tables/table-footer-group-001.xht

@@ -4,6 +4,7 @@
         <title>CSS Test: Table-footer-group</title>
         <link rel="author" title="Microsoft" href="http://www.microsoft.com/" />
         <link rel="help" href="http://www.w3.org/TR/CSS21/tables.html#table-display" />
+        <link rel="match" href="table-row-group-001-ref.xht" />
         <meta name="assert" content="An element with 'display: table-footer-group' is rendered as if it were a table footer group." />
         <style type="text/css">
             .table

tests/wpt/tests/css/CSS2/tables/table-header-group-001.xht

@@ -4,6 +4,7 @@
         <title>CSS Test: Table-header-group</title>
         <link rel="author" title="Microsoft" href="http://www.microsoft.com/" />
         <link rel="help" href="http://www.w3.org/TR/CSS21/tables.html#table-display" />
+        <link rel="match" href="table-row-group-001-ref.xht" />
         <meta name="assert" content="An element with 'display: table-header-group' is rendered as if it were a table header group." />
         <style type="text/css">
             .table

tests/wpt/tests/css/CSS2/tables/table-row-group-001-ref.xht

@@ -0,0 +1,9 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml">
+    <body>
+        <p>Test passes if there is a square below, and the top half of the square is blue.</p>
+        <div style="width: 8em; height: 8em; border: 2px solid black">
+            <div style="height: 4em; background: blue"></div>
+        </div>
+    </body>
+</html>

tests/wpt/tests/css/CSS2/tables/table-row-group-001.xht

@@ -4,6 +4,7 @@
         <title>CSS Test: Table-row-group</title>
         <link rel="author" title="Microsoft" href="http://www.microsoft.com/" />
         <link rel="help" href="http://www.w3.org/TR/CSS21/tables.html#table-display" />
+        <link rel="match" href="table-row-group-001-ref.xht" />
         <meta name="assert" content="An element with 'display: table-row-group' is rendered as if it were a table row group." />
         <style type="text/css">
             .table