Merge pull request #1968 from VoiceLessQ/fix/1522-anchored-copy-ids

Give anchored objects fresh ids when copying text frames (fixes #1522)

ROADMAP.md

@@ -17,6 +17,10 @@ DesignCraft aims at full Adobe InDesign parity — and to be better: faster, ope
   the next column or frame when needed (#1901). Cell-local footnotes contribute to row height
   and sit inside the cell rather than outside its measuring frame. Vertical justification keeps
   paragraphs and table fragments above the footnote area. No readiness percentage change.
+- **2026-10-28** · Duplicate, paste, parent overrides and data merge give anchored objects in the
+  copied story fresh ids (#1522). Copies used to keep the original's ids, and the saved file then
+  failed to open with "duplicate item id". Files already saved that way still don't open; no
+  readiness percentage change.
 - **2026-10-25** · The application bar uses craft-window's shared title-bar code. A drag that
   starts on a floating panel over the bar moves the panel, not the window, and in full screen the
   bar no longer zooms or moves the window. No readiness percentage change.

crates/engine/src/cmd/datamerge/fill.rs

@@ -71,11 +71,17 @@ pub fn copy_item(
 }
 
 fn renumber(dst: &mut designcraft_doc::Document, src: &designcraft_doc::Document, it: &mut Item, maps: &mut Maps) {
+    renumber_in(dst, src, it, maps, true);
+}
+
+/// `adopt` is false inside a story copy, whose asset references the copy has already adopted.
+fn renumber_in(dst: &mut designcraft_doc::Document, src: &designcraft_doc::Document, it: &mut Item, maps: &mut Maps, adopt: bool) {
     let old = it.id;
     let new_id = ItemId(dst.alloc());
     maps.items.insert(old, new_id);
     it.id = new_id;
-    if let Some(m) = it.media.as_mut()
+    if adopt
+        && let Some(m) = it.media.as_mut()
         && let Some(p) = m.poster
     {
         m.poster = dst.adopt_asset(src, p).or(Some(p));
@@ -98,8 +104,10 @@ fn renumber(dst: &mut designcraft_doc::Document, src: &designcraft_doc::Document
                 }
                 designcraft_doc::remap_story_assets(&mut st, &assets);
                 st.frames.clear();
-                dst.stories.insert(n, Arc::new(st));
+                // Mapped first: the map is the seen-set that stops story recursion.
                 maps.stories.insert(tf.story, n);
+                renumber_anchored(dst, src, &mut st, maps);
+                dst.stories.insert(n, Arc::new(st));
                 n
             };
             tf.story = new_sid;
@@ -108,19 +116,32 @@ fn renumber(dst: &mut designcraft_doc::Document, src: &designcraft_doc::Document
             }
         }
         Content::Graphic(g) => {
-            if let Some(a) = dst.adopt_asset(src, g.asset) {
+            if adopt && let Some(a) = dst.adopt_asset(src, g.asset) {
                 g.asset = a;
             }
         }
         Content::Group { items } => {
             for child in items.iter_mut() {
-                renumber(dst, src, Arc::make_mut(child), maps);
+                renumber_in(dst, src, Arc::make_mut(child), maps, adopt);
             }
         }
         Content::Unassigned => {}
     }
 }
 
+/// Fresh ids for the objects anchored in story copy `st`, its table cells, footnotes and endnotes (#1522).
+fn renumber_anchored(dst: &mut designcraft_doc::Document, src: &designcraft_doc::Document, st: &mut Story, maps: &mut Maps) {
+    let mut fresh = |t: &mut Story| {
+        for o in &mut t.objects {
+            renumber_in(dst, src, &mut Arc::make_mut(o).item, maps, false);
+        }
+    };
+    st.for_each_text_mut(&mut fresh);
+    for n in &mut st.endnotes {
+        Arc::make_mut(n).text.for_each_text_mut(&mut fresh);
+    }
+}
+
 struct Rep {
     story: StoryId,
     start: usize,

crates/engine/src/cmd/datamerge/tests.rs

@@ -1269,3 +1269,19 @@ fn facing_pages_merge_tiles_short_blocks_without_stacking() {
     let unique: std::collections::BTreeSet<_> = origins.iter().collect();
     assert_eq!(unique.len(), 3, "no two records may share a page and coordinates: {origins:?}");
 }
+
+// #1522: each record's copy of an anchored object needs its own id, or the merged file won't reopen.
+#[test]
+fn merged_records_give_anchored_objects_their_own_ids() {
+    let mut s = Session::new();
+    new_doc(&mut s, 1);
+    select_csv(&mut s, "Name\nAda\nGrace\n");
+    let made = text_frame(&mut s, [36.0, 36.0, 400.0, 80.0], "<<Name>> here", None);
+    let b = exec(&mut s, "frame.create", json!({"rect": [450, 450, 474, 490]}));
+    exec(&mut s, "anchored.insert", json!({"ids": [b["id"]], "story": made["story"], "pos": 2}));
+    assert_eq!(exec(&mut s, "data.merge", json!({}))["records"], 2);
+    let doc = &s.documents()[s.active_index().unwrap()].doc;
+    assert_eq!(doc.anchored_items().len(), 2);
+    doc.check().unwrap();
+    designcraft_format::load(&designcraft_format::save(doc).unwrap()).unwrap();
+}

crates/engine/src/cmd/interchange.rs

@@ -74,7 +74,7 @@ fn with_pdf_layer_variants(doc: &Document, embed: bool) -> Result<Option<Documen
     }
     let mut out = doc.clone();
     let mut variants = std::collections::HashMap::new();
-    // Duplicated anchors can share an id. Reuse a variant only when source and choice agree.
+    // Ids should be unique (#1522); placements that still share one reuse a variant only when source and choice agree.
     let mut expected = std::collections::HashMap::new();
     for (id, asset, hidden, overridden) in choices {
         let signature = (asset.id, hidden.clone(), overridden);
@@ -832,8 +832,8 @@ mod nested_layer_tests {
         session.execute("file.exportIdml", &json!({"embedImages": false})).unwrap();
     }
 
-    // Duplicating a text frame that holds an anchored placed PDF leaves two placements sharing
-    // one ItemId (renumber gap, tracked separately). Export must still apply the choice to both.
+    // Duplicating a text frame that holds an anchored placed PDF copies the placement under a
+    // fresh id (#1522). Export must apply the choice to both.
     #[test]
     fn duplicated_frame_keeps_its_anchored_pdf_choice_in_idml() {
         let pdf = designcraft_render::pdf_hide_layers(&super::super::pdflayers::tests::two_layer_pdf(), &["Red".into()]).unwrap();
@@ -855,8 +855,9 @@ mod nested_layer_tests {
         s.execute("transform.move", &json!({"ids": [frame.0], "dx": 10, "dy": 10, "copy": true})).unwrap();
         let doc = s.doc().unwrap().doc.clone();
         let with_choice: Vec<_> = doc.placed_items().into_iter().filter(|i| i.has_pdf_layer_override()).map(|i| i.id).collect();
-        assert_eq!(with_choice, [chosen, chosen], "both placements share one id");
-        assert!(doc.check().unwrap_err().to_string().contains(&format!("duplicate item id {chosen}")));
+        assert_eq!(with_choice.len(), 2);
+        assert!(with_choice.contains(&chosen) && with_choice[0] != with_choice[1], "{with_choice:?}");
+        doc.check().unwrap();
         let result = s.execute("file.exportIdml", &json!({})).unwrap();
         let imported = designcraft_idml::import_idml(&base64_decode(result["base64"].as_str().unwrap())).unwrap();
         let mut layers: Vec<_> =
@@ -865,8 +866,8 @@ mod nested_layer_tests {
         assert_eq!(layers, vec![states(false, true), states(true, false)]);
     }
 
-    // Shared anchored ids are a pre-existing duplication gap. Conflicting choices must not
-    // silently select one variant; this fixture changes only the copied anchor's choice.
+    // Duplication no longer shares anchored ids (#1522), so this fixture forces the shared id.
+    // Conflicting choices must not silently select one variant; only the copy's choice changes.
     #[test]
     fn shared_id_with_divergent_pdf_choices_is_rejected() {
         let mut s = Session::new();
@@ -874,23 +875,25 @@ mod nested_layer_tests {
         let pdf = designcraft_render::pdf_hide_layers(&super::super::pdflayers::tests::two_layer_pdf(), &["Red".into()]).unwrap();
         let placed = s.execute("file.place", &json!({"base64": base64_encode(&pdf), "name": "layers.pdf"})).unwrap();
         let control = ItemId(placed["id"].as_u64().unwrap());
-        let frame = {
+        let (frame, anchored) = {
             let doc = Arc::make_mut(&mut s.doc_mut().unwrap().doc);
             let mut item = doc.item(control).unwrap().clone();
             item.id = ItemId(doc.alloc());
             item.pdf_hidden_layers = vec!["Blue".into()];
+            let anchored = item.id;
             let layer = doc.default_layer();
             let (frame, sid) = doc.add_text_frame(SpreadRef::Doc(0), Rect::new(0.0, 0.0, 200.0, 200.0), layer, "Body", Default::default()).unwrap();
             doc.story_mut(sid).unwrap().insert_object(0, AnchoredObject { item, position: Default::default() });
-            frame
+            (frame, anchored)
         };
         s.execute("transform.move", &json!({"ids": [frame.0], "dx": 10, "dy": 10, "copy": true})).unwrap();
-        // Change ONLY the copy in the newest story to explicit all-on.
+        // Give ONLY the copy in the newest story the original's id and explicit all-on.
         {
             let doc = Arc::make_mut(&mut s.doc_mut().unwrap().doc);
             let (_, st) = doc.stories.iter_mut().filter(|(_, st)| !st.objects.is_empty()).last().unwrap();
             let o = &mut Arc::make_mut(st).objects[0];
             let it = &mut Arc::make_mut(o).item;
+            it.id = anchored;
             it.pdf_hidden_layers.clear();
             it.pdf_layer_override = true;
         }
@@ -905,8 +908,8 @@ mod nested_layer_tests {
         assert!(Arc::ptr_eq(&before, &s.doc().unwrap().doc));
     }
 
-    // Cross-document paste: the pasted frame's anchored placement keeps its source id,
-    // which can collide with an unrelated placement in the destination.
+    // Cross-document paste used to keep the anchored placement's source id (#1522), colliding
+    // with an unrelated placement in the destination. The fixture forces that collision.
     #[test]
     fn shared_id_with_different_pdf_assets_is_rejected() {
         let mut s = Session::new();
@@ -927,7 +930,13 @@ mod nested_layer_tests {
             .as_u64()
             .unwrap();
         s.execute("edit.paste", &json!({})).unwrap();
-        assert_eq!(a, b, "fresh documents allocate the same first placement id");
+        {
+            let doc = Arc::make_mut(&mut s.doc_mut().unwrap().doc);
+            let (_, st) = doc.stories.iter_mut().find(|(_, st)| !st.objects.is_empty()).unwrap();
+            let pasted = &mut Arc::make_mut(&mut Arc::make_mut(st).objects[0]).item;
+            assert_ne!(pasted.id.0, b, "paste gives the anchored placement a fresh id");
+            pasted.id = ItemId(b);
+        }
         let before = s.doc().unwrap().doc.clone();
         let graphics: Vec<_> = before.placed_items().into_iter().filter(|i| i.graphic().is_some()).collect();
         assert_eq!(graphics.len(), 2);

crates/engine/src/cmd/object.rs

@@ -1105,9 +1105,15 @@ fn adopt_or_dangle(dst: &mut Document, src: &Document, id: AssetId) -> AssetId {
 
 /// Give `it` (an item of `src`) fresh ids from `dst`, with copies of its stories and the assets it uses.
 pub(crate) fn renumber(dst: &mut Document, src: &Document, it: &mut Item, stories: &mut HashMap<StoryId, StoryId>) {
+    renumber_in(dst, src, it, stories, true);
+}
+
+/// `adopt` is false inside a story copy, whose asset references the copy has already adopted.
+fn renumber_in(dst: &mut Document, src: &Document, it: &mut Item, stories: &mut HashMap<StoryId, StoryId>, adopt: bool) {
     it.id = ItemId(dst.alloc());
     let my_id = it.id;
-    if let Some(m) = it.media.as_mut()
+    if adopt
+        && let Some(m) = it.media.as_mut()
         && let Some(p) = m.poster
     {
         m.poster = Some(adopt_or_dangle(dst, src, p));
@@ -1129,8 +1135,10 @@ pub(crate) fn renumber(dst: &mut Document, src: &Document, it: &mut Item, storie
                     }
                     designcraft_doc::remap_story_assets(&mut st, &assets);
                     st.frames.clear();
-                    dst.stories.insert(n, Arc::new(st));
+                    // Mapped first: the map is the seen-set that stops story recursion.
                     stories.insert(tf.story, n);
+                    renumber_anchored(dst, src, &mut st, stories);
+                    dst.stories.insert(n, Arc::new(st));
                     n
                 }
             };
@@ -1140,17 +1148,32 @@ pub(crate) fn renumber(dst: &mut Document, src: &Document, it: &mut Item, storie
             }
         }
         Content::Graphic(g) => {
-            g.asset = adopt_or_dangle(dst, src, g.asset);
+            if adopt {
+                g.asset = adopt_or_dangle(dst, src, g.asset);
+            }
         }
         Content::Group { items } => {
             for c in items.iter_mut() {
-                renumber(dst, src, Arc::make_mut(c), stories);
+                renumber_in(dst, src, Arc::make_mut(c), stories, adopt);
             }
         }
         Content::Unassigned => {}
     }
 }
 
+/// Fresh ids for the objects anchored in story copy `st`, its table cells, footnotes and endnotes (#1522).
+fn renumber_anchored(dst: &mut Document, src: &Document, st: &mut Story, stories: &mut HashMap<StoryId, StoryId>) {
+    let mut fresh = |t: &mut Story| {
+        for o in &mut t.objects {
+            renumber_in(dst, src, &mut Arc::make_mut(o).item, stories, false);
+        }
+    };
+    st.for_each_text_mut(&mut fresh);
+    for n in &mut st.endnotes {
+        Arc::make_mut(n).text.for_each_text_mut(&mut fresh);
+    }
+}
+
 fn transform_move(s: &mut Session, p: &Value) -> Result<Value> {
     let ids = targets(s, p)?;
     let (dx, dy) = (f64_or(p, "dx", 0.0), f64_or(p, "dy", 0.0));
@@ -5937,3 +5960,75 @@ mod frame_baseline_grid_tests {
         assert_eq!(grid(&s), None);
     }
 }
+
+#[cfg(test)]
+mod anchored_copy_tests {
+    use super::*;
+    use designcraft_doc::AnchoredObject;
+
+    /// A text frame with an inline rectangle; returns the frame and the anchored item's id.
+    fn frame_with_inline_box(s: &mut Session) -> (ItemId, ItemId) {
+        s.execute("file.new", &json!({})).unwrap();
+        let r = s.execute("frame.create", &json!({"rect": [72, 72, 400, 400], "content": "text", "text": "Before after."})).unwrap();
+        let frame = ItemId(r["id"].as_u64().unwrap());
+        let sid = s.doc().unwrap().doc.item(frame).unwrap().text_frame().unwrap().story;
+        let b = s.execute("frame.create", &json!({"rect": [450, 450, 474, 490]})).unwrap();
+        s.execute("anchored.insert", &json!({"ids": [b["id"]], "story": sid.0, "pos": 7})).unwrap();
+        (frame, s.doc().unwrap().doc.story(sid).unwrap().objects[0].item.id)
+    }
+
+    fn assert_reopens(doc: &Document) {
+        doc.check().unwrap();
+        designcraft_format::load(&designcraft_format::save(doc).unwrap()).unwrap();
+    }
+
+    // #1522: the copy kept the anchored object's id, and the saved file no longer opened.
+    #[test]
+    fn duplicating_a_frame_with_an_anchored_object_saves_a_file_that_reopens() {
+        let mut s = Session::new();
+        let (frame, anchored) = frame_with_inline_box(&mut s);
+        s.execute("transform.move", &json!({"ids": [frame.0], "dx": 10, "dy": 10, "copy": true})).unwrap();
+        let doc = &s.doc().unwrap().doc;
+        assert_eq!(doc.anchored_items().iter().filter(|(_, i)| i.id == anchored).count(), 1);
+        assert_eq!(doc.anchored_items().len(), 2);
+        assert_reopens(doc);
+    }
+
+    #[test]
+    fn pasting_a_frame_with_an_anchored_object_twice_keeps_ids_unique() {
+        let mut s = Session::new();
+        let (frame, _) = frame_with_inline_box(&mut s);
+        s.execute("selection.set", &json!({"ids": [frame.0]})).unwrap();
+        s.execute("edit.copy", &json!({})).unwrap();
+        s.execute("edit.paste", &json!({})).unwrap();
+        s.execute("edit.paste", &json!({})).unwrap();
+        let doc = &s.doc().unwrap().doc;
+        assert_eq!(doc.anchored_items().len(), 3);
+        assert_reopens(doc);
+    }
+
+    // IDML import can anchor a text frame; its story is copied too, with the copy in its thread.
+    #[test]
+    fn an_anchored_text_frame_gets_its_own_story_copy() {
+        let mut d = Document::new(&designcraft_doc::build::NewDocument::default());
+        let layer = d.default_layer();
+        let (host, host_sid) = d.add_text_frame(SpreadRef::Doc(0), Rect::new(0.0, 0.0, 300.0, 300.0), layer, "Host", ParaFormat::default()).unwrap();
+        let (inner, inner_sid) = d.add_text_frame(SpreadRef::Doc(0), Rect::new(0.0, 0.0, 50.0, 50.0), layer, "Inner", ParaFormat::default()).unwrap();
+        let item = d.item(inner).unwrap().clone();
+        let inner_story = d.stories.get(&inner_sid).unwrap().clone();
+        d.remove_item(inner).unwrap();
+        d.stories.insert(inner_sid, inner_story);
+        d.story_mut(host_sid).unwrap().insert_object(0, AnchoredObject { item, position: Default::default() });
+        d.check().unwrap();
+        let src = d.clone();
+        let copy = duplicate_from(&mut d, &src, &[host], SpreadRef::Doc(0), Vec2::new(10.0, 10.0)).unwrap();
+        let copy_sid = d.item(copy[0]).unwrap().text_frame().unwrap().story;
+        let anchored = &d.story(copy_sid).unwrap().objects[0].item;
+        assert_ne!(anchored.id, inner);
+        let anchored_sid = anchored.text_frame().unwrap().story;
+        assert_ne!(anchored_sid, inner_sid);
+        assert_eq!(d.story(anchored_sid).unwrap().frames, [anchored.id]);
+        assert_eq!(d.story(inner_sid).unwrap().frames, [inner]);
+        assert_reopens(&d);
+    }
+}