Merge pull request #2917 from dotCooCoo/fix/menu-commands-refuse-text-numbers

Transform Each, Rasterize, Mosaic, Trim Marks and Rearrange: a number given as text is refused

crates/engine/src/cmd/menucmds.rs

@@ -341,6 +341,7 @@ pub(crate) fn transform_each_again(s: &mut Session, p: &Value) -> Result<Value>
 /// repeats; a dialog's preview records it as it commits (`Session::commit_interaction`).
 fn transform_each_as(s: &mut Session, p: &Value, label: &str) -> Result<Value> {
     let roots = selected_roots(s)?;
+    check_numbers(p, &["scaleH", "scaleV", "moveH", "moveV", "rotate"], "object.transformEach")?;
     // Bounded so coordinates stay finite (and the document stays serializable).
     let sh = f64_or(p, "scaleH", 100.0).clamp(-100_000.0, 100_000.0);
     let sv = f64_or(p, "scaleV", 100.0).clamp(-100_000.0, 100_000.0);
@@ -480,6 +481,7 @@ impl Rasterized {
 /// `cmd`, each defaulting to `doc`'s raster effects settings: over the bounds of what the art draws
 /// ([`vectorcraft_render::drawn_bounds`]) and the room around them, in whole pixels.
 pub(crate) fn rasterize_art(doc: &vectorcraft_doc::Document, nodes: Vec<Node>, p: &Value, cmd: &str) -> Result<Rasterized> {
+    check_numbers(p, &["ppi"], cmd)?;
     let ppi = f64_or(p, "ppi", doc.raster_effects_ppi);
     if !(1.0..=2400.0).contains(&ppi) {
         return Err(bad(cmd, "ppi must be between 1 and 2400"));
@@ -600,6 +602,7 @@ fn object_mosaic(s: &mut Session, p: &Value) -> Result<Value> {
     let colors = vectorcraft_trace::mosaic(&raster, cols, rows);
     // Tiles in image space, placed by the image transform (scaled to a new size if asked).
     let b = node.geometric_bounds().ok_or_else(|| bad(C, "image has no bounds"))?;
+    check_numbers(p, &["width", "height", "spacingX", "spacingY"], C)?;
     let (nw, nh) = (f64_or(p, "width", b.width()).max(0.1), f64_or(p, "height", b.height()).max(0.1));
     let resize = Affine::translate(b.origin().to_vec2())
         * Affine::scale_non_uniform(nw / b.width().max(1e-9), nh / b.height().max(1e-9))
@@ -671,8 +674,8 @@ fn crop_image(s: &mut Session, p: &Value) -> Result<Value> {
         return trim_image(s, id, (**im).clone());
     }
     let ib = node.geometric_bounds().ok_or_else(|| bad(C, "image has no bounds"))?;
-    let rect = match p.get("rect").and_then(Value::as_array) {
-        Some(a) if a.len() == 4 => {
+    let rect = match p.get("rect").filter(|v| !v.is_null()) {
+        Some(Value::Array(a)) if a.len() == 4 => {
             let v: Vec<f64> = a.iter().filter_map(Value::as_f64).collect();
             if v.len() != 4 {
                 return Err(bad(C, "rect must be [x, y, width, height]"));
@@ -832,6 +835,7 @@ impl Session {
 
 fn trim_marks(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "object.createTrimMarks";
+    check_numbers(p, &["offset", "length", "bleed", "weight"], C)?;
     let style = match str_param(p, "style") {
         Some(v) => MarkStyle::parse(v).ok_or_else(|| bad(C, "`style` must be roman or japanese"))?,
         None => s.crop_mark_style(),
@@ -1001,6 +1005,7 @@ fn rearrange_artboards(s: &mut Session, p: &Value) -> Result<Value> {
         L::Row => (1, n_ab),
         L::Column => (n_ab, 1),
     };
+    check_numbers(p, &["spacing"], CMD)?;
     let spacing = f64_or(p, "spacing", 20.0).clamp(-1.0e5, 1.0e5);
     let move_art = bool_or(p, "moveArtwork", true);
     let locked_and_hidden = s.prefs.move_locked_with_artboard;

crates/engine/src/tests.rs

@@ -2452,6 +2452,29 @@ fn object_mosaic_refuses_columns_and_rows_that_are_not_whole_numbers() {
     assert_eq!(r["tiles"], json!(2));
 }
 
+/// A new width or height, or a spacing, given as text or as another value that is not a number is an error naming the
+/// key, and no mosaic is made.
+#[test]
+fn object_mosaic_refuses_a_size_or_spacing_given_as_text() {
+    let mut s = session();
+    rect(&mut s, 0.0, 0.0, 100.0, 50.0);
+    s.execute("object.rasterize", &json!({"ppi": 72})).unwrap();
+    let undo = |s: &Session| s.doc().unwrap().history.undo.len();
+    let before = undo(&s);
+    for (p, msg) in [
+        (json!({"width": "200"}), "`width` must be a number, not \"200\""),
+        (json!({"columns": 2, "height": "50 pt"}), "`height` must be a number, not \"50 pt\""),
+        (json!({"spacingX": "4"}), "`spacingX` must be a number, not \"4\""),
+        (json!({"spacingY": [4]}), "`spacingY` must be a number, not [4]"),
+    ] {
+        let r = s.execute("object.createObjectMosaic", &p);
+        assert!(matches!(&r, Err(EngineError::BadParams { msg: m, .. }) if m == msg), "{p}: {r:?}");
+        assert_eq!(undo(&s), before, "{p}");
+    }
+    let r = s.execute("object.createObjectMosaic", &json!({"columns": 2, "rows": 1, "width": 200, "spacingX": null})).unwrap();
+    assert_eq!(r["tiles"], json!(2));
+}
+
 #[test]
 fn snap_to_pixel_rounds_drawing_and_moves() {
     let mut s = session();

crates/engine/src/tests_menucmds.rs

@@ -83,6 +83,13 @@ fn close(a: f64, b: f64) -> bool {
     (a - b).abs() < 1e-6
 }
 
+/// `cmd` with `p` fails with the error for `key`, which `p` gives as something other than a number.
+fn refuses_non_number(s: &mut Session, cmd: &str, p: &Value, key: &str) {
+    let r = s.execute(cmd, p);
+    let want = format!("`{key}` must be a number, not {}", p[key]);
+    assert!(matches!(&r, Err(EngineError::BadParams { msg, .. }) if *msg == want), "{cmd} {p}: {r:?}");
+}
+
 // ---------- Object → Lock / Hide ----------
 
 #[test]
@@ -399,6 +406,29 @@ fn transform_each_refuses_a_reference_or_seed_that_is_not_a_whole_number() {
     assert_eq!(bounds(&s, a), Rect::new(0.0, 0.0, 50.0, 50.0));
 }
 
+/// Transform Each refuses a scale, move or rotation given as text or as another value that is not a
+/// number, and the object stays as it was; numbers for the scales and the horizontal move transform it.
+#[test]
+fn transform_each_refuses_a_number_given_as_text() {
+    let mut s = session();
+    let a = rect(&mut s, 0.0, 0.0, 100.0, 100.0);
+    sel(&mut s, &[a]);
+    let before = (undo_len(&s), bounds(&s, a));
+    for (key, p) in [
+        ("scaleH", json!({"scaleH": "50"})),
+        ("scaleV", json!({"scaleH": 50, "scaleV": "50%"})),
+        ("moveH", json!({"moveH": "10"})),
+        ("moveV", json!({"moveH": 10, "moveV": [5]})),
+        ("rotate", json!({"rotate": "45"})),
+        ("rotate", json!({"moveH": 10, "rotate": true})),
+    ] {
+        refuses_non_number(&mut s, "object.transformEach", &p, key);
+        assert_eq!((undo_len(&s), bounds(&s, a)), before, "{p}");
+    }
+    s.execute("object.transformEach", &json!({"scaleH": 50, "scaleV": 50, "moveH": 10, "moveV": null})).unwrap();
+    assert_eq!(bounds(&s, a), Rect::new(35.0, 25.0, 85.0, 75.0));
+}
+
 /// #1910: Transform Each's Reflect X flips left-right; Reflect Y flips top-bottom (same as
 /// Effect › Distort › Transform and Illustrator's Transform Each checkboxes).
 #[test]
@@ -571,6 +601,22 @@ fn rasterize_replaces_selection_with_image() {
     assert_eq!(selected(&s), vec![img]);
 }
 
+/// Rasterize refuses a resolution given as text or as another value that is not a number, and the
+/// selected art stays as it was.
+#[test]
+fn rasterize_refuses_a_ppi_given_as_text() {
+    let mut s = session();
+    let a = rect(&mut s, 10.0, 20.0, 40.0, 30.0);
+    sel(&mut s, &[a]);
+    let before = (undo_len(&s), node(&s, a));
+    for p in [json!({"ppi": "144"}), json!({"ppi": "300 ppi", "padding": 0}), json!({"ppi": [72]})] {
+        refuses_non_number(&mut s, "object.rasterize", &p, "ppi");
+        assert_eq!((undo_len(&s), node(&s, a)), before, "{p}");
+    }
+    let img = id_of(&s.execute("object.rasterize", &json!({"ppi": 144, "padding": 0})).unwrap());
+    assert!(matches!(node(&s, img).kind, NodeKind::Image(_)));
+}
+
 #[test]
 fn rasterize_says_when_an_image_could_not_be_decoded() {
     let mut s = session();
@@ -608,6 +654,25 @@ fn crop_image_to_rect() {
     assert!(s.execute("object.cropImage", &json!({})).is_err());
 }
 
+/// Crop Image refuses a `rect` that is not an array, such as text, and the image stays as it was; a
+/// null `rect` crops to the artboard as a missing one does.
+#[test]
+fn crop_image_refuses_a_rect_given_as_text() {
+    let mut s = session();
+    let a = rect(&mut s, -20.0, -20.0, 100.0, 100.0);
+    sel(&mut s, &[a]);
+    let img = id_of(&s.execute("object.rasterize", &json!({"ppi": 72})).unwrap());
+    let before = (undo_len(&s), node(&s, img));
+    for p in [json!({"rect": "0, 0, 20, 20"}), json!({"rect": {"x": 0, "y": 0, "width": 20, "height": 20}}), json!({"rect": 20})] {
+        let r = s.execute("object.cropImage", &p);
+        assert!(matches!(&r, Err(EngineError::BadParams { msg, .. }) if msg == "rect must be [x, y, width, height]"), "{p}: {r:?}");
+        assert_eq!((undo_len(&s), node(&s, img)), before, "{p}");
+    }
+    s.execute("object.cropImage", &json!({"rect": null})).unwrap();
+    let b = bounds(&s, img);
+    assert!(close(b.x0, 0.0) && close(b.y0, 0.0), "{b:?}");
+}
+
 #[test]
 fn crop_image_disabled_without_image() {
     let mut s = session();
@@ -656,6 +721,30 @@ fn trim_marks_around_selection() {
     assert!(close(b.x0, vb.x0 - 30.0) && close(b.y1, vb.y1 + 30.0), "{b:?} {vb:?}");
 }
 
+/// Create Trim Marks refuses an offset, length, bleed or weight given as text or as another value
+/// that is not a number, around the selection and around the artboards, and makes no marks.
+#[test]
+fn trim_marks_refuse_a_number_given_as_text() {
+    let mut s = session();
+    let a = rect(&mut s, 100.0, 100.0, 200.0, 100.0);
+    let calls = [
+        ("offset", json!({"offset": "10"})),
+        ("length", json!({"length": "20 pt"})),
+        ("bleed", json!({"style": "japanese", "bleed": "3"})),
+        ("weight", json!({"weight": true})),
+        ("offset", json!({"allArtboards": true, "offset": {"value": 10}})),
+    ];
+    for ids in [vec![a], vec![]] {
+        sel(&mut s, &ids);
+        let before = (undo_len(&s), s.doc().unwrap().doc.layers[0].children().unwrap().len());
+        for (key, p) in &calls {
+            refuses_non_number(&mut s, "object.createTrimMarks", p, key);
+            assert_eq!((undo_len(&s), s.doc().unwrap().doc.layers[0].children().unwrap().len()), before, "{p}");
+        }
+    }
+    s.execute("object.createTrimMarks", &json!({"offset": 10, "length": 20, "bleed": null})).unwrap();
+}
+
 // ---------- Convert to Shape ----------
 
 #[test]
@@ -791,6 +880,22 @@ fn rearrange_refuses_columns_that_are_not_a_whole_number() {
     assert_eq!((r["rows"].clone(), r["columns"].clone()), (json!(2), json!(1)));
 }
 
+/// Rearrange All Artboards refuses a spacing given as text or as another value that is not a number,
+/// and the artboards stay where they were.
+#[test]
+fn rearrange_refuses_a_spacing_given_as_text() {
+    let mut s = session();
+    s.execute("artboard.new", &json!({"x": 1000, "y": 0, "width": 400, "height": 300})).unwrap();
+    let boards = |s: &Session| s.doc().unwrap().doc.artboards.iter().map(|a| a.rect).collect::<Vec<_>>();
+    let before = (boards(&s), undo_len(&s));
+    for p in [json!({"spacing": "50"}), json!({"columns": 1, "spacing": "50 pt"}), json!({"layout": "row", "spacing": false})] {
+        refuses_non_number(&mut s, "artboard.rearrange", &p, "spacing");
+        assert_eq!((boards(&s), undo_len(&s)), before, "{p}");
+    }
+    s.execute("artboard.rearrange", &json!({"columns": 1, "spacing": 50})).unwrap();
+    assert_eq!(boards(&s)[1], Rect::new(0.0, 650.0, 400.0, 950.0));
+}
+
 /// Fit to Artwork Bounds fits each artboard it is given to the art on it, in one undo step: an
 /// object partly on an artboard counts for each artboard it overlaps, art flush with an edge from
 /// outside or on no artboard doesn't count, and an artboard with no art stays as it is.

docs/mcp.md

@@ -876,6 +876,14 @@ of the format they write (one object can carry several formats' options). A comm
 (`document.setup`, `text.areaOptions`) says so the same way. The app's own journal isn't checked: an action or
 journal recorded by an older version replays as it did, except for an effect `color` that the effect can't read.
 
+These Object menu commands refuse a string or another value that is not a number with
+`` `scaleH` must be a number, not "50" ``, naming the key, and change nothing: `scaleH`, `scaleV`, `moveH`, `moveV`
+and `rotate` in `object.transformEach`; `ppi` in `object.rasterize`; `width`, `height`, `spacingX` and `spacingY` in
+`object.createObjectMosaic`; `offset`, `length`, `bleed` and `weight` in `object.createTrimMarks`; and `spacing` in
+`artboard.rearrange`. A null is read as a key left out. `object.cropImage` refuses a `rect` that is not an array of
+four numbers, such as a string, with `rect must be [x, y, width, height]`; a null `rect` crops to the artboard, as a
+missing one does.
+
 `object.createObjectMosaic` (`columns`, `rows`), `object.transformEach` (`reference`, `seed`) and `artboard.rearrange`
 (`columns`) take whole numbers, written `4` or `4.0`. A fraction or a negative number is refused with
 `` `columns` must be a whole number ``. A string or another value that is not a number is refused with

docs/target-app-parity.md

@@ -680,6 +680,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
   Object › Lock › All Artwork Above and Object › Hide › All Artwork Above go through each top-level layer once, checking each object that is not selected against the selected objects below it: with 100 tiles selected under 25,000 rectangles, Lock took 6.4 s and now takes under 0.1 s, and with all of 5,001 objects selected it took 8.8 s and now takes under 0.1 s.
   All Artwork Above and Other Layers (Object › Lock and Object › Hide), Transform Each, Rearrange All Artboards, the width point and width profile commands, Liquify, Puppet Warp, Define Perspective Grid, Attach to Active Plane, Release with Perspective, moves and transforms in perspective, Move Plane and Move Plane to Match Object record no undo step when they change nothing.
   Create Object Mosaic (its columns and rows), Transform Each (its reference point and Random seed) and Rearrange All Artboards (its columns) refuse a fraction, a negative number or a string, such as 2.5, -1 or "4"; 4.0 counts as 4.
+  Transform Each (its scales, moves and rotation), Rasterize (its resolution), Create Object Mosaic (its new width and height and its spacing), Create Trim Marks (its offset, length, bleed and weight) and Rearrange All Artboards (its spacing) refuse a value other than a number or null, such as "50", and change nothing. Crop Image refuses a rectangle that is not four numbers, such as "0, 0, 20, 20".
   Change Case, Smart Punctuation, Find and Replace, Convert To Area Type, Convert To Point Type, Type on a Path Options, the Type on a Path effects and Fill with Placeholder Text record no undo step when they change nothing.
 - **Fonts:** a font is found by any of its names (family and style in every language of its name table, legacy family/style pairs, PostScript name), so documents from other apps open with the font they name; Find Font and `text.fonts` say for each font whether it was found exactly, by another name, substituted or missing, and how many glyphs are missing. PDF and `.ai` import brings Japanese text back as live type: CID fonts without a ToUnicode map read through the standard CMaps, vertical columns (WMode 1) as vertical type, letter spacing as tracking; a glyph is live type only when the installed font (in the closest style of its family) draws the same glyph, and text in a font that isn't installed comes in as live type shown in the fallback font until the font is installed (exports that draw it — PDF, EPS, EMF/WMF, raster, SVG outlines or embedded fonts — warn that they wrote the fallback font, and Print and Print Preview warn about the type that prints in it; `textAs: outlines` keeps the PDF's embedded glyphs). Two installed versions of one family and style (a font library keeping an old file beside a new one) both load, the first staying the one the family and style resolve to; PDF and `.ai` import tries each version when the glyphs differ, and type set in a version other than that one names it (`CharStyle::font_version`), which layout, the substituted-glyph highlight, Find Font, Package and Document Info follow. Fonts outside the font folders are listed too: those font services such as Adobe Fonts load on Windows, through DirectWrite, and those apps and font managers register with CoreText on macOS (#579). When a document opens with fonts that aren't available, fonts used only in symbols included, a Missing Fonts dialog lists them after the linked-file questions, with Find Fonts (Type › Find Font on them) and Close. Its Find in Folder… (in Find Font too) searches a folder on several threads, reads only what the font scan reads of font files and skips app packages, the Program Files and ProgramData folders, the system's folders and the app data folders in a home folder; Add Fonts copies the font files the user chooses, suitcase fonts with their resource fork, into VectorCraft's own Fonts folder, which the desktop app and `vectorcraft-cli` read (`text.missingFonts`, `text.findFontFiles`, `text.addFontFiles`). The font scan on Linux also reads the folders fontconfig's configuration lists, such as TeX fonts under `/usr/share/texmf/fonts` (#1548, fixes #1529; not verified on Linux).
   A variable font's type gets one slider and field per variation axis under the Character panel's Variable Font button, each named as the font names the axis (Weight, Width), so the type can be set between or past its named instances: the settings stay with the characters (`CharStyle::font_axes`, `text.setFormat {fontAxes}` and `text.setRangeStyle {fontAxes}`, clamped to each axis's range; `text.fontList {family, style}` lists the axes), and layout, outlines, the substituted-glyph highlight and PDF and SVG export (`font-variation-settings`, embedded fonts) use the font instanced at them. Another font or style clears them; legacy `.ai` export writes the style and warns that they are left out.