From c212c89bab697768c9839292cb3d2e098f474dd3 Mon Sep 17 00:00:00 2001 From: Hamza Date: Thu, 22 Oct 2026 17:00:05 +0500 Subject: [PATCH] A text anchor's name is the document's, so it is not reused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `anchor.create` took any name, including one another anchor already had, so a document could hold two anchors called "Photo 1". Every reference to the name then means whichever the search finds first: `xref.insert {"anchor": "Photo 1"}` and `hyperlink.create {"anchor": "Photo 1"}` take the first, and an export writes both destinations under the one name. A name nobody can resolve is not a name. A name already in the document is a usage error now, naming it and where to look: "a text anchor named `Photo 1` is already in this document (anchor.list names them)". A name that is empty or only spaces is refused too — a cross-reference built from `` would show nothing. Case still distinguishes names, as it does elsewhere in the document, and nothing about existing anchors changes: the check is on creation. The test covers the same name in the same story and in another one, the empty names, that only one anchor exists afterwards, and that another name and a name differing in case are both fine. Without the check the duplicate is created again. Not from a ticket: found probing `anchor.create`. --- crates/engine/src/cmd/xref.rs | 48 ++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/xref.rs b/crates/engine/src/cmd/xref.rs index d462f3d1..44614809 100644 --- a/crates/engine/src/cmd/xref.rs +++ b/crates/engine/src/cmd/xref.rs @@ -22,7 +22,7 @@ pub fn specs() -> Vec { "New Hyperlink Destination…", ["Type", "Hyperlinks & Cross-References"], None, - "{name} — a text anchor at the insertion point", + "{name} — a text anchor at the insertion point; the name is the document's, so it must not be one another anchor already has", in_story_text, create_anchor ), @@ -103,6 +103,14 @@ fn insert(s: &mut Session, p: &Value) -> Result { fn create_anchor(s: &mut Session, p: &Value) -> Result { let name = str_param(p, "name").ok_or_else(|| bad("anchor.create", "missing name"))?.to_string(); + if name.trim().is_empty() { + return Err(bad("anchor.create", "a text anchor needs a name (a cross-reference and a hyperlink find it by that name)")); + } + // Two anchors of the same name made every reference to it ambiguous: `xref.insert` and + // `hyperlink.create` take the first they find, and a file writes both under one name. + if s.doc()?.doc.stories.values().any(|st| st.anchors.iter().any(|a| a.name == name)) { + return Err(bad("anchor.create", format!("a text anchor named `{name}` is already in this document (anchor.list names them)"))); + } s.edit(|d, sel| { let t = sel.text.ok_or_else(|| bad("anchor.create", "no insertion point"))?; let st = d.story_mut(t.story).ok_or(designcraft_doc::DocError::NoStory(t.story))?; @@ -215,6 +223,44 @@ mod tests { assert_eq!(s.execute("xref.list", &json!({})).unwrap()[0]["text"], "(p. 3)"); } + /// Two text anchors of the same name made every reference to it ambiguous: `xref.insert + /// {"anchor": "A"}` and `hyperlink.create {"anchor": "A"}` take the first they find, and an + /// exported file writes both destinations under one name. + #[test] + fn a_text_anchor_name_is_the_documents_so_it_is_not_reused() { + let mut s = Session::new(); + s.execute("file.new", &json!({"pages": 2, "facingPages": false})).unwrap(); + let a = s.execute("frame.create", &json!({"rect": [72, 72, 400, 200], "content": "text", "text": "Photo one here"})).unwrap(); + let select = |s: &mut Session, story: serde_json::Value, from: usize| { + s.execute("text.select", &json!({"story": story, "anchor": from, "focus": from})).unwrap(); + }; + select(&mut s, a["story"].clone(), 0); + s.execute("anchor.create", &json!({"name": "Photo 1"})).unwrap(); + + // The same name again, in the same story and in another one. + select(&mut s, a["story"].clone(), 10); + let e = s.execute("anchor.create", &json!({"name": "Photo 1"})).expect_err("refused").to_string(); + assert!(e.contains("already in this document"), "{e}"); + let sp = s.doc().unwrap().doc.page_loc(1).unwrap().0; + let b = s.execute("frame.create", &json!({"spread": sp, "rect": [72, 72, 400, 200], "content": "text", "text": "Photo two"})).unwrap(); + select(&mut s, b["story"].clone(), 0); + assert!(s.execute("anchor.create", &json!({"name": "Photo 1"})).is_err(), "not in another story either"); + // A name of spaces is not a name. + assert!(s.execute("anchor.create", &json!({"name": " "})).is_err()); + assert!(s.execute("anchor.create", &json!({"name": ""})).is_err()); + + // One anchor, so a reference by name can only mean that one. + let anchors = s.execute("anchor.list", &json!({})).unwrap(); + assert_eq!(anchors.as_array().unwrap().len(), 1, "{anchors}"); + // Another name is fine. + s.execute("anchor.create", &json!({"name": "Photo 2"})).unwrap(); + assert_eq!(s.execute("anchor.list", &json!({})).unwrap().as_array().unwrap().len(), 2); + // The check is on the name, not on where it is: a name that differs only in case is a + // different name, as it is everywhere else in the document. + select(&mut s, b["story"].clone(), 0); + s.execute("anchor.create", &json!({"name": "photo 1"})).expect("a different name"); + } + #[test] fn cross_reference_into_a_story_with_a_running_header_composes() { let mut s = Session::new();