Merge pull request #2671 from dotCooCoo/fix/isolate-refuses-locked-art

Isolation mode: object.isolate refuses hidden or locked objects and locked guide paths

crates/engine/src/cmd/object.rs

@@ -227,7 +227,15 @@ pub fn specs() -> Vec<CommandSpec> {
             has_selection,
             |s, _| clip_edit(s, true)
         ),
-        cmd!("object.isolate", "Enter Isolation Mode", [], None, "{id?} (default: the selection's first object)", has_doc, isolate),
+        cmd!(
+            "object.isolate",
+            "Enter Isolation Mode",
+            [],
+            None,
+            "{id?} isolate a group, a layer or a single object (default: the first selected object); type, a hidden or locked object, an object in a hidden or locked group or layer, and a guide path while Lock Guides is on are refused",
+            has_doc,
+            isolate
+        ),
         cmd!("object.exitIsolation", "Exit Isolation Mode", [], None, "{}", has_doc, exit_isolation),
         cmd!(
             "object.setProps",
@@ -1053,6 +1061,7 @@ fn isolate(s: &mut Session, p: &Value) -> Result<Value> {
     let id = id_param(p, "id")
         .or_else(|| s.active().and_then(|d| d.selection.objects.first().copied()))
         .ok_or_else(|| bad("object.isolate", "missing id"))?;
+    let guides_locked = s.menu.guides_locked;
     let st = s.doc_mut()?;
     // Groups, layers and single objects (a path, a compound path, an image…); type isn't
     // isolated: a double-click on it edits its text.
@@ -1060,6 +1069,15 @@ fn isolate(s: &mut Session, p: &Value) -> Result<Value> {
         return Err(bad("object.isolate", "type objects can't be isolated"));
     }
     let isolation = st.doc.node(id).filter(|n| n.extras().shaper.is_some()).and_then(|n| n.children()).and_then(|c| c.first()).map_or(id, |n| n.id);
+    // A hidden or locked object, one in a hidden or locked group or layer, and a guide path while
+    // Lock Guides is on aren't isolated either: a click with the Selection tool doesn't select them.
+    // A Shaper Group isolates its source group, so that group is the one checked.
+    if !st.doc.is_editable(isolation) {
+        return Err(bad("object.isolate", "the object is hidden or locked"));
+    }
+    if guides_locked && crate::is_guide_path(&st.doc, isolation) {
+        return Err(EngineError::Other("guides are locked".into()));
+    }
     st.isolation = Some(isolation);
     st.selection.clear();
     st.revision += 1;

crates/engine/src/tests.rs

@@ -2035,6 +2035,88 @@ fn isolating_an_object_that_doesnt_exist_says_so() {
     assert!(!e.to_string().contains("type"), "{e}");
 }
 
+/// `object.isolate` refuses a locked or hidden group, a group in a locked or hidden layer, a locked
+/// or hidden member of a group, and a guide path while Lock Guides is on. Isolation mode stays off,
+/// and a Selection tool drag over the art does what it does without the call. A group that is neither is isolated, and the drag moves its
+/// member.
+#[test]
+fn isolate_refuses_locked_or_hidden_art_and_locked_guides() {
+    const HIDDEN_OR_LOCKED: &str = "invalid parameters for `object.isolate`: the object is hidden or locked";
+    let x0 = |s: &Session, id: NodeId| s.doc().unwrap().doc.node(id).unwrap().geometric_bounds().unwrap().x0;
+    // A drag 30 pt to the right from the left edge of `a`.
+    let drag = |s: &mut Session| {
+        let v = ViewInfo::default();
+        s.select_tool("selection", v).unwrap();
+        s.pointer(&PointerEvent::new(PointerKind::Down, 101.0, 115.0), v).unwrap();
+        for x in [111.0, 121.0, 131.0] {
+            s.pointer(&PointerEvent::new(PointerKind::Drag, x, 115.0), v).unwrap();
+        }
+        s.pointer(&PointerEvent::new(PointerKind::Up, 131.0, 115.0), v).unwrap();
+    };
+    // A group of `a` and another rectangle, on the only layer.
+    let group = |s: &mut Session| {
+        let a = rect(s, 100.0, 100.0, 30.0, 30.0);
+        let b = rect(s, 200.0, 100.0, 30.0, 30.0);
+        s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap();
+        (NodeId(s.execute("object.group", &json!({})).unwrap()["id"].as_u64().unwrap()), a)
+    };
+    // Each case locks, hides or guides something, given the layer, the group and `a`, and returns
+    // the id to isolate.
+    type Case = (&'static str, fn(&mut Session, NodeId, NodeId, NodeId) -> NodeId, &'static str);
+    let cases: [Case; 7] = [
+        ("locked group", |s, _, g, _| set_then(s, "object.setProps", g, "locked", true, g), HIDDEN_OR_LOCKED),
+        ("hidden group", |s, _, g, _| set_then(s, "object.setProps", g, "visible", false, g), HIDDEN_OR_LOCKED),
+        ("locked layer", |s, l, g, _| set_then(s, "layer.setProps", l, "locked", true, g), HIDDEN_OR_LOCKED),
+        ("hidden layer", |s, l, g, _| set_then(s, "layer.setProps", l, "visible", false, g), HIDDEN_OR_LOCKED),
+        ("locked member", |s, _, _, a| set_then(s, "object.setProps", a, "locked", true, a), HIDDEN_OR_LOCKED),
+        ("hidden member", |s, _, _, a| set_then(s, "object.setProps", a, "visible", false, a), HIDDEN_OR_LOCKED),
+        (
+            "guide path",
+            |s, _, _, a| {
+                s.execute("select.set", &json!({"ids": [a.0]})).unwrap();
+                s.execute("view.guides.make", &json!({})).unwrap();
+                s.execute("view.guides.lock", &json!({"locked": true})).unwrap();
+                a
+            },
+            "guides are locked",
+        ),
+    ];
+    // Each case runs with and without `object.isolate` before the drag. Every case runs, and the
+    // wrong ones are listed: (case, what isolating gave, the isolated id, where the drag left `a`,
+    // where it leaves `a` with no isolating).
+    let mut wrong = vec![];
+    for (name, make, error) in cases {
+        let run = |isolate: bool| {
+            let mut s = session();
+            let (g, a) = group(&mut s);
+            let layer = s.doc().unwrap().doc.layers[0].id;
+            let target = make(&mut s, layer, g, a);
+            let r = isolate.then(|| s.execute("object.isolate", &json!({"id": target.0})).map_err(|e| e.to_string()));
+            let isolation = s.doc().unwrap().isolation;
+            drag(&mut s);
+            (r, isolation, x0(&s, a))
+        };
+        let ((r, isolation, x), (_, _, without)) = (run(true), run(false));
+        if (&r, isolation, x) != (&Some(Err(error.to_string())), None, without) {
+            wrong.push((name, r, isolation, x, without));
+        }
+    }
+    assert!(wrong.is_empty(), "{wrong:#?}");
+    // Neither locked nor hidden: isolated, and the drag moves the member.
+    let mut s = session();
+    let (g, a) = group(&mut s);
+    s.execute("object.isolate", &json!({"id": g.0})).unwrap();
+    assert_eq!(s.doc().unwrap().isolation, Some(g));
+    drag(&mut s);
+    assert_eq!(x0(&s, a), 130.0, "the member moved");
+}
+
+/// Sets `key` of `id` to `on` with `cmd`, and gives back `then`.
+fn set_then(s: &mut Session, cmd: &str, id: NodeId, key: &str, on: bool, then: NodeId) -> NodeId {
+    s.execute(cmd, &json!({"ids": [id.0], key: on})).unwrap();
+    then
+}
+
 /// A ring (a compound path with a hole) and its isolated paths.
 fn isolated_ring(s: &mut Session) -> (NodeId, NodeId, NodeId) {
     let outer = rect(s, 0.0, 0.0, 40.0, 40.0);

crates/engine/src/tests_menucmds.rs

@@ -2077,8 +2077,12 @@ fn guide_path(s: &mut Session, x: f64) -> NodeId {
 fn exit_isolation_leaves_a_locked_guide_path_deselected() {
     let mut s = session();
     let g = guide_path(&mut s, 0.0);
+    // A locked guide path can't be isolated; one isolated before Lock Guides went on stays isolated.
     s.execute("view.guides.lock", &json!({"locked": true})).unwrap();
+    assert!(s.execute("object.isolate", &json!({"id": g.0})).is_err());
+    s.execute("view.guides.lock", &json!({"locked": false})).unwrap();
     s.execute("object.isolate", &json!({"id": g.0})).unwrap();
+    s.execute("view.guides.lock", &json!({"locked": true})).unwrap();
     s.execute("object.exitIsolation", &json!({})).unwrap();
     assert!(selected(&s).is_empty());
     let _ = s.execute("object.move", &json!({"dx": 10, "dy": 0}));

crates/engine/src/tests_shaper.rs

@@ -311,6 +311,23 @@ fn shaper_construction_taps_stay_in_the_isolated_group() {
     assert_eq!(s.doc().unwrap().isolation, Some(sources));
 }
 
+/// `object.isolate` on a Shaper Group whose source group is locked or hidden is refused, and
+/// isolation mode stays off.
+#[test]
+fn shaper_isolate_refuses_a_locked_or_hidden_source_group() {
+    for (key, on) in [("locked", true), ("visible", false)] {
+        let mut s = session();
+        rectangle(&mut s, 0.0, "#ff0000");
+        rectangle(&mut s, 50.0, "#0000ff");
+        let g = scribble(&mut s, &[[25.0, 20.0], [75.0, 35.0], [30.0, 50.0], [125.0, 65.0]]);
+        let sources = node(&s, g).children().unwrap()[0].id;
+        s.execute("object.setProps", &json!({"ids": [sources.0], key: on})).unwrap();
+        let r = s.execute("object.isolate", &json!({"id": g.0})).map_err(|e| e.to_string());
+        assert_eq!(r, Err("invalid parameters for `object.isolate`: the object is hidden or locked".into()), "{key}");
+        assert_eq!(s.doc().unwrap().isolation, None, "{key}");
+    }
+}
+
 #[test]
 fn shaper_lines_keep_free_angles_with_or_without_shift() {
     for shift in [false, true] {

docs/mcp.md

@@ -1757,7 +1757,9 @@ and `layerRows`, the rows highlighted in the panel.
   `layer.outlineOthers` / `layer.previewAll` (Ctrl/Cmd+Alt-clicking an eye runs Outline Others for the row's layer, or
   Preview All Layers when the others are in Outline already), `layer.lockOthers` / `layer.unlockAll` (Alt-clicking an eye or a lock
   runs Hide or Lock Others for the row's layer, or Show or Unlock All when the others already are), `layer.pasteRemembersLayers`,
-  `object.isolate {id}` (a group, a layer or a single object; not type) / `object.exitIsolation`.
+  `object.isolate {id?}` (a group, a layer or a single object, by default the first selected object; not type, not a
+  hidden or locked object or one in a hidden or locked group or layer, and not a guide path while Lock Guides is on) /
+  `object.exitIsolation`.
 - **Dialogs and panel state** (control channel and the app's MCP): `ui.layerOptions {ids?}` and `ui.newLayer
   {sublayer?}` open Layer Options (dialog `layerOptions`: name, color, template, locked, visible, printable, preview,
   dimImages, dimPercent; OK is one undo step; Cmd/Ctrl+Alt+L runs `ui.newLayer` for a new layer), `ui.layersPanelOptions` opens Panel Options (`layersPanelOptions`: