Merge pull request #6905 from dotCooCoo/fix/type-size-range-error

Type: text.create and the type style commands refuse a size outside 0.1 to 1296 pt

crates/engine/src/cmd/addons/color/legend.rs

@@ -137,7 +137,7 @@ fn legend(s: &mut Session, p: &Value) -> Result<Value> {
     let parent = st.target_parent()?;
     let text_size = (size / 7.0).clamp(4.0, 12.0);
     let pad = (size / 12.0).clamp(1.0, 8.0);
-    let style = super::super::super::create::new_type_style(s, &json!({ "size": text_size }));
+    let style = super::super::super::create::new_type_style(s, &json!({ "size": text_size }), C)?;
     let count = swatches.len();
     // Each chip's fill links to its swatch, as paint.setFill {swatch} links art: editing a global or
     // spot swatch then recolours the legend too.

crates/engine/src/cmd/addons/typetools.rs

@@ -139,7 +139,8 @@ fn text_tweaker(s: &mut Session, p: &Value) -> Result<Value> {
         let mut run = 0;
         for (i, at) in chars.into_iter().enumerate() {
             let random = if shape == "random" { unit(&mut rng) } else { 0.0 };
-            let value = from + (to - from) * amount(shape, i, n, random);
+            // Rounding can carry a value a hair past the range's ends.
+            let value = (from + (to - from) * amount(shape, i, n, random)).clamp(lo, hi);
             let mut change = CharChange::parse(&json!({ key: value }), C, &setup)?;
             change.localize(&t, C)?;
             while ends.get(run).is_some_and(|&e| e <= at) {

crates/engine/src/cmd/clipboard/flavours.rs

@@ -491,7 +491,7 @@ fn import_text(s: &mut Session, p: &Value) -> Result<Value> {
     if text.trim().is_empty() {
         return Err(bad(C, "the text is empty"));
     }
-    let style = super::super::create::new_type_style(s, &Value::Null);
+    let style = super::super::create::new_type_style(s, &Value::Null, C)?;
     let system_layout = s.doc()?.doc.setup.system_layout;
     let layout = |t: &mut TextObject| {
         t.system_layout = system_layout;

crates/engine/src/cmd/create.rs

@@ -109,7 +109,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Create Text",
             [],
             None,
-            "{x, y, text, rotation?: deg clockwise about x, y (0: level), vertical?: bool = false, size?: pt, font?: family, style? (default Regular, or the family's closest style when it has no Regular), color? (#rrggbbaa and #rgba are refused), keepModel?: bool (as paint.setFill), area?: {width, height, fit?: none|autoHeight|shrinkText|block, fitMinPercent?, wordGap?, lineGap? (as text.areaOptions)}, placeholder?: bool (fill it with placeholder text instead, as the Type tools do with prefs placeholderText), justify?: \"auto\" (default: the start of each paragraph's direction)|\"left\"|\"center\"|\"right\"|\"justifyAll\" (point type: aligned on x, so \"center\" centres each line on the point and \"right\" ends it there; text.setStyle changes it later), leadingModel?: \"romanBaseline\"|\"emBoxTop\", charAlign?: \"romanBaseline\"|\"emBoxTop\"|\"emBoxCenter\"|\"emBoxBottom\"|\"icfTop\"|\"icfBottom\" (default: emBoxTop and emBoxCenter while the interface is in Japanese, else romanBaseline)} → {id, bounds: {x, y, width, height} (the new type's box as document.inspect reports it, after justify), overflow?: [id] (area type whose text doesn't fit its frame: the lines past it aren't drawn; left out when it fits), warning?: text (the font or style given isn't available: the type is drawn in the fallback font or the family's closest style)}; new area type gets Auto Size (fit autoHeight) when the autoSizeAreaType preference is on",
+            "{x, y, text, rotation?: deg clockwise about x, y (0: level), vertical?: bool = false, size?: pt 0.1..1296 (outside it: an error, and no type is made), font?: family, style? (default Regular, or the family's closest style when it has no Regular), color? (#rrggbbaa and #rgba are refused), keepModel?: bool (as paint.setFill), area?: {width, height, fit?: none|autoHeight|shrinkText|block, fitMinPercent?, wordGap?, lineGap? (as text.areaOptions)}, placeholder?: bool (fill it with placeholder text instead, as the Type tools do with prefs placeholderText), justify?: \"auto\" (default: the start of each paragraph's direction)|\"left\"|\"center\"|\"right\"|\"justifyAll\" (point type: aligned on x, so \"center\" centres each line on the point and \"right\" ends it there; text.setStyle changes it later), leadingModel?: \"romanBaseline\"|\"emBoxTop\", charAlign?: \"romanBaseline\"|\"emBoxTop\"|\"emBoxCenter\"|\"emBoxBottom\"|\"icfTop\"|\"icfBottom\" (default: emBoxTop and emBoxCenter while the interface is in Japanese, else romanBaseline)} → {id, bounds: {x, y, width, height} (the new type's box as document.inspect reports it, after justify), overflow?: [id] (area type whose text doesn't fit its frame: the lines past it aren't drawn; left out when it fits), warning?: text (the font or style given isn't available: the type is drawn in the fallback font or the family's closest style)}; new area type gets Auto Size (fit autoHeight) when the autoSizeAreaType preference is on",
             has_doc,
             text_create
         ),
@@ -739,7 +739,7 @@ fn text_create(s: &mut Session, p: &Value) -> Result<Value> {
         check_numbers(a, &["width", "height"], "text.create")?;
     }
     let text = str_param(p, "text").unwrap_or("");
-    let mut t = TextObject::point(Point::new(x, y), text, new_type_style(s, p));
+    let mut t = TextObject::point(Point::new(x, y), text, new_type_style(s, p, "text.create")?);
     // Turned about the point (the first baseline's start, an area's top-left corner): type set
     // along a Constrain Angle.
     let rotation = if p.get("rotation").is_some() { f64_req(p, "rotation", "text.create")? } else { 0.0 };
@@ -848,11 +848,11 @@ pub(crate) fn new_area_fit(s: &Session, p: &Value, c: &str, t: &mut TextObject)
 /// The character style new type gets: `size`, `font`, `style` and `color` from `p` (an RGB
 /// `color` in a CMYK document as [`super::paint::new_color`] stores it), else the defaults,
 /// filled with the current fill unless that is None or white (black then), in the document's
-/// language.
-pub(crate) fn new_type_style(s: &Session, p: &Value) -> CharStyle {
+/// language. A `size` outside [`super::typecmd::TYPE_SIZES`] is an error.
+pub(crate) fn new_type_style(s: &Session, p: &Value, cmd: &str) -> Result<CharStyle> {
     let mut style = CharStyle::default();
     if let Some(sz) = p.get("size").and_then(Value::as_f64) {
-        style.size = sz.clamp(0.1, 1296.0);
+        style.size = super::typecmd::given_type_size(sz, cmd)?;
     }
     if let Some(f) = str_param(p, "font") {
         style.font_family = f.to_string();
@@ -873,5 +873,5 @@ pub(crate) fn new_type_style(s: &Session, p: &Value) -> CharStyle {
     let chosen = |name: &str| vectorcraft_doc::setup::char_language(name).ok().flatten();
     let document = s.doc().ok().and_then(|d| chosen(&d.doc.setup.language));
     style.language = document.or_else(|| chosen(&s.prefs.hyphenation_language)).map(str::to_string);
-    style
+    Ok(style)
 }

crates/engine/src/cmd/typecmd.rs

@@ -138,7 +138,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Character",
             [],
             None,
-            "{ids?|id?, font?, style? (font alone: the style is kept when the new family has it, else its closest), size?: pt, leading?: pt|\"auto\", tracking?: 1/1000 em, justify?: \"auto\" (the start of each paragraph's direction)|\"left\"|\"center\"|\"right\"|\"justifyAll\", fill?: colour|\"none\", stroke?: colour|\"none\" (characters without a stroke weight get 1 pt; #rrggbbaa and #rgba are refused in both), keepModel?: bool (fill and stroke, as paint.setFill), strokeWidth?: pt (the character stroke's weight, in document points as stroke.set), features?: [\"dlig\", \"-liga\", \"salt=2\", …] OpenType (swsh, ss01–ss20, salt, titl, pnum, lnum, sups, subs, numr, dnom and aalt take the font's alternate number as `tag=N`, N 0–65535: \"salt=2\" draws the second alternate of each glyph that has one, \"salt\" is \"salt=1\", \"-salt\" and \"salt=0\" are off), featureChanges?: [\"tnum\", \"-onum\", \"salt=2\", …] (OpenType features turned on, or with - off, on top of each character's own, a value replacing the feature's last one; its other features are kept), start?: byte, end?: byte} (size/leading are document points after object scaling; with a range: the character attributes style that range and justify the paragraphs it touches; without: all the text) → {ids, overflow?: [id…] (area type and type on a path whose text no longer fits; left out when all fits), warning?: text (the font or style set isn't available: its type is drawn in the fallback font or the family's closest style)}",
+            "{ids?|id?, font?, style? (font alone: the style is kept when the new family has it, else its closest), size?: pt 0.1..1296 (outside it: an error), leading?: pt|\"auto\", tracking?: 1/1000 em, justify?: \"auto\" (the start of each paragraph's direction)|\"left\"|\"center\"|\"right\"|\"justifyAll\", fill?: colour|\"none\", stroke?: colour|\"none\" (characters without a stroke weight get 1 pt; #rrggbbaa and #rgba are refused in both), keepModel?: bool (fill and stroke, as paint.setFill), strokeWidth?: pt (the character stroke's weight, in document points as stroke.set), features?: [\"dlig\", \"-liga\", \"salt=2\", …] OpenType (swsh, ss01–ss20, salt, titl, pnum, lnum, sups, subs, numr, dnom and aalt take the font's alternate number as `tag=N`, N 0–65535: \"salt=2\" draws the second alternate of each glyph that has one, \"salt\" is \"salt=1\", \"-salt\" and \"salt=0\" are off), featureChanges?: [\"tnum\", \"-onum\", \"salt=2\", …] (OpenType features turned on, or with - off, on top of each character's own, a value replacing the feature's last one; its other features are kept), start?: byte, end?: byte} (size/leading are document points after object scaling; with a range: the character attributes style that range and justify the paragraphs it touches; without: all the text) → {ids, overflow?: [id…] (area type and type on a path whose text no longer fits; left out when all fits), warning?: text (the font or style set isn't available: its type is drawn in the fallback font or the family's closest style)}",
             has_doc,
             set_style
         ),
@@ -715,6 +715,20 @@ pub(crate) fn text_targets(s: &Session, p: &Value, cmd: &str) -> Result<Vec<Node
     Ok(t)
 }
 
+/// The type sizes a command takes, in points.
+pub(crate) const TYPE_SIZES: (f64, f64) = (0.1, 1296.0);
+
+/// A type size `size` that a command was given: an error naming the range when it is outside
+/// [`TYPE_SIZES`].
+pub(crate) fn given_type_size(size: f64, cmd: &str) -> Result<f64> {
+    let (lo, hi) = TYPE_SIZES;
+    if !(lo..=hi).contains(&size) {
+        let key = "size";
+        return Err(bad(cmd, format!("`{key}` must be from {lo} to {hi}")));
+    }
+    Ok(size)
+}
+
 /// Clamp in the units shown to the user, then compensate the text's affine. Clamping
 /// the local result would make small sizes impossible after enlarging an object.
 pub(crate) fn local_type_value(t: &TextObject, value: f64, limits: (f64, f64), horizontal: bool, cmd: &str) -> Result<f64> {
@@ -820,10 +834,7 @@ fn set_style(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "text.setStyle";
     let font = text_param(p, "font", C)?.map(str::to_string);
     let style = text_param(p, "style", C)?.map(str::to_string);
-    let size = length_given(p, "size", C)?;
-    if size.is_some_and(|v| v <= 0.0) {
-        return Err(bad(C, "size must be positive"));
-    }
+    let size = length_given(p, "size", C)?.map(|v| given_type_size(v, C)).transpose()?;
     let leading = match p.get("leading") {
         None | Some(Value::Null) => None,
         Some(Value::String(a)) if a.eq_ignore_ascii_case("auto") => Some(None),

crates/engine/src/tests/textedit.rs

@@ -87,6 +87,50 @@ fn set_range_style_splits_runs() {
     assert!(s.execute("text.setRangeStyle", &json!({"id": id.0, "size": -1})).is_err());
 }
 
+/// `text.create`, `text.createInPath`, `text.setStyle` and `text.setRangeStyle` refuse a size
+/// outside 0.1 to 1296 pt with an error naming the range, and change nothing; both ends are taken.
+#[test]
+fn a_type_size_outside_its_range_is_refused() {
+    let mut s = session();
+    let message = "`size` must be from 0.1 to 1296";
+    let state = |s: &Session| (s.doc().unwrap().doc.node_count(), s.doc().unwrap().history.undo.len());
+    let before = state(&s);
+    for size in [2835.0, 1296.5, 0.05, 0.0, -1.0] {
+        let e = s.execute("text.create", &json!({"x": 10, "y": 20, "text": "Big", "size": size})).unwrap_err().to_string();
+        assert!(e.contains(message), "{size}: {e}");
+    }
+    assert_eq!(state(&s), before, "no type is made");
+    let big = s.execute("text.create", &json!({"x": 10, "y": 20, "text": "Big", "size": 1296})).unwrap();
+    let big = NodeId(big["id"].as_u64().unwrap());
+    assert_eq!(obj(&s, big).runs[0].style.size, 1296.0);
+    let small = s.execute("text.create", &json!({"x": 10, "y": 20, "text": "small", "size": 0.1})).unwrap();
+    assert_eq!(obj(&s, NodeId(small["id"].as_u64().unwrap())).runs[0].style.size, 0.1);
+    let path = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 200, "height": 100})).unwrap()["id"].clone();
+    let before = state(&s);
+    let e = s.execute("text.createInPath", &json!({"path": path, "text": "In", "size": 2835})).unwrap_err().to_string();
+    assert!(e.contains(message), "{e}");
+    assert_eq!(state(&s), before);
+    s.execute("text.createInPath", &json!({"path": path, "text": "In", "size": 1296})).unwrap();
+    let id = text(&mut s, "Hello");
+    let before = state(&s);
+    for (cmd, params) in [
+        ("text.setStyle", json!({"ids": [id.0], "size": 2835})),
+        ("text.setStyle", json!({"ids": [id.0], "size": "1300 pt"})),
+        ("text.setStyle", json!({"ids": [id.0], "size": 0.05})),
+        ("text.setRangeStyle", json!({"id": id.0, "start": 0, "end": 2, "size": 2835})),
+        ("text.setRangeStyle", json!({"id": id.0, "size": 0})),
+    ] {
+        let e = s.execute(cmd, &params).unwrap_err().to_string();
+        assert!(e.contains(message), "{cmd} {params}: {e}");
+    }
+    assert_eq!(state(&s), before, "nothing changes");
+    assert_eq!(obj(&s, id).runs[0].style.size, 20.0);
+    s.execute("text.setStyle", &json!({"ids": [id.0], "size": 1296})).unwrap();
+    assert_eq!(obj(&s, id).runs[0].style.size, 1296.0);
+    s.execute("text.setRangeStyle", &json!({"id": id.0, "start": 0, "end": 1, "size": 0.1})).unwrap();
+    assert_eq!(obj(&s, id).runs[0].style.size, 0.1);
+}
+
 /// `featureChanges` turns features on and off on top of each run's own and keeps the rest, in
 /// `text.setStyle` and `text.setRangeStyle`, in one undo step; an unknown tag is an error.
 #[test]

docs/mcp.md

@@ -1631,7 +1631,9 @@ wrong. A value that is not a number gets `` `dx` must be a number, not "10" ``:
 `` `size` must be a number, not true ``, and a `tracking` that is neither a number nor a number written as a string
 (`"20"`) with `` `tracking` must be a number, not false ``. It and `text.setRangeStyle` refuse a `font` or `style` that is not a string
 with `` `font` must be a string ``, and a `features` list with an entry that is not a string with
-`features must be a list of tags`; `text.setStyle` refuses such a `justify` too. `type.ruby` refuses a `text`,
+`features must be a list of tags`; `text.setStyle` refuses such a `justify` too. `text.create`, `text.createInPath`,
+`text.setStyle` and `text.setRangeStyle` refuse a `size` outside 0.1 to 1,296 pt with
+`` `size` must be from 0.1 to 1296 `` and change nothing. `type.ruby` refuses a `text`,
 `position` or `align` that is not a string, and `text.setPathHandle` refuses a `which` other than `in` or `out` with
 `` `which` must be one of in, out ``. Each leaves the document, the selection and the undo history as they were, also
 when the call gives another param it could apply. A null is read as a key left out.