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>

1 files changed, 66 insertions, 63 deletions

diff --git a/components/layout_2020/table/construct.rs b/components/layout_2020/table/construct.rs
index db7d515aa3e..61f939e9300 100644
--- a/components/layout_2020/table/construct.rs
+++ b/components/layout_2020/table/construct.rs
@@ -370,82 +370,85 @@ 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.
-        let removed_row_group = self.table.row_groups.remove(index_to_move);
-        self.table.row_groups.insert(0, removed_row_group);
-
-        for row in self.table.rows.iter_mut() {
-            match row.group_index.as_mut() {
-                Some(group_index) if *group_index < index_to_move => *group_index += 1,
-                Some(group_index) if *group_index == index_to_move => *group_index = 0,
-                _ => {},
+        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);
+
+            for row in self.table.rows.iter_mut() {
+                match row.group_index.as_mut() {
+                    Some(group_index) if *group_index < index_to_move => *group_index += 1,
+                    Some(group_index) if *group_index == index_to_move => *group_index = 0,
+                    _ => {},
+                }
             }
         }
 
-        // 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();
+        let row_range = self.table.row_groups[0].track_range.clone();
+        if row_range.start > 0 {
+            // 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.splice(0..0, 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.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;
-        if index_to_move == last_row_group_index {
-            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);
-
-        // 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.extend(removed_rows);
 
         // Move the group itself.
-        let removed_row_group = self.table.row_groups.remove(index_to_move);
-        self.table.row_groups.push(removed_row_group);
-
-        for row in self.table.rows.iter_mut() {
-            match row.group_index.as_mut() {
-                Some(group_index) if *group_index > index_to_move => *group_index -= 1,
-                Some(group_index) if *group_index == index_to_move => {
-                    *group_index = last_row_group_index
-                },
-                _ => {},
+        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);
+
+            for row in self.table.rows.iter_mut() {
+                match row.group_index.as_mut() {
+                    Some(group_index) if *group_index > index_to_move => *group_index -= 1,
+                    Some(group_index) if *group_index == index_to_move => {
+                        *group_index = last_row_group_index
+                    },
+                    _ => {},
+                }
             }
         }
 
-        self.regenerate_track_ranges();
+        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