Commands: an id that is not an object id is refused instead of acting on the selection

targets() read `id` as an object only when it was a number. Any other
value ("abc", "5", true, {}, [1]) fell through to the selection, so a
command such as css.export, whose `id` the shared check does not read,
acted on the selected objects instead of failing. targets() now reads
`ids` and `id` through named_ids(): a value that is not a non-negative
integer is an error, and an omitted or null `id` still means the
selection. A numeric string is refused, as it already is in `ids`.

effect.apply and appearance.addEffect accept the effect as `id` when
`effect` is left out; they now read their targets with that `id` set to
null, so the effect still applies to the selection as before.
This commit is contained in:
dotCooCoo
2026-11-08 13:17:30 +07:00
committed by bflatastic
parent 31dd69ff4e
commit 864e606505
5 changed files with 58 additions and 15 deletions
+5 -14
View File
@@ -575,21 +575,12 @@ pub(crate) fn named_id(p: &Value) -> Result<Option<NodeId>> {
}
}
/// 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<Vec<NodeId>> {
// 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()),
}
}
+10
View File
@@ -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<Value> {
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"));
+39
View File
@@ -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();
+3 -1
View File
@@ -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"}`,
+1
View File
@@ -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.