diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 98dfe3aaa..e74db417b 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -575,21 +575,12 @@ pub(crate) fn named_id(p: &Value) -> Result> { } } -/// Objects a command targets: explicit `ids` param or the selection. +/// Objects a command targets: its `ids`, else its `id` as `ids` of one ([`named_ids`]), else the +/// selection. An `ids` or `id` that isn't an object id is an error, never the selection. pub(crate) fn targets(s: &Session, p: &Value) -> Result> { - // Objects given are used as given: a value that isn't an object id fails rather than the - // command acting on the selection instead (#785). - match p.get("ids").filter(|v| !v.is_null()) { - Some(Value::Array(a)) => return a.iter().map(object_id).collect(), - Some(v) => return Err(EngineError::Other(format!("`ids` must be an array of object ids, not {v}"))), - None => {} - } - match p.get("id").filter(|v| !v.is_null()) { - Some(v @ Value::Number(_)) => Ok(vec![object_id(v)?]), - // A step reference a batch couldn't resolve (other strings are other ids, such as an - // effect's). - Some(Value::String(r)) if r.starts_with('$') => Err(unresolved_reference(r)), - _ => Ok(s.doc()?.selection.objects.clone()), + match named_ids(p)? { + Some(ids) => Ok(ids), + None => Ok(s.doc()?.selection.objects.clone()), } } diff --git a/crates/engine/src/cmd/effectcmd.rs b/crates/engine/src/cmd/effectcmd.rs index e6568c5ea..21f43e0ee 100644 --- a/crates/engine/src/cmd/effectcmd.rs +++ b/crates/engine/src/cmd/effectcmd.rs @@ -222,6 +222,16 @@ pub(crate) const ONE_3D: &str = "An object takes one 3D effect. Edit the existin pub(crate) fn apply(s: &mut Session, p: &Value) -> Result { const C: &str = "effect.apply"; let id = str_param(p, "effect").or_else(|| str_param(p, "id")).ok_or_else(|| bad(C, "missing `effect` id"))?; + // An effect given as `id` names no objects: the targets are read with that `id` as null. + let effect_as_id; + let p = if str_param(p, "effect").is_none() { + let mut q = p.clone(); + q["id"] = Value::Null; + effect_as_id = q; + &effect_as_id + } else { + p + }; let mut params = p.get("params").cloned().unwrap_or(Value::Null); if !params.is_null() && !params.is_object() { return Err(bad(C, "`params` must be an object")); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e2fe63c17..bc8edc7e5 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3660,6 +3660,45 @@ fn malformed_ids_are_refused_by_commands_that_default_to_the_selection() { assert!(wrong.is_empty(), "{}", wrong.join("\n")); } +/// The objects a command reads from `ids`, `id` or the selection: an `id` that isn't an object id +/// (a numeric string included, as in `ids`) is an error, never the selection. An omitted or null +/// `id` means the selection. +#[test] +fn targets_refuse_an_id_that_is_not_an_object_id() { + let mut s = session(); + let a = rect(&mut s, 100.0, 100.0, 80.0, 60.0); + let b = rect(&mut s, 200.0, 100.0, 80.0, 60.0); + s.execute("select.set", &json!({"ids": [a.0]})).unwrap(); + for bad in [json!("abc"), json!("5"), json!(true), json!({}), json!([b.0]), json!(-1), json!(2.5)] { + let r = crate::cmd::targets(&s, &json!({ "id": bad })); + assert!(r.as_ref().is_err_and(|e| e.to_string().contains("object id")), "{bad}: {r:?}"); + } + assert_eq!(crate::cmd::targets(&s, &json!({})).unwrap(), vec![a]); + assert_eq!(crate::cmd::targets(&s, &json!({ "id": null })).unwrap(), vec![a]); + assert_eq!(crate::cmd::targets(&s, &json!({ "id": b.0 })).unwrap(), vec![b]); +} + +/// `css.export`, whose `id` the shared check doesn't read, refuses an `id` that isn't an object id +/// instead of exporting the selection. `effect.apply` with the effect given as `id` (its older key) +/// still applies it to the selection, as one undo step. +#[test] +fn css_export_refuses_a_bad_id_and_effect_apply_takes_an_effect_as_id() { + let mut s = session(); + let a = rect(&mut s, 100.0, 100.0, 80.0, 60.0); + s.execute("select.set", &json!({"ids": [a.0]})).unwrap(); + for bad in [json!("abc"), json!("5"), json!(true), json!({}), json!([a.0])] { + let r = s.execute("css.export", &json!({"id": bad, "unnamed": true})); + assert!(r.as_ref().is_err_and(|e| e.to_string().contains("object id")), "{bad}: {r:?}"); + } + for p in [json!({"unnamed": true}), json!({"id": null, "unnamed": true}), json!({"id": a.0, "unnamed": true})] { + assert_eq!(s.execute("css.export", &p).unwrap()["rules"], 1, "{p}"); + } + let undo = s.doc().unwrap().history.undo.len(); + let r = s.execute("effect.apply", &json!({"id": "stylize.dropShadow"})).unwrap(); + assert_eq!(r["ids"], json!([a.0])); + assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1); +} + #[test] fn malformed_anchor_selection_never_replaces_or_partially_changes_selection() { let mut s = session(); diff --git a/docs/mcp.md b/docs/mcp.md index 96c0f72dc..dbec2aa42 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -380,7 +380,9 @@ explicit `ids`, which stand in for the selection, so they work with nothing sele `ids?|id?` (or `id?, ids?: … (instead of id)`), `id` is `ids` of one object: `{"id": 7}` does what `{"ids": [7]}` does, with or without a selection, and an `id` or `ids` that is malformed (not a non-negative integer) or names no object is refused under the command's name, never replaced by the selection (`effect.apply {"id": "warp.arc"}` -without `effect` names the effect). A command that leaves out an object selected inside a selected group also +without `effect` names the effect). Other commands that act on `id` or else the selection, such as `css.export`, +refuse an `id` that is not a non-negative integer (`"abc"`, `"5"`, `true`) instead of using the selection; an omitted +or null `id` means the selection. A command that leaves out an object selected inside a selected group also leaves it out when `ids` list it together with a group that holds it. Paint values (`fill`, `stroke`) accept `"#rrggbb"`, `"none"`, `[r,g,b]` (0..1), `{"c","m","y","k"}`, `{"gray"}`, diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index dcd2f3a06..d98ef94cb 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -805,6 +805,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur Edit › Keyboard Shortcuts lists each of the Align panel's Align Objects, Distribute Objects and Distribute Spacing buttons under Align, with no default shortcut, as the reference app's [release notes](https://web.archive.org/web/20261006170350/https://helpx.adobe.com/illustrator/desktop/new-features/release-notes.html) describe for its Align and Distribute commands. A shortcut set there runs what a click on the button runs, with the Align To choice, the key object and the Distribute Spacing value (`align.horizontalLeft` and the other `align.*` UI commands). The command palette lists `select.key` as Cancel Key Object, and a pick cancels the key object (#541). Agent selection commands validate every object id before changing the selection and report the resulting count and ids (#784). + Commands that act on `id` or else the selection, such as `css.export`, refuse an `id` that is not an object id (`"abc"`, `"5"`, `true`) instead of acting on the selection; `effect.apply` with the effect given as `id` still applies it to the selection. Agents read the key object: `select.key` returns it, and `document.inspect` and the `ui.pointer` and `pointer_gesture` replies report it as `key`. `select.key` with an id that isn't a selected top-level object returns an error and leaves the key unchanged. The Select menu's Same commands, its Object commands that select art, Next Object Above and Below, saved selections and the Magic Wand (#2655) select only objects that Select All selects and objects inside them: nothing hidden or locked, nothing in a hidden or locked group, layer or sublayer, nothing on a template layer, and in isolation mode nothing outside the isolated group or layer. In isolation mode Select All, All on Active Artboard and Inverse select nothing while the isolated group or layer, or a group or layer it is in, is hidden, locked or a template layer. Select ▸ Object ▸ Bristle Brush Strokes selects the objects whose topmost stroke has a bristle brush, whatever the brush is named, and leaves out objects whose brush is of another type, also when its name has "Bristle" in it.