diff --git a/ROADMAP.md b/ROADMAP.md index d9b9b92b5..a13912f7c 100644 --- a/ROADMAP.md +++ b/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. diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index 53fd36fd5..825179ac9 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/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, diff --git a/crates/engine/src/cmd/datamerge/tests.rs b/crates/engine/src/cmd/datamerge/tests.rs index 212175d78..e2c76a90f 100644 --- a/crates/engine/src/cmd/datamerge/tests.rs +++ b/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], "<> 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(); +} diff --git a/crates/engine/src/cmd/interchange.rs b/crates/engine/src/cmd/interchange.rs index c38a5db96..335773ee5 100644 --- a/crates/engine/src/cmd/interchange.rs +++ b/crates/engine/src/cmd/interchange.rs @@ -74,7 +74,7 @@ fn with_pdf_layer_variants(doc: &Document, embed: bool) -> Result = 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); diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index aac0d0eb1..e834a96d7 100644 --- a/crates/engine/src/cmd/object.rs +++ b/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) { + 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, 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) { + 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 { 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); + } +}