From add48514432ffbece30b494d8491c195ae742f8e Mon Sep 17 00:00:00 2001 From: Dmitrii Khlapov Date: Thu, 15 Oct 2026 06:55:10 +0200 Subject: [PATCH] layout.pageSetup validates its section values (#1582) (#1584) * #1582: WIP * #1582: layout.pageSetup {section} validates like Size, Margins and Columns Co-Authored-By: Claude Opus 5.5 * #1582: cargo fmt Co-Authored-By: Claude Opus 5.5 * #1582: shared page limits; notes, borders and columns validated too Co-Authored-By: Claude Opus 5.5 * #1582: page limits are pub, as in #1496 Co-Authored-By: Claude Opus 5.5 * #1582: drop page constants #1496 now defines on main Co-Authored-By: Claude Opus 5.5 * #1582: a page smaller than 36 pt passes Page Setup with zero margins Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Dmitrii Khlapov <19991457+dkhlapov@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 Co-authored-by: Brandon Thomas --- ROADMAP.md | 2 + crates/engine/src/cmd/page.rs | 230 +++++++++++++++++++++++++++++++++- crates/engine/src/tests.rs | 1 + 3 files changed, 230 insertions(+), 3 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 734932a10..7663dc43c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -170,6 +170,7 @@ Ranked, to beta (detail and remaining milestones in [docs/roadmap.md](docs/roadm | 2026-10-14 | All Caps and Small Caps draw ß as SS, as Word does (#1581): "Straße" in All Caps reads STRASSE, not STRASE, and other letters whose capital is two characters (ff, ʼn) draw in full. The ß is still one character: the caret never stops between the two S, and a click on them lands before or after it | | 2026-10-13 | Windows Magnifier follows the caret onto a new line: after Enter or Shift+Enter the view moves to the empty line at once, instead of staying on the line above until two more letters are typed (#1119) | | 2026-10-13 | A window too big for its screen shrinks to fit when it opens (#1215): on a laptop screen the first window reached under the taskbar or past the screen edge, with the end of the scroll bar, so dragging the scroll bar stopped where the pointer met the edge. A window saved that size is fitted on its next launch too | +| 2026-10-14 | `layout.pageSetup` with a `section` checks its values like Margins, Size and Columns (#1582): non-finite numbers and margins that leave no room for text are refused with nothing changed; page sides, column count and gap, header/footer distances and line-numbering values are clamped to the same ranges | | 2026-10-13 | `wordcraft --control` checks its port (#1426): a missing port, one outside 1–65535 or one that isn't a number prints an error naming it and exits with status 2, instead of starting without the control channel (and dropping `WORDCRAFT_CONTROL_PORT`); a flag right after it, such as `--sample` or `--version`, is read as a flag rather than as the port | | 2026-10-14 | Text wrap (#1376): in the narrow strip beside a square-wrapped picture, a word that doesn't fit moves on whole to the next row; it used to be split across the strip ("docu" / "mentazione") | | 2026-10-14 | DOCX files with legacy VML preset shapes show them (#1377): Word's right arrow (`_x0000_t13`) and the other presets appear with their fill, outline and size, drawn as WordCraft draws the same DrawingML preset (the nearest shape when it has no exact one), and saving keeps them as DrawingML shapes; before, they were dropped | @@ -429,6 +430,7 @@ Ranked, to beta (detail and remaining milestones in [docs/roadmap.md](docs/roadm | 2026-10-14 | trivial | Progress log: All Caps and Small Caps draw ß as SS (#1581) | | 2026-10-13 | trivial | Progress log: Windows Magnifier follows the caret onto a new line (#1119) | | 2026-10-13 | trivial | Progress log: a window too big for its screen shrinks to fit (#1215) | +| 2026-10-14 | trivial | Progress log: Page Setup sections are validated (#1582) | | 2026-10-13 | trivial | Progress log: `--control` checks its port (#1426) | | 2026-10-14 | trivial | Progress log: words beside a floating picture are no longer split (#1376) | | 2026-10-14 | trivial | Progress log: VML preset shapes (#1377) | diff --git a/crates/engine/src/cmd/page.rs b/crates/engine/src/cmd/page.rs index 66a38fa60..4cee3bd96 100644 --- a/crates/engine/src/cmd/page.rs +++ b/crates/engine/src/cmd/page.rs @@ -1,8 +1,9 @@ //! Layout tab: page setup (margins, orientation, size, columns), breaks, line numbers, hyphenation. use serde_json::{Value, json}; +use wordcraft_doc::props::{Border, Borders}; use wordcraft_doc::section::{ - Columns, DocGrid, DocGridType, LineNumbering, MAX_CHAR_SPACE, MAX_GRID_PITCH, MIN_GRID_PITCH, SectionProps, SectionStart, + Columns, DocGrid, DocGridType, LineNumbering, MAX_CHAR_SPACE, MAX_GRID_PITCH, MAX_NOTE_START, MIN_GRID_PITCH, SectionProps, SectionStart, }; use wordcraft_doc::{Block, Pos, StoryRef}; @@ -40,13 +41,14 @@ pub fn specs() -> Vec { .pure(), CommandSpec::new("layout.pageSetup", "Page Setup", "Layout › Page Setup", |s, v| { if let Some(props) = v.get("section") { - let new: SectionProps = serde_json::from_value(props.clone()).map_err(|e| CmdError::Params(e.to_string()))?; + let mut new: SectionProps = serde_json::from_value(props.clone()).map_err(|e| CmdError::Params(e.to_string()))?; + validate_section(&mut new)?; return with_sect(s, |x| *x = new.clone()); } s.ui_requests.push(json!({"open": "pageSetup"})); Ok(serde_json::to_value(sect(s)).unwrap_or(Value::Null)) }) - .params(r#"{"section"?: SectionProps}"#), + .params(r#"{"section"?: SectionProps} (replaces the whole section, headers and footers included; checked like Size, Margins, Columns, Line Numbers, Footnotes and Page Borders: non-finite numbers and margins that leave no room for text are refused; page sides 7.2–1584 pt, 1–12 columns with 0–300 pt gaps, header/footer/gutter 0–1584 pt, page border width 0.25–6 pt and distance 0–31 pt, note start 1–32767, line numbers start 1–32767, countBy 1–100, distance 0–1584 pt are clamped)"#), CommandSpec::new("layout.verticalAlign", "Vertical Alignment", "Layout › Page Setup › Layout", |s, v| { let va = wordcraft_doc::section::PageVAlign::parse(p::str(v, "value").unwrap_or_default()); with_sect(s, |x| x.valign = va) @@ -371,6 +373,90 @@ pub fn columns_preset_at(s: &Session) -> Option<&'static str> { } } +/// Checks a whole section handed to `layout.pageSetup` by the rules of Size, Margins, Columns, +/// Line Numbers, Footnotes and Page Borders: a non-finite number or margins that leave no room +/// for text is an error; the rest is clamped to the ranges those commands use. +fn validate_section(x: &mut SectionProps) -> Result<(), CmdError> { + let bad = |what: &str| CmdError::Params(format!("section: {what} must be a finite number")); + let finite = [x.page_w, x.page_h, x.margin_top, x.margin_bottom, x.margin_left, x.margin_right, x.header, x.footer, x.gutter, x.columns.space]; + if finite.iter().any(|n| !n.is_finite()) { + return Err(bad("sizes, margins, distances and column gaps")); + } + if x.columns.widths.iter().any(|(w, sp)| !w.is_finite() || !sp.is_finite()) { + return Err(bad("column widths")); + } + if x.line_numbers.as_ref().is_some_and(|l| !l.distance.is_finite()) { + return Err(bad("the line number distance")); + } + if x.doc_grid.as_ref().is_some_and(|g| !g.line_pitch.is_finite()) { + return Err(bad("the grid pitch")); + } + if x.page_borders.as_ref().is_some_and(|b| border_sides(b).any(|d| !d.width.is_finite() || !d.space.is_finite())) { + return Err(bad("page border widths and distances")); + } + x.page_w = x.page_w.clamp(MIN_PAGE_SIDE, MAX_PAGE_SIDE); + x.page_h = x.page_h.clamp(MIN_PAGE_SIDE, MAX_PAGE_SIDE); + // Like Size, the orientation follows the sides. + if x.page_w != x.page_h { + x.landscape = x.page_w > x.page_h; + } + x.margin_top = x.margin_top.max(0.0); + x.margin_bottom = x.margin_bottom.max(0.0); + x.margin_left = x.margin_left.max(0.0); + x.margin_right = x.margin_right.max(0.0); + x.header = x.header.clamp(0.0, x.page_h); + x.footer = x.footer.clamp(0.0, x.page_h); + x.gutter = x.gutter.clamp(0.0, MAX_PAGE_SIDE); + // A page narrower than the usual text room (the smallest is 0.1") needs only margins that + // leave the whole of it: zero margins, gutter included. + let room = |side: f32| MIN_TEXT_ROOM.min(side); + if x.margin_left + x.margin_right + x.gutter > x.page_w - room(x.page_w) || x.margin_top + x.margin_bottom > x.page_h - room(x.page_h) { + return Err(CmdError::Params("margins leave no room for text".into())); + } + x.columns.space = x.columns.space.clamp(0.0, 300.0); + x.columns.widths.truncate(12); + for (w, sp) in &mut x.columns.widths { + *w = w.clamp(18.0, MAX_PAGE_SIDE); + *sp = sp.clamp(0.0, 300.0); + } + // As in Columns: custom widths set the count; a single width is no custom layout. + if x.columns.widths.len() > 1 { + x.columns.count = x.columns.widths.len() as u32; + } else { + x.columns.widths.clear(); + x.columns.count = x.columns.count.clamp(1, 12); + } + if let Some(l) = &mut x.line_numbers { + l.start = l.start.clamp(1, 32_767); + l.count_by = l.count_by.clamp(1, 100); + l.distance = l.distance.clamp(0.0, 1584.0); + } + if let Some(g) = &mut x.doc_grid { + g.line_pitch = g.line_pitch.clamp(MIN_GRID_PITCH, MAX_GRID_PITCH); + g.char_space = g.char_space.clamp(-MAX_CHAR_SPACE, MAX_CHAR_SPACE); + } + x.page_num_start = x.page_num_start.map(|n| n.min(100_000)); + for n in [&mut x.footnote_pr, &mut x.endnote_pr] { + n.num_start = n.num_start.map(|v| v.clamp(1, MAX_NOTE_START)); + } + // Page Borders: width 0.25–6 pt, distance from the text 0–31 pt. + if let Some(b) = &mut x.page_borders { + for d in border_sides_mut(b) { + d.width = d.width.clamp(0.25, 6.0); + d.space = d.space.clamp(0.0, 31.0); + } + } + Ok(()) +} + +fn border_sides(b: &Borders) -> impl Iterator { + [&b.top, &b.left, &b.bottom, &b.right, &b.between, &b.inside_v].into_iter().flatten() +} + +fn border_sides_mut(b: &mut Borders) -> impl Iterator { + [&mut b.top, &mut b.left, &mut b.bottom, &mut b.right, &mut b.between, &mut b.inside_v].into_iter().flatten() +} + fn margins(s: &mut Session, v: &Value) -> CmdResult { let preset = p::str(v, "preset"); let (t, b, l, r) = match preset { @@ -600,6 +686,144 @@ mod tests { Session::new(wordcraft_doc::Document::from_text("one\ntwo\nthree")) } + fn with_section(f: impl Fn(&mut Value)) -> Value { + let mut v = serde_json::to_value(SectionProps::default()).unwrap(); + f(&mut v); + json!({"section": v}) + } + + /// `layout.pageSetup {section}` checks the values like the dedicated commands do. + #[test] + fn page_setup_section_is_validated() { + let mut s = doc(); + let before = sect(&s); + // Refused with an error, nothing changes: non-finite (overflowing), margins that leave no text room. + for (what, p) in [ + ("inf width", with_section(|v| v["pageW"] = json!(1e300))), + ("inf margin", with_section(|v| v["marginTop"] = json!(1e300))), + ("inf header", with_section(|v| v["header"] = json!(1e300))), + ("inf column space", with_section(|v| v["columns"]["space"] = json!(1e300))), + ("inf column width", with_section(|v| v["columns"]["widths"] = json!([[1e300, 0.0], [100.0, 0.0]]))), + ("inf line number distance", with_section(|v| v["lineNumbers"] = json!({"distance": 1e300}))), + ("inf grid pitch", with_section(|v| v["docGrid"] = json!({"type": "lines", "linePitch": 1e300, "charSpace": 0}))), + ("inf border space", with_section(|v| v["pageBorders"] = json!({"top": {"style": "single", "width": 1.0, "space": 1e300}}))), + ("inf border width", with_section(|v| v["pageBorders"] = json!({"top": {"style": "single", "width": 1e300, "space": 1.0}}))), + ("margins > page", with_section(|v| v["marginLeft"] = json!(5000.0))), + ("margins eat the height", with_section(|v| v["marginTop"] = json!(700.0))), + ("gutter eats the width", with_section(|v| v["gutter"] = json!(600.0))), + ("negative page", with_section(|v| v["pageW"] = json!(-5.0))), + ("zero page height", with_section(|v| v["pageH"] = json!(0.0))), + ] { + assert!(s.run("layout.pageSetup", &p).is_err(), "{what}"); + assert_eq!(sect(&s), before, "{what}"); + } + // Clamped. + s.run( + "layout.pageSetup", + &with_section(|v| { + v["pageW"] = json!(100.0); + v["pageH"] = json!(100.0); + v["marginTop"] = json!(-10.0); + v["marginBottom"] = json!(0.0); + v["marginLeft"] = json!(0.0); + v["marginRight"] = json!(0.0); + v["header"] = json!(-3.0); + v["footer"] = json!(1e6); + v["columns"]["count"] = json!(1000); + v["columns"]["space"] = json!(-4.0); + v["lineNumbers"] = json!({"countBy": 0, "start": 0, "distance": 1e6, "restart": "page"}); + v["docGrid"] = json!({"type": "lines", "linePitch": 1e9, "charSpace": 2_000_000_000}); + v["pageNumStart"] = json!(4_000_000_000u64); + v["footnotePr"] = json!({"numStart": 4_000_000_000u64}); + v["endnotePr"] = json!({"numStart": 0}); + v["pageBorders"] = + json!({"top": {"style": "single", "width": 1e6, "space": -5.0}, "left": {"style": "single", "width": 0.0, "space": 1e6}}); + }), + ) + .unwrap(); + let x = sect(&s); + assert_eq!((x.page_w, x.page_h), (100.0, 100.0)); + assert_eq!(x.margin_top, 0.0); + assert_eq!(x.header, 0.0); + assert_eq!(x.footer, x.page_h, "a footer distance stays on the page"); + assert_eq!(x.columns.count, 12); + assert_eq!(x.columns.space, 0.0); + let l = x.line_numbers.unwrap(); + assert_eq!((l.count_by, l.start, l.distance), (1, 1, 1584.0)); + let g = x.doc_grid.unwrap(); + assert_eq!((g.line_pitch, g.char_space), (wordcraft_doc::section::MAX_GRID_PITCH, wordcraft_doc::section::MAX_CHAR_SPACE)); + assert_eq!(x.page_num_start, Some(100_000)); + assert_eq!(x.footnote_pr.num_start, Some(wordcraft_doc::section::MAX_NOTE_START)); + assert_eq!(x.endnote_pr.num_start, Some(1)); + let b = x.page_borders.unwrap(); + let (t, lf) = (b.top.unwrap(), b.left.unwrap()); + assert_eq!((t.width, t.space), (6.0, 0.0)); + assert_eq!((lf.width, lf.space), (0.25, 31.0)); + // Huge pages clamp; columns and widths agree; orientation follows the sides. + let mut s = doc(); + s.run( + "layout.pageSetup", + &with_section(|v| { + v["pageW"] = json!(1e5); + v["pageH"] = json!(792.0); + v["landscape"] = json!(false); + v["columns"]["count"] = json!(3); + v["columns"]["widths"] = json!(vec![json!([1.0, 1e5]); 40]); + }), + ) + .unwrap(); + let x = sect(&s); + assert_eq!(x.page_w, 1584.0); + assert!(x.landscape, "wider than tall is landscape"); + assert_eq!(x.columns.widths.len(), 12); + assert_eq!(x.columns.count, 12, "the count follows the widths"); + assert!(x.columns.widths.iter().all(|(w, sp)| *w >= 18.0 && (0.0..=300.0).contains(sp))); + // One width is no custom layout: equal columns. + s.run("layout.pageSetup", &with_section(|v| v["columns"]["widths"] = json!([[200.0, 0.0]]))).unwrap(); + assert!(sect(&s).columns.widths.is_empty()); + } + + /// A valid section still applies as given. + #[test] + fn page_setup_section_applies_a_valid_one() { + let mut s = doc(); + s.run( + "layout.pageSetup", + &with_section(|v| { + v["pageW"] = json!(595.0); + v["pageH"] = json!(842.0); + v["marginLeft"] = json!(50.0); + v["columns"]["count"] = json!(2); + }), + ) + .unwrap(); + let x = sect(&s); + assert_eq!((x.page_w, x.page_h, x.margin_left, x.columns.count), (595.0, 842.0, 50.0, 2)); + } + + /// A page smaller than the usual 36 pt of text room is fine with no margins; any margin or + /// gutter on it is refused. + #[test] + fn page_setup_section_allows_a_tiny_page_without_margins() { + let tiny = |m: f32, g: f32| { + with_section(|v| { + v["pageW"] = json!(7.2); + v["pageH"] = json!(20.0); + for k in ["marginTop", "marginBottom", "marginLeft", "marginRight"] { + v[k] = json!(m); + } + v["gutter"] = json!(g); + }) + }; + let mut s = doc(); + let before = sect(&s); + assert!(s.run("layout.pageSetup", &tiny(1.0, 0.0)).is_err(), "a margin on a tiny page"); + assert!(s.run("layout.pageSetup", &tiny(0.0, 1.0)).is_err(), "a gutter on a tiny page"); + assert_eq!(sect(&s), before); + s.run("layout.pageSetup", &tiny(0.0, 0.0)).unwrap(); + assert_eq!((sect(&s).page_w, sect(&s).page_h), (7.2, 20.0)); + } + /// Layout › Text Direction turns the section, or the table cell the caret is in; one undo step. #[test] fn text_direction_turns_the_section_or_the_cell() { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 79df10556..7e5b9b09d 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2692,6 +2692,7 @@ fn hostile_params_never_panic() { json!({"text": 5, "size": "x", "path": [], "pos": {"block": 9999, "off": 99999}}), json!({"value": -1e308, "rows": 1e9}), json!({"width": 1e-9, "height": 1e30, "top": -1e9, "left": 1e9}), + json!({"section": {"pageW": -1e308, "pageH": 0, "marginTop": 1e30, "header": -5, "columns": {"count": 1000000, "space": -1e300, "widths": [[0, 0]]}}}), ]; for spec in reg.all() { // These reach outside the session (files). Read Aloud (`review.readAloud`, `readAloud.*`)