From 230b29be60d6506f91c0f3ee7d4fa792750196f0 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 16 Oct 2026 05:54:41 +0000 Subject: [PATCH] Refuse table row moves across the header/footer boundary Found while testing crates/doc/src/table.rs:882 (Table::move_row, called from crates/engine/src/cmd/table.rs:1437 and :1369) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/doc/src/table.rs | 17 ++++++++++++++++- crates/engine/src/cmd/table.rs | 25 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/crates/doc/src/table.rs b/crates/doc/src/table.rs index 9e5f9c167..ae7bb1ef2 100644 --- a/crates/doc/src/table.rs +++ b/crates/doc/src/table.rs @@ -878,7 +878,8 @@ impl Table { } /// Move row `from` to position `to` (indices before the move). Refused when merged cells span - /// rows (the move would cut them). + /// rows (the move would cut them) or when the move would take a row across a header/footer + /// boundary (`from` and `to` must be the same kind of row). pub fn move_row(&mut self, from: usize, to: usize) -> Result<(), String> { let (nr, nc) = (self.nrows(), self.ncols()); if from >= nr || to >= nr { @@ -887,6 +888,9 @@ impl Table { if self.regions().iter().any(|r| r.2 > 1) { return Err("unmerge cells that span rows first".into()); } + if self.rows[from].kind != self.rows[to].kind { + return Err("header and footer rows must stay first and last".into()); + } let row = self.rows.remove(from); self.rows.insert(to, row); let cells: Vec = self.cells.drain(from * nc..(from + 1) * nc).collect(); @@ -1195,6 +1199,17 @@ mod tests { Story::with_text(StoryId(1), t, ParaFormat::default()) } + #[test] + fn move_row_keeps_header_and_footer_rows_in_place() { + let mut t = Table::new(1, 3, 2, 1, 1, 120.0); + for (from, to) in [(0, 2), (0, 4), (2, 0), (2, 4), (4, 2), (4, 0)] { + assert!(t.move_row(from, to).is_err(), "move_row({from}, {to}) should be refused"); + t.check().unwrap(); + } + t.move_row(1, 3).unwrap(); + t.check().unwrap(); + } + #[test] fn selected_owners_include_covered_cells_without_expanding_range() { for column in [true, false] { diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 4b2962b4e..a8aaadb42 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -1677,6 +1677,31 @@ mod sort_tests { assert_eq!((cell(&s, 2, 0), cell(&s, 2, 1)), ("x".to_string(), "a".to_string())); } + #[test] + fn move_row_cannot_move_a_header_row_out_of_place() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let r = s.execute("frame.create", &json!({"rect": [72, 72, 500, 400], "content": "text", "text": ""})).unwrap(); + s.execute("text.select", &json!({"story": r["story"], "anchor": 0, "focus": 0})).unwrap(); + s.execute("table.insert", &json!({"rows": 3, "cols": 2, "headerRows": 1})).unwrap(); + assert!(s.execute("table.moveRow", &json!({"from": 0, "to": 2})).is_err()); + s.doc().unwrap().doc.check().unwrap(); + // Dragging the selected header row onto the last body row is refused too. + let d = s.doc().unwrap().doc.clone(); + let (sid, tid) = d.stories.values().find_map(|st| st.tables.keys().next().map(|k| (st.id, *k))).unwrap(); + let fid = d.stories[&sid].frames[0]; + let cs = s.cache.get(&d, sid, None); + let cells = &cs.frames[0].tables[0].cells; + let centre = |r: usize| { + let c = cells.iter().find(|c| c.row == r && c.col == 0).unwrap().rect.center(); + d.text_xf(d.item(fid).unwrap()) * c + }; + let (from, to) = (centre(0), centre(2)); + s.execute("table.select", &json!({"story": sid.0, "table": tid, "rows": [0, 0], "what": "row"})).unwrap(); + assert!(s.execute("table.dropCells", &json!({"frame": fid.0, "from": [from.x, from.y], "to": [to.x, to.y]})).is_err()); + s.doc().unwrap().doc.check().unwrap(); + } + #[test] fn table_options_limits_header_and_footer_rows() { let mut s = Session::new();