From be7ebc078a87eedc8fdc2cb431dadb248062e6b6 Mon Sep 17 00:00:00 2001 From: SBaksa <18648812+SBaksa@users.noreply.github.com> Date: Sun, 8 Nov 2026 17:36:30 +1100 Subject: [PATCH] Lasso: refuse a history name that shows nothing or hides formatting A historyName of zero-width, filler or BOM characters showed an empty History row, and a bidi control or line separator could make a row read differently from its name. A history name now has no control or line-break character anywhere, loses white space and ZWNJ/ZWJ at its ends, and is then held to the plug-in menu-label rule, now shared as photocraft_plugins::manifest::printable_name (1-64 characters with a visible one, no hidden format characters; plug-in names also refuse U+2028/U+2029). --- .../retouch_cmds/patch_history_names_tests.rs | 33 ++++++++++++++++++- crates/engine/src/cmd/selection_cmds.rs | 20 +++++++++-- crates/plugins/src/manifest.rs | 12 +++++-- 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/cmd/retouch_cmds/patch_history_names_tests.rs b/crates/engine/src/cmd/retouch_cmds/patch_history_names_tests.rs index 6172d5c376..d72958fc3a 100644 --- a/crates/engine/src/cmd/retouch_cmds/patch_history_names_tests.rs +++ b/crates/engine/src/cmd/retouch_cmds/patch_history_names_tests.rs @@ -51,10 +51,41 @@ fn the_lasso_is_named_for_the_tool_that_drew_it() { fn a_hostile_history_name_is_refused_without_a_step() { let mut s = session(); let steps = s.active().unwrap().history.past_len(); - for name in [json!(""), json!(" "), json!("x".repeat(65)), json!("a\nb"), json!(7), json!(["Patch Tool"])] { + let invisible = ["\u{200B}", " \u{200B}\u{FEFF} ", "\u{3164}", "\u{FE0F}", "\u{E0020}", "\u{00AD}"]; + let hidden = ["Patch\u{202E}looT", "\u{FEFF}Patch Tool", "Patch Tool\u{200B}", "\u{2066}Patch", "Patch\u{2028}Tool", "a\u{2029}b"]; + let ends = ["Patch Tool\n", "\r\nPatch Tool", "\tPatch", "\u{0085}Patch", "Patch\u{2028}"]; + let others = [json!(""), json!(" "), json!("x".repeat(65)), json!("a\nb"), json!(7), json!(["Patch Tool"])]; + let others = ends.iter().map(|n| json!(n)).chain(others); + for name in invisible.iter().chain(&hidden).map(|n| json!(n)).chain(others) { assert!(s.execute("select.lasso", json!({"points": TRIANGLE, "historyName": name})).is_err(), "{name}"); let polygonal = json!({"points": TRIANGLE, "polygonal": true, "historyName": name}); assert!(s.execute("select.lasso", polygonal).is_err(), "polygonal {name}"); } assert_eq!(s.active().unwrap().history.past_len(), steps); } + +#[test] +fn a_name_joining_its_letters_with_a_zero_width_joiner_is_kept() { + let mut s = session(); + for name in ["\u{0645}\u{200C}\u{06CC}", "\u{1F469}\u{200D}\u{1F4BB}", "Patch Tool \u{2713}"] { + s.execute("select.lasso", json!({"points": TRIANGLE, "historyName": name})).unwrap(); + assert_eq!(last_step(&s).as_deref(), Some(name)); + } +} + +#[test] +fn spaces_and_joiners_at_the_ends_are_trimmed_and_only_what_is_left_counts() { + let mut s = session(); + let padded = format!("{}{}", " ".repeat(10), "x".repeat(60)); + let joined = format!("Patch Tool{}", "\u{200D}".repeat(60)); + let cases = [ + ("Patch Tool\u{200D}", "Patch Tool"), + (" \u{200C}Patch Tool ", "Patch Tool"), + ("Love \u{2764}\u{FE0F}", "Love \u{2764}\u{FE0F}"), + (joined.as_str(), "Patch Tool"), + ]; + for (name, stored) in cases.iter().copied().chain([(padded.as_str(), &padded[10..])]) { + s.execute("select.lasso", json!({"points": TRIANGLE, "historyName": name})).unwrap(); + assert_eq!(last_step(&s).as_deref(), Some(stored), "{name:?}"); + } +} diff --git a/crates/engine/src/cmd/selection_cmds.rs b/crates/engine/src/cmd/selection_cmds.rs index 90a1c14d88..ed4b60cc96 100644 --- a/crates/engine/src/cmd/selection_cmds.rs +++ b/crates/engine/src/cmd/selection_cmds.rs @@ -627,13 +627,27 @@ fn lasso(s: &mut Session, p: &Value) -> Result { // A bad name is refused even when `polygonal` picks the step's name. let named = match p.get("historyName") { None | Some(Value::Null) => None, - Some(Value::String(n)) if !n.trim().is_empty() && n.chars().count() <= 64 && !n.chars().any(char::is_control) => Some(n.trim()), - Some(_) => return Err(EngineError::BadParams { cmd: "select.lasso".into(), msg: "historyName must be a short text".into() }), + Some(Value::String(n)) if history_name(n).is_some() => history_name(n), + Some(_) => { + let msg = "historyName must be 1-64 characters with a visible one, and no control, line-break or hidden format characters"; + return Err(EngineError::BadParams { cmd: "select.lasso".into(), msg: msg.into() }); + } }; let step = if b(p, "polygonal", false) { "Polygonal Lasso" } else { named.unwrap_or("Lasso") }; polygon_selection(s, "select.lasso", step, &pts, p) } +/// The History step name for `historyName` `n`: refused when it has a control or line-break +/// character anywhere; otherwise without white space or zero-width (non-)joiners at its ends, so a +/// name that looks like "Patch Tool" is "Patch Tool", and then held to the plug-in menu-label rule. +fn history_name(n: &str) -> Option<&str> { + if n.chars().any(|c| c.is_control() || matches!(c, '\u{2028}' | '\u{2029}')) { + return None; + } + let t = n.trim_matches(|c: char| c.is_whitespace() || matches!(c, '\u{200C}' | '\u{200D}')); + photocraft_plugins::manifest::printable_name(t).then_some(t) +} + /// Selects the polygon `pts` with the lasso tools' `mode`, `antiAlias` and `feather` params, as /// one history step called `label`. pub(crate) fn polygon_selection(s: &mut Session, cmd: &str, label: &str, pts: &[(f32, f32)], p: &Value) -> Result { @@ -764,7 +778,7 @@ pub fn specs() -> Vec { "select.lasso", "Lasso", [], - r##"{"points":[[x,y],…],"mode":"replace|add|subtract|intersect"="replace","antiAlias":bool=true,"feather":px=0,"polygonal":bool=false,"historyName"?:text (1..64 characters, no control characters; the History step name when a tool other than the Lasso draws it, e.g. "Patch Tool"; default "Lasso")}"##, + r##"{"points":[[x,y],…],"mode":"replace|add|subtract|intersect"="replace","antiAlias":bool=true,"feather":px=0,"polygonal":bool=false,"historyName"?:text (1..64 characters with a visible one, no control, line-break or hidden format characters, spaces and joiners at the ends trimmed; the History step name when a tool other than the Lasso draws it, e.g. "Patch Tool"; default "Lasso")}"##, has_doc, lasso ), diff --git a/crates/plugins/src/manifest.rs b/crates/plugins/src/manifest.rs index 133c0fa843..f8116198fe 100644 --- a/crates/plugins/src/manifest.rs +++ b/crates/plugins/src/manifest.rs @@ -143,9 +143,7 @@ impl Manifest { if !valid_id(&self.id) { return Err(Error::Manifest(format!("invalid id {:?}: use 1-64 characters from A-Z a-z 0-9 . _ -", self.id))); } - let name = self.name.trim(); - let printable = name.chars().all(|c| !c.is_control() && !hidden_format(c)) && name.chars().any(visible); - if !printable || name.chars().count() > 64 { + if !printable_name(self.name.trim()) { return Err(Error::Manifest("`name` must be 1-64 printable characters".into())); } if self.version.len() > 32 || self.description.len() > 1024 || self.author.len() > 128 { @@ -399,6 +397,12 @@ fn hidden_format(c: char) -> bool { !matches!(c, '\u{200C}' | '\u{200D}') && in_ranges(c, FORMAT) } +/// Whether `name` (already trimmed) can be shown as a label: 1-64 characters, at least one of them +/// visible, and no control characters, line or paragraph separators, or hidden format characters. +pub fn printable_name(name: &str) -> bool { + name.chars().count() <= 64 && name.chars().any(visible) && !name.chars().any(|c| c.is_control() || matches!(c, '\u{2028}' | '\u{2029}') || hidden_format(c)) +} + /// A character that draws something on its own. fn visible(c: char) -> bool { !c.is_whitespace() && !c.is_control() && !in_ranges(c, FORMAT) && !in_ranges(c, BLANK) @@ -631,6 +635,8 @@ mod tests { "\u{3164}\u{FE0F}", " \u{3000} ", "Tone\u{7}", + "Tone\u{2028}Pro", + "Tone\u{2029}Pro", ] { assert!(with_name(bad).is_err(), "{bad:?}"); }