Symbols: Break Link keeps new art on a locked artboard selected

Break Link to Symbol on an instance just placed on a locked artboard
selected nothing: the art that replaced the instance has a new id, so the
locked-artboard check did not see it as new art. The art of an instance
that was new art is now checked without the locked artboards, and the
session keeps it selected as new art. The art of an older instance on a
locked artboard, named by id, is still left out of the selection.

crates/engine/src/cmd/brushsym/symbols.rs

@@ -391,9 +391,10 @@ fn expanded_bound(d: &Document, inst: &Node, own: bool) -> u64 {
 }
 
 /// Replace instances `ids` with their art ([`expanded_instance`], `own` as there). A hidden or
-/// locked instance's art stays hidden or locked. Returns the art's ids. Fails before any edit when
-/// the instances together would make more than [`MAX_EXPAND_NODES`] objects.
-fn break_links(d: &mut Document, ids: &[NodeId], own: bool) -> Result<Vec<NodeId>> {
+/// locked instance's art stays hidden or locked. Returns each replaced instance's id with its art's
+/// id. Fails before any edit when the instances together would make more than
+/// [`MAX_EXPAND_NODES`] objects.
+fn break_links(d: &mut Document, ids: &[NodeId], own: bool) -> Result<Vec<(NodeId, NodeId)>> {
     let total = ids.iter().filter_map(|id| d.node(*id)).fold(0u64, |sum, n| sum.saturating_add(expanded_bound(d, n, own)));
     if total > MAX_EXPAND_NODES as u64 {
         return Err(EngineError::Other("Too many pieces to make at once".into()));
@@ -408,7 +409,7 @@ fn break_links(d: &mut Document, ids: &[NodeId], own: bool) -> Result<Vec<NodeId
         let aid = art.id;
         d.remove(*id)?;
         d.insert(par, idx, art)?;
-        out.push(aid);
+        out.push((*id, aid));
     }
     Ok(out)
 }
@@ -495,12 +496,20 @@ pub(super) fn symbol_break_link(s: &mut Session, p: &Value) -> Result<Value> {
     }
     let iso = s.doc()?.isolation;
     let out = s.edit("Break Link to Symbol", |d, sel| {
-        let out = break_links(d, &ids, true)?;
+        let fresh = d.new_art.get();
+        let made = break_links(d, &ids, true)?;
         // Art that is hidden or locked, or in a hidden or locked group or layer, is left out of the
-        // selection.
+        // selection, as is art on a locked artboard unless the instance it replaces was new art.
         let locks = d.locked_artboards();
-        sel.set(out.iter().copied().filter(|id| crate::cmd::select::is_selectable(d, iso, &locks, *id)));
-        Ok(out)
+        sel.set(made.iter().filter_map(|(inst, art)| {
+            let keep = if fresh.contains(inst) {
+                crate::cmd::select::is_selectable_on_any_artboard(d, iso, *art)
+            } else {
+                crate::cmd::select::is_selectable(d, iso, &locks, *art)
+            };
+            keep.then_some(*art)
+        }));
+        Ok(made.into_iter().map(|(_, art)| art).collect::<Vec<_>>())
     })?;
     Ok(json!({ "ids": ids_json(&out) }))
 }

crates/engine/src/cmd/select.rs

@@ -252,6 +252,18 @@ pub(crate) fn selectable_ids(d: &Document, iso: Option<NodeId>) -> BTreeSet<Node
 /// Whether [`walk_selectable`] visits `id`, read from the objects that hold it instead of a walk of
 /// the document. `locks` is the document's [`Document::locked_artboards`].
 pub(crate) fn is_selectable(d: &Document, iso: Option<NodeId>, locks: &LockedArtboards, id: NodeId) -> bool {
+    is_selectable_within(d, iso, Some(locks), id)
+}
+
+/// [`is_selectable`] without the locked artboards, for an edit's new selection: after the edit,
+/// the session takes the art on a locked artboard out of the selection, except the new art the
+/// edit made.
+pub(crate) fn is_selectable_on_any_artboard(d: &Document, iso: Option<NodeId>, id: NodeId) -> bool {
+    is_selectable_within(d, iso, None, id)
+}
+
+/// [`is_selectable`], leaving out the locked artboards when `locks` is `None`.
+fn is_selectable_within(d: &Document, iso: Option<NodeId>, locks: Option<&LockedArtboards>, id: NodeId) -> bool {
     let Some(a) = d.ancestry(id) else { return false };
     if !a.iter().all(|i| d.node(*i).is_some_and(|n| n.visible && !n.locked && !n.is_template())) {
         return false;
@@ -262,7 +274,7 @@ pub(crate) fn is_selectable(d: &Document, iso: Option<NodeId>, locks: &LockedArt
         None => {
             // The object Select All takes: the outermost one that is not a layer.
             let top = a.iter().filter_map(|i| d.node(*i)).find(|n| !n.is_layer());
-            top.is_some_and(|t| locks.is_empty() || !locks.holds(t))
+            top.is_some_and(|t| locks.is_none_or(|l| l.is_empty() || !l.holds(t)))
         }
     }
 }

crates/engine/src/tests/brushsym.rs

@@ -1386,6 +1386,31 @@ fn break_link_leaves_art_in_hidden_or_locked_groups_unselected() {
     assert!(!a.contains(&hidden) && !a.contains(&locked), "{a:?}");
 }
 
+/// Break Link to Symbol on an instance just placed on a locked artboard selects the art that
+/// replaces it, which stays new art there. The art of an older instance there, named by id, is not
+/// selected.
+#[test]
+fn break_link_keeps_new_art_on_a_locked_artboard_selected() {
+    let mut s = session();
+    let (old, name, _) = make_symbol(&mut s);
+    s.execute("artboard.new", &json!({"x": 1000, "y": 0, "width": 400, "height": 400})).unwrap();
+    s.execute("artboard.setProps", &json!({"index": 1, "locked": true})).unwrap();
+    select(&mut s, &[old]);
+    s.execute("object.move", &json!({"dx": 1000, "dy": 0})).unwrap();
+    assert!(s.doc().unwrap().selection.objects.is_empty(), "moved onto the locked artboard");
+    let inst = NodeId(s.execute("symbol.place", &json!({"name": name, "x": 1100, "y": 100})).unwrap()["id"].as_u64().unwrap());
+    assert_eq!(s.doc().unwrap().selection.objects.to_vec(), [inst], "placed art is selected");
+    let r = s.execute("symbol.breakLink", &json!({})).unwrap();
+    let art: Vec<NodeId> = r["ids"].as_array().unwrap().iter().map(|v| NodeId(v.as_u64().unwrap())).collect();
+    assert_eq!(art.len(), 1, "{r}");
+    assert_eq!(s.doc().unwrap().selection.objects.to_vec(), art, "the art that replaces it");
+    s.execute("object.move", &json!({"dx": 10, "dy": 0})).unwrap();
+    assert_eq!(s.doc().unwrap().selection.objects.to_vec(), art, "the art stays editable");
+    let r = s.execute("symbol.breakLink", &json!({"ids": [old.0]})).unwrap();
+    assert_eq!(r["ids"].as_array().unwrap().len(), 1, "{r}");
+    assert!(s.doc().unwrap().selection.objects.is_empty(), "the older instance's art");
+}
+
 /// Delete Symbol, expanding the instances, keeps a hidden instance's art hidden and a locked
 /// instance's art locked.
 #[test]