diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 3087b991e..a8bd4bc70 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -516,6 +516,9 @@ pub(super) fn col(s: &mut Session, right: bool) -> CmdResult { let k = before / total; t.grid.iter_mut().for_each(|w| *w *= k); } + // The inserted cells carry the pre-scale column width, and the layout prefers a cell width + // over the grid: sync them, or the shrink never reaches the layout or the saved `w:tcW`. + sync_cell_widths(t); fix_caret(s, &tp, r, if right { c + 1 } else { c }) } diff --git a/crates/engine/src/cmd/tools.rs b/crates/engine/src/cmd/tools.rs index b72c65309..22561c931 100644 --- a/crates/engine/src/cmd/tools.rs +++ b/crates/engine/src/cmd/tools.rs @@ -466,18 +466,30 @@ pub fn specs() -> Vec { }) .params(r#"{"mode": "none|readOnly|comments|trackedChanges"} (starting enforcement; "none" doesn't stop a protected document — use review.restrict {"stop": true})"#), CommandSpec::new("file.versions", "Version History", "File › Info", |s, v| { + // Validate a restore before anything is saved: a rejected command must leave the + // version list as it was. A `restore` that is there but not a non-negative integer + // is refused, not read as absent. + let restore = if v.get("restore").is_some_and(|x| !x.is_null()) { + let i = p::u64(v, "restore").ok_or_else(|| CmdError::Params("restore (non-negative integer) is required".into()))?; + // A `u64` that doesn't fit in `usize` names no version: `as usize` would wrap it + // to a small index on a 32-bit target and restore the wrong document. + let i = usize::try_from(i).map_err(|_| CmdError::Params("no such version".into()))?; + let version = s.versions.get(i).cloned().ok_or_else(|| CmdError::Params("no such version".into()))?; + // The restore replaces the whole document, including the content the protection + // kept read-only, so the protection is stopped first (with its password, when it + // has one) — a restore must not go around it. + super::protect::check_version_restore(s)?; + Some(version) + } else { + None + }; if let Some(label) = p::str(v, "save") { s.versions.push((label.to_string(), super::now_iso(), s.doc.clone())); if s.versions.len() > 50 { s.versions.remove(0); } } - if let Some(i) = p::u64(v, "restore") { - let (_, _, d) = s.versions.get(i as usize).cloned().ok_or_else(|| CmdError::Params("no such version".into()))?; - // The restore replaces the whole document, including the content the protection - // kept read-only, so the protection is stopped first (with its password, when it - // has one) — a restore must not go around it. - super::protect::check_version_restore(s)?; + if let Some((_, _, d)) = restore { s.checkpoint("Restore Version"); // A snapshot's own protection is stale: it was taken at some earlier moment, and // restoring it must not bring a protection back that the live document doesn't diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 75bb875b9..f9e95f165 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1674,6 +1674,54 @@ fn tables_commands() { assert!(s.run("table.merge", &json!({})).is_err()); } +#[test] +fn inserting_a_column_keeps_cell_widths_in_step_with_the_grid() { + let mut s = s(); + run(&mut s, "insert.table", json!({"rows": 2, "cols": 2})); + let widths = |s: &Session| { + let t = s.doc.body.iter().find_map(|b| b.as_table()).unwrap(); + (t.grid.clone(), t.rows[0].cells.iter().map(|c| c.props.width.unwrap_or(0.0)).collect::>()) + }; + let (grid_before, _) = widths(&s); + assert!(grid_before.iter().all(|w| *w > 0.0)); + run(&mut s, "table.insertColumnRight", json!({})); + let (grid_after, cells) = widths(&s); + // The table keeps its total width, and each cell carries its grid columns' width: the + // layout and the saved `w:tcW` see the same shrink the grid got. + let sum = |v: &[f32]| v.iter().sum::(); + assert!((sum(&grid_after) - sum(&grid_before)).abs() < 0.5, "{grid_after:?} vs {grid_before:?}"); + assert_eq!(cells.len(), grid_after.len()); + for (g, w) in grid_after.iter().zip(cells.iter()) { + assert!((w - g).abs() < 0.01, "cell width {w} vs grid {g}"); + } +} + +#[test] +fn a_rejected_version_restore_saves_nothing() { + let mut s = Session::new(wordcraft_doc::Document::from_text("Original text.")); + s.run("review.restrict", &json!({"mode": "readOnly", "password": "pw"})).unwrap(); + s.run("file.versions", &json!({"save": "before"})).unwrap(); + // One command asking for both a save and a restore the protection refuses: the save must + // not happen either. + let e = s.run("file.versions", &json!({"save": "mid", "restore": 0})).unwrap_err().to_string(); + assert!(e.contains("stop protection"), "{e}"); + assert_eq!(s.versions.len(), 1, "the refused command saved a version anyway"); + // A bad index is refused before anything is saved, too. + let e = s.run("file.versions", &json!({"save": "mid", "restore": 99})).unwrap_err().to_string(); + assert!(e.contains("no such version"), "{e}"); + assert_eq!(s.versions.len(), 1, "the refused command saved a version anyway"); + // A restore that is there but not a non-negative integer is refused, not read as absent: + // {"save": "mid", "restore": -1} must not save quietly without restoring. + let e = s.run("file.versions", &json!({"save": "mid", "restore": -1})).unwrap_err().to_string(); + assert!(e.contains("restore (non-negative integer)"), "{e}"); + assert_eq!(s.versions.len(), 1, "the refused command saved a version anyway"); + // An index too large for `usize` names no version: on a 32-bit target `as usize` would wrap + // it to 0 and restore the first version instead. + let e = s.run("file.versions", &json!({"save": "mid", "restore": 4294967296u64})).unwrap_err().to_string(); + assert!(e.contains("no such version"), "{e}"); + assert_eq!(s.versions.len(), 1, "the refused command saved a version anyway"); +} + /// #3931: the page as a command would leave it, laid out without running it on the document: /// Insert › Table's grid previews the table it would insert. The document, the caret, the undo /// history and Repeat stay as they were.