IDML export: a document's hyperlinks are written out (#1446) (#1659)

* IDML import: a page item's or a text run's hyperlink survives (#1446)

IDML carries a hyperlink as three elements: a source, a destination and a
`Hyperlink` joining them by `Self`. Only `CrossReferenceSource` was mapped, so
an image frame linked to a photo and a run of text linked to a page were
dropped on import, although both are native here and both export to PDF.

The names are collected as the package is read and the two ends put together
in `finish`, once the pages, page items and anchors they name exist. A link
whose source or destination is not in the file is dropped with a warning
saying which, rather than silently or as a link that goes nowhere.

Carries all three review fixes:

- a source recorded in a table cell's or a footnote's story is dropped. Those
  stories are built under the StoryId(0) placeholder, so the link named a
  story nothing can look up, with offsets into text that is not there. The
  warning says where the text was instead of claiming it was not text.
- the same for a HyperlinkTextDestination in such a story: its anchor id is
  (0 << 24) | k, so it collides with other cells' anchors and belongs to no
  story the cross-reference index holds; the link resolved to Anchor(1) and
  the PDF left the annotation out without a word.
- the index's topic cross-references go in at the start of the first story
  with text after the ranges are recorded, so the ranges for that story are
  shifted with the markers; a range left alone covered the wrong nine bytes.

Rebased onto the post-split main, hunk by hunk: the Importer struct, its
fields, the element scan, hyperlink_list and finish are in import.rs, while
StoryBuilder, walk_story and the anchor list are in import/story.rs. finish
answers (Document, Vec<String>) so the warnings reach the caller.

* An anchor a hyperlink cannot mean is refused, not ignored

This branch carries its own copy of `dest_param`'s anchor arm -- the IDML import
needed anchors as link destinations and added it here independently of #1555 --
and it has the same hole, so review is right that the fix has to be here as well
rather than only on that PR.

`dest_param` returns `Result<Option<_>>`. `None` still means no destination was
given, which `hyperlink.edit` needs for an edit that only renames; an `anchor`
that is given and names none is an error.

An unknown *id* was accepted outright, since `as_u64` answers for any number, so
a link was stored to an anchor nobody has and the export quietly left it out. On
`hyperlink.edit` the `None` read as "no destination given", so the command
reported success and kept the old destination, which is the case review
reported. An unknown *name* did already fail on `hyperlink.create`, but with
"give `url`, `email`, `page` or `anchor`" -- the wrong problem named.

The same change is on #1555, deliberately: each PR introduces this code on its
own line and each has to be right on its own. Whichever lands first, the other's
conflict is the same text on both sides.

* IDML export: a document's hyperlinks are written out (#1446)

Import reads hyperlinks now, but export dropped them, so a round trip lost
every link the document had. The three elements IDML wants are written: a
destination, a source, and a `Hyperlink` joining them.

A link on a run of text is a `HyperlinkTextSource` around the ranges it
covers, and the element nests inside a `ParagraphStyleRange`, so a link whose
text crosses a paragraph is written as one source per paragraph and each piece
stays clickable. Consecutive ranges of one link share a single source element.

Carries both review fixes:

- a link to an anchor whose text has been deleted is not written. The anchor
  is resolved first, as the page destination's page already was, so the file
  cannot hold a `Hyperlink` whose `Destination` names no element.
- every `Self` names one element. Three links on one frame wrote three
  `HyperlinkPageItemSource` elements all called i12, and each link to a page
  pushed another `HyperlinkPageDestination`; the item source carries the
  link's index now, and the page destination is written once per page as
  url_dest_el already did for URLs. The round-trip test collects every `Self`
  in the designmap and asserts none repeats, so an element added later is
  covered without anyone remembering to extend it.

Rebased onto the post-split main, on top of the rebased #1652: the `Ex` state
is in export.rs, the hyperlink elements are written in export/designmap.rs
with url_dest_el, fnv and has_self beside their only caller, and the run
cutting and wrapping is in export/story.rs.

* Split the hyperlink and index-marker steps out of Importer::run, under clippy's complexity limit

* Split the hyperlink and index-marker steps out of Importer::run, under clippy's complexity limit

* Split the hyperlink steps out of story_para, under clippy's complexity limit

* IDML import: say when a link's frame is anchored in text, which PDF export cannot place yet

---------

Co-authored-by: skylar taylor-barrick <hi@skylar.cc>
This commit is contained in:
Hamza Naveedandskylar taylor-barrick authored and GitHub committed 2026-11-06 10:01:02 +01:00
1 parent 6a85f42263
commit 8fce28258d
4 files changed
+328 -1

No files matched your search

+5
View File
@@ -99,6 +99,10 @@ struct Ex<'a> {
threads: HashMap<u64, (Option<u64>, Option<u64>)>,
/// Cross-reference sources written so far: (Self, target anchor).
xref_sources: Vec<(String, u64)>,
/// `HyperlinkTextSource`s written into the stories: (Self, index in `Document::hyperlinks`).
/// A link whose text crosses a paragraph gets one source per paragraph, since the element
/// nests inside a `ParagraphStyleRange` (#1446).
link_text_sources: Vec<(String, usize)>,
/// Index topic path → first serialized sort key; See / See also cross-references.
index_topics: BTreeMap<Vec<String>, String>,
index_xrefs: Vec<(Vec<String>, &'static str, String)>,
@@ -114,6 +118,7 @@ impl<'a> Ex<'a> {
fonts: BTreeMap::new(),
threads: HashMap::new(),
xref_sources: Vec::new(),
link_text_sources: Vec::new(),
index_topics: BTreeMap::new(),
index_xrefs: Vec::new(),
};
+100
View File
@@ -156,6 +156,78 @@ impl<'a> Ex<'a> {
for (name, _) in stories {
root.push(El::new("idPkg:Story").attr("src", name));
}
// The document's own hyperlinks (#1446): a destination element, a source element for a
// page item (a text source is already in its story) and a `Hyperlink` joining them.
// Stories are written before this, so `link_text_sources` is complete.
let text_sources: Vec<(String, usize)> = std::mem::take(&mut self.link_text_sources);
for (i, h) in d.hyperlinks.iter().enumerate() {
use designcraft_doc::{HyperlinkDest as De, HyperlinkSource as So};
// A text destination is already written where its anchor sits.
let dest = match &h.dest {
// Resolved first, as the page branch resolves its page: an anchor whose text was
// deleted names no element, so the file would hold a `Hyperlink` whose
// `Destination` points at nothing -- import only warns and InDesign may refuse
// the file (noted in review).
De::Anchor(a) => d.find_anchor(*a).map(|_| anchor_self(*a)),
De::Url(u) => Some(self.url_dest_el(&mut root, u, &h.name)),
De::Email(e) => Some(self.url_dest_el(&mut root, &format!("mailto:{e}"), &h.name)),
De::Page(p) => d.page(*p).map(|pg| {
// One element per page, as `url_dest_el` writes one per URL: every link to
// the same page pushed another `HyperlinkPageDestination` with the same
// `Self`, and IDML's references rely on a `Self` naming one element (noted
// in review).
let me = format!("HyperlinkPageDestination/p{}", pg.id.0);
if !has_self(&root, "HyperlinkPageDestination", &me) {
root.push(
El::new("HyperlinkPageDestination")
.attr("Self", &me)
.attr("Name", &h.name)
.attr("DestinationPage", uid(pg.id.0))
.attr("ViewSetting", "FitPage")
.attr("Hidden", "false"),
);
}
me
}),
};
let Some(dest) = dest else { continue };
// One `Hyperlink` per source element: a link whose text crosses a paragraph has one
// source per paragraph, so each piece stays clickable.
let sources: Vec<String> = match &h.source {
So::Item { id } if d.item(*id).is_some() => {
// The link's index as well as the item's: InDesign joins one source to one
// destination, so three links on one frame are three source elements, and
// naming them all `i{item}` gave three elements one `Self` (noted in review).
let me = format!("HyperlinkPageItemSource/i{}k{i}", id.0);
root.push(
El::new("HyperlinkPageItemSource")
.attr("Self", &me)
.attr("Name", &h.name)
.attr("SourcePageItem", uid(id.0))
.attr("Hidden", "false"),
);
vec![me]
}
So::Item { .. } => vec![],
So::Text { .. } => text_sources.iter().filter(|(_, k)| *k == i).map(|(me, _)| me.clone()).collect(),
};
for src in sources {
let mut el = El::new("Hyperlink")
.attr("Self", self.fresh())
.attr("Name", &h.name)
.attr("Source", src)
.attr("Visible", "true")
.attr("Highlight", "None")
.attr("Width", "Thin")
.attr("BorderStyle", "Solid")
.attr("Hidden", "false")
.attr("DestinationUniqueKey", h.id & 0xFFFF_FFFF);
let mut props = El::new("Properties");
props.push(p("Destination", "object", &dest));
el.push(props);
root.push(el);
}
}
for (src, target) in self.xref_sources.clone() {
let mut h = El::new("Hyperlink")
.attr("Self", self.fresh())
@@ -358,6 +430,34 @@ fn text_variable_el(v: &designcraft_doc::vars::TextVariable) -> El {
e
}
impl Ex<'_> {
/// A `HyperlinkURLDestination` for `url`, written once however many links lead to it.
fn url_dest_el(&mut self, root: &mut El, url: &str, name: &str) -> String {
let me = format!("HyperlinkURLDestination/u{:x}", fnv(url));
if !has_self(root, "HyperlinkURLDestination", &me) {
root.push(
El::new("HyperlinkURLDestination")
.attr("Self", &me)
.attr("Name", if name.is_empty() { url } else { name })
.attr("DestinationURL", url)
.attr("Hidden", "false"),
);
}
me
}
}
/// A short stable name for a string, so one URL gives one destination element.
fn fnv(s: &str) -> u64 {
s.bytes().fold(0xcbf2_9ce4_8422_2325u64, |h, b| (h ^ u64::from(b)).wrapping_mul(0x100_0000_01b3))
}
/// Whether `root` already holds an element of this name with this `Self`. IDML references name
/// one element, so a `Self` written twice is a broken file.
fn has_self(root: &El, name: &str, me: &str) -> bool {
root.children.iter().any(|n| matches!(n, crate::xml::Node::El(e) if e.name == name && e.get("Self") == Some(me)))
}
#[cfg(test)]
mod tests {
use super::*;
+76 -1
View File
@@ -359,7 +359,13 @@ impl<'a> Ex<'a> {
let mut psr = self.psr_el(&pf);
let ends_with_break = s.text.get(r.clone()).and_then(|t| t.chars().next_back()).is_some_and(st::is_break_char);
let segs = Self::paragraph_runs(s, &r);
// A hyperlink on a run of text is written as a `HyperlinkTextSource` around the
// ranges it covers, so the runs are cut at its edges first (#1446).
let (links, segs) = self.cut_runs_at_links(s, &r, segs);
let nseg = segs.len();
// Each segment's ranges, with the link they belong to; wrapped once the paragraph is
// built so that consecutive ranges of one link share a single source element.
let mut psr_csrs: Vec<(Option<usize>, El)> = Vec::new();
for (k, (rr, f)) in segs.into_iter().enumerate() {
let seg_start = rr.start;
let text = &s.text[rr];
@@ -520,13 +526,82 @@ impl<'a> Ex<'a> {
csrs.push(c);
}
self.apply_tracked_changes(s, &f, &mut csrs, para_br, r.end);
let link = links.iter().find(|(_, l)| l.contains(&seg_start) || (l.start == seg_start && l.start == l.end)).map(|(i, _)| *i);
for c in csrs {
psr.push(c);
psr_csrs.push((link, c));
}
}
self.wrap_link_sources(&mut psr, psr_csrs);
psr
}
/// The hyperlinks on story text that fall in the paragraph `r` (each link's index and its
/// range clipped to `r`), and the paragraph's runs `segs` cut at their edges, so a link's
/// ranges can be wrapped on their own (#1446).
fn cut_runs_at_links(
&self,
s: &Story,
r: &std::ops::Range<usize>,
segs: Vec<(std::ops::Range<usize>, CharFormat)>,
) -> (Vec<(usize, std::ops::Range<usize>)>, Vec<(std::ops::Range<usize>, CharFormat)>) {
let links: Vec<(usize, std::ops::Range<usize>)> = self
.d
.hyperlinks
.iter()
.enumerate()
.filter_map(|(i, h)| match &h.source {
designcraft_doc::HyperlinkSource::Text { story, start, end } if *story == s.id => {
let (a, b) = ((*start).max(r.start), (*end).min(r.end));
(a < b).then_some((i, a..b))
}
_ => None,
})
.collect();
let mut cuts: Vec<usize> = links.iter().flat_map(|(_, l)| [l.start, l.end]).collect();
cuts.sort_unstable();
cuts.dedup();
let segs = segs.into_iter().fold(Vec::new(), |mut out: Vec<(std::ops::Range<usize>, CharFormat)>, (rr, f)| {
let mut at = rr.start;
for c in cuts.iter().copied().filter(|c| rr.start < *c && *c < rr.end) {
out.push((at..c, f.clone()));
at = c;
}
out.push((at..rr.end, f));
out
});
(links, segs)
}
/// Push the paragraph's character ranges into `psr`, with each run of consecutive ranges of
/// one link inside a single `HyperlinkTextSource` (#1446).
fn wrap_link_sources(&mut self, psr: &mut El, psr_csrs: Vec<(Option<usize>, El)>) {
let mut groups: Vec<(Option<usize>, Vec<El>)> = Vec::new();
for (link, c) in psr_csrs {
match groups.last_mut() {
Some((l, v)) if *l == link => v.push(c),
_ => groups.push((link, vec![c])),
}
}
for (link, cs) in groups {
match link.and_then(|i| self.d.hyperlinks.get(i).map(|h| (i, h.name.clone()))) {
None => {
for c in cs {
psr.push(c);
}
}
Some((i, name)) => {
let me = self.fresh();
let mut src = El::new("HyperlinkTextSource").attr("Self", &me).attr("Name", name).attr("Hidden", "false");
for c in cs {
src.push(c);
}
psr.push(src);
self.link_text_sources.push((me, i));
}
}
}
}
pub(super) fn psr_el(&mut self, pf: &ParaFormat) -> El {
let mut el = El::new("ParagraphStyleRange").attr("AppliedParagraphStyle", names::style_self("ParagraphStyle", PARA_BUILTINS, &pf.style));
let mut props = Vec::new();
+147
View File
@@ -1585,6 +1585,153 @@ fn page_item_and_text_hyperlinks_survive_import() {
);
}
/// The other direction of #1446: a document's own hyperlinks are written to IDML and read back.
/// Export wrote none of them, so a link made here (or imported from an InDesign file) was lost
/// the moment the document went out as IDML.
#[test]
fn round_trips_page_item_and_text_hyperlinks() {
use designcraft_doc::{Hyperlink, HyperlinkDest as D, HyperlinkSource as S};
let mut d = Document::new(&NewDocument { pages: 2, ..NewDocument::default() });
let lid = d.default_layer();
let (_, sid) =
d.add_text_frame(SpreadRef::Doc(0), Rect::new(40.0, 40.0, 400.0, 400.0), lid, "See the plate overleaf.", ParaFormat::default()).unwrap();
let photo = designcraft_doc::ItemId(d.alloc());
let frame = designcraft_doc::Item::new(photo, lid, Shape::Rectangle, shapes::rectangle(Rect::new(40.0, 420.0, 200.0, 500.0)));
d.insert_item(SpreadRef::Doc(0), frame, None).unwrap();
let caption = d.paragraph_anchor(sid, 0).unwrap();
// One of each kind a link can have: a frame to a URL, a frame to an email address, a frame to
// a text anchor, and a run of text to a page.
let at = d.story(sid).unwrap().text.find("the plate").unwrap();
for (name, source, dest) in [
("Photo", S::Item { id: photo }, D::Url("https://example.com/photo.jpg".into())),
("Studio", S::Item { id: photo }, D::Email("studio@example.com".into())),
("Caption", S::Item { id: photo }, D::Anchor(caption)),
("Plate", S::Text { story: sid, start: at, end: at + "the plate".len() }, D::Page(1)),
] {
let id = d.alloc();
d.hyperlinks.push(Hyperlink { id, name: name.into(), source, dest });
}
d.check().unwrap();
let idml = export_idml(&d);
let back = import_idml(&idml).unwrap();
back.check().unwrap();
let named = |n: &str| back.hyperlinks.iter().find(|h| h.name == n).unwrap_or_else(|| panic!("{n} came back: {:?}", back.hyperlinks));
assert_eq!(back.hyperlinks.len(), 4, "every link came back: {:?}", back.hyperlinks);
assert_eq!(named("Photo").dest, D::Url("https://example.com/photo.jpg".into()));
assert_eq!(named("Studio").dest, D::Email("studio@example.com".into()), "an email link is not written as a bare URL");
assert_eq!(named("Plate").dest, D::Page(1), "the page destination is the same page");
// The frame links point at the frame, which has a new id on the way through.
let S::Item { id } = named("Photo").source else { panic!("a page item source: {:?}", named("Photo").source) };
assert!(back.item(id).is_some(), "the source frame is in the document");
assert_eq!(back.item(id).map(|i| i.shape), Some(Shape::Rectangle), "and it is the frame the link was on");
// The text link covers the same words, and the anchor link resolves to the same paragraph.
let S::Text { story, start, end } = named("Plate").source else { panic!("a text source: {:?}", named("Plate").source) };
assert_eq!(&back.story(story).unwrap().text[start..end], "the plate");
let D::Anchor(a) = named("Caption").dest else { panic!("an anchor destination: {:?}", named("Caption").dest) };
assert_eq!(back.find_anchor(a).map(|(_, pos)| pos), Some(0), "the anchor is the paragraph it was on");
// One destination element per URL, however many links lead to it.
let text = zip_text(&idml, "designmap.xml");
assert_eq!(text.matches("<HyperlinkURLDestination").count(), 2, "one for the photo and one for the mailto: {text}");
assert_eq!(text.matches("mailto:studio@example.com").count(), 1);
// Every `Self` names one element. Three links sit on `photo`, so there are three source
// elements and they cannot share a `Self`; the page destination is written once however
// many links lead to it, as the URL destinations are (noted in review).
let selfs: Vec<&str> = text.match_indices("Self=\"").map(|(i, _)| &text[i + 6..][..text[i + 6..].find('"').unwrap_or(0)]).collect();
let mut seen = std::collections::HashSet::new();
let repeated: Vec<&&str> = selfs.iter().filter(|s| !seen.insert(**s)).collect();
assert!(repeated.is_empty(), "no Self is written twice, but these are: {repeated:?}");
assert_eq!(text.matches("<HyperlinkPageItemSource").count(), 3, "one per link on the frame: {text}");
assert_eq!(text.matches("<HyperlinkPageDestination").count(), 1, "one for the page, not one per link to it: {text}");
}
/// A link to an anchor whose text has been deleted names no element, so writing it put a
/// `Hyperlink` in the file whose `Destination` points at nothing: import only warns, and
/// InDesign may refuse the file. The anchor is resolved first, as the page destination's page is
/// (noted in review of #1446).
#[test]
fn a_link_to_an_anchor_the_document_no_longer_has_is_not_written() {
use designcraft_doc::{Hyperlink, HyperlinkDest as D, HyperlinkSource as S};
let mut d = Document::new(&NewDocument { pages: 2, ..NewDocument::default() });
let lid = d.default_layer();
let (_, sid) =
d.add_text_frame(SpreadRef::Doc(0), Rect::new(40.0, 40.0, 400.0, 400.0), lid, "See the plate overleaf.", ParaFormat::default()).unwrap();
let photo = designcraft_doc::ItemId(d.alloc());
let frame = designcraft_doc::Item::new(photo, lid, Shape::Rectangle, shapes::rectangle(Rect::new(40.0, 420.0, 200.0, 500.0)));
d.insert_item(SpreadRef::Doc(0), frame, None).unwrap();
// An anchor id the document has never had, alongside a link that is fine.
for (name, dest) in [("Dangling", D::Anchor(999_999)), ("Fine", D::Page(1))] {
let id = d.alloc();
d.hyperlinks.push(Hyperlink { id, name: name.into(), source: S::Item { id: photo }, dest });
}
assert!(d.find_anchor(999_999).is_none(), "the document really has no such anchor");
d.check().unwrap();
let idml = export_idml(&d);
let text = zip_text(&idml, "designmap.xml");
assert!(!text.contains("Dangling"), "the dangling link is not written: {text}");
assert_eq!(text.matches("<Hyperlink ").count(), 1, "only the link that leads somewhere: {text}");
let back = import_idml(&idml).unwrap();
back.check().unwrap();
assert_eq!(back.hyperlinks.len(), 1, "{:?}", back.hyperlinks);
assert_eq!(back.hyperlinks[0].name, "Fine");
let _ = sid;
}
/// A `HyperlinkTextSource` nests inside a `ParagraphStyleRange`, so a link whose text crosses a
/// paragraph cannot be one element: it is written as one source per paragraph, and each piece
/// comes back as a link of its own. Pinned so the behaviour is a decision and not a surprise, and
/// so it is clear what has to change if IDML ever lets one source span paragraphs (#1446).
#[test]
fn a_hyperlink_across_a_paragraph_is_written_as_one_source_per_paragraph() {
use designcraft_doc::{Hyperlink, HyperlinkDest as D, HyperlinkSource as S};
let mut d = Document::new(&NewDocument::default());
let lid = d.default_layer();
let (_, sid) =
d.add_text_frame(SpreadRef::Doc(0), Rect::new(40.0, 40.0, 400.0, 400.0), lid, "first line\nsecond line", ParaFormat::default()).unwrap();
let text = d.story(sid).unwrap().text.clone();
let (start, end) = (text.find("line").unwrap(), text.find("second").unwrap() + "second".len());
let id = d.alloc();
d.hyperlinks.push(Hyperlink {
id,
name: "Across".into(),
source: S::Text { story: sid, start, end },
dest: D::Url("https://example.com/".into()),
});
d.check().unwrap();
let idml = export_idml(&d);
assert_eq!(zip_text(&idml, "Stories/").matches("<HyperlinkTextSource").count(), 2, "one source in each paragraph");
let back = import_idml(&idml).unwrap();
back.check().unwrap();
let across: Vec<_> = back.hyperlinks.iter().filter(|h| h.name == "Across").collect();
assert_eq!(across.len(), 2, "each piece is a link of its own: {:?}", back.hyperlinks);
let piece = |h: &Hyperlink| {
let S::Text { story, start, end } = h.source else { panic!("a text source: {:?}", h.source) };
back.story(story).unwrap().text[start..end].to_string()
};
let mut pieces: Vec<String> = across.iter().map(|h| piece(h)).collect();
pieces.sort();
// The first piece keeps the paragraph break, because the link's range covered it: the break
// is the last character of its paragraph, so the source that wraps that paragraph's text
// wraps the break with it. Between them the pieces cover exactly what the link covered.
assert_eq!(pieces, vec!["line\n".to_string(), "second".to_string()], "between them they cover the text the link was on");
assert_eq!(pieces.concat(), d.story(sid).unwrap().text[start..end], "and nothing of it is lost or added");
assert!(across.iter().all(|h| h.dest == D::Url("https://example.com/".into())), "both pieces lead where the link led");
}
/// A link from a frame anchored in text is kept, but PDF export finds a link's frame among the
/// spreads' items, where an anchored frame is not: the import report says the PDF will not have it,
/// rather than the link going missing from the PDF without a word.