diff --git a/crates/engine/src/session/files.rs b/crates/engine/src/session/files.rs index 44bed85cde..85ed558f94 100644 --- a/crates/engine/src/session/files.rs +++ b/crates/engine/src/session/files.rs @@ -289,7 +289,8 @@ impl Session { /// [`Self::combine_unlocked`] where sources with the same `groups` key (by position) are one /// file split into several runs placed apart: each file is opened and copied once, so it keeps - /// one bookmark, its links between its own pages, its fields and its attachments whole (see + /// one bookmark, and its links between its own pages and its fields whole across its runs; its + /// attachments come along when its runs use all of its pages (see /// `pdfcraft_organize::combine_grouped`). A group's later sources use its first source's bytes /// and password; a source without a key is a file of its own. pub fn combine_grouped(&self, sources: &[CombineSource], groups: &[u64], passwords: &[Option<&str>]) -> Result>, EditError> { diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 9f86d6455a..e967e08533 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -100,8 +100,8 @@ fn added_objects(dst: &Document, before: &HashSet) -> Vec { /// Insert Pages from File: [`import_pages_with`], and the inserted file's bookmarks come along /// under one new bookmark titled `title` (the file's name) at its first inserted page, as Combine /// Files does for each file. That bookmark goes among the top-level bookmarks where the pages went: -/// before the first one that points after them. Bookmarks to pages left out lose their destination; -/// a file without bookmarks adds none. +/// before the first one that points after them. Bookmarks to pages left out stay behind, unless a +/// bookmark below them leads to an inserted page; a file without bookmarks adds none. pub fn insert_file_pages( dst: &mut Document, src: &Document, diff --git a/crates/organize/src/import/carry.rs b/crates/organize/src/import/carry.rs index 15a412e231..00fb9bd0b8 100644 --- a/crates/organize/src/import/carry.rs +++ b/crates/organize/src/import/carry.rs @@ -98,9 +98,13 @@ pub(super) fn source_outline(src: &Document, pages: HashMap) -> .and_then(|r| src.get(r).as_dict().cloned()) .and_then(|c| c.get(b"Outlines").map(|o| src.resolve(o))) .and_then(|o| o.as_dict().and_then(|d| d.reference(b"First"))); + let whole = crate::page_count(src).is_ok_and(|n| pages.len() >= n); let mapper = Copier::new(src, HashMap::new(), pages); let mut budget = 10_000usize; - outline_level(&mapper, first, 0, &mut budget) + let outline = outline_level(&mapper, first, 0, &mut budget); + // Of some of the pages, only the bookmarks that lead to a copied page and those above them: + // the others would carry their titles along into a file without their pages. + if whole { outline } else { kept_bookmarks(outline) } } /// `nodes` without the bookmarks whose page was not copied (no destination left), unless a bookmark diff --git a/crates/organize/src/import/combine.rs b/crates/organize/src/import/combine.rs index 06972ae101..7782e5de18 100644 --- a/crates/organize/src/import/combine.rs +++ b/crates/organize/src/import/combine.rs @@ -1,5 +1,5 @@ use super::carry::{Mark, add_outline, collect_attachments, kept_bookmarks, set_attachments, source_outline, write_outline_level}; -use super::{Dict, Document, FieldConflicts, HashMap, ObjRef, Object, OrganizeError, import_pages_mapped, rebuild}; +use super::{Dict, Document, FieldConflicts, HashMap, HashSet, ObjRef, Object, OrganizeError, import_pages_mapped, rebuild}; /// A new document containing copies of `pages` from `src` (Extract Pages / Split). /// Document information (title, author…) is carried over, and so is the print standard the @@ -95,7 +95,8 @@ pub fn combine_with(sources: &[(&str, &Document)], conflicts: FieldConflicts) -> } /// Combine Files with chosen pages: each source contributes `pages` (0-based, in that order; -/// `None` for all of them). Bookmarks that point at pages left out lose their destination. +/// `None` for all of them). A source whose pages are not all used brings only the bookmarks that +/// lead to its copied pages (and those above them), and none of its attachments. pub fn combine_selected(sources: &[(&str, &Document, Option<&[usize]>)]) -> Result { combine_selected_with(sources, FieldConflicts::Rename).map(|o| o.doc) } @@ -187,7 +188,11 @@ impl Combiner { let outline = source_outline(src, imported.page_map); self.marks.push(Mark { title: title.to_string(), page: *first, outline }); } - collect_attachments(&mut self.out, src, &mut self.attachments)?; + // Attachments belong to the whole file: they come along with all of its pages, not with a + // selection of them. + if pages.iter().collect::>().len() == n { + collect_attachments(&mut self.out, src, &mut self.attachments)?; + } let declared = crate::pdfx::PrintStandard::of(src); match self.standard.as_ref().map(|first| first.declared.same_as(&declared)) { Some(same) => self.agreed &= same, @@ -227,9 +232,9 @@ pub type Run<'a> = (usize, &'a str, &'a Document, Option<&'a [usize]>); /// Combine Files where a file's pages may be split into several runs with other files' pages /// between them. Each file (group) is copied once, all its pages together, then the pages are put -/// in the runs' order: its links between its own pages, its form fields and its attachments stay -/// whole, and it gets one top-level bookmark (the title of its first run) at its first page, its -/// own bookmarks nested. Otherwise as [`combine_selected`] (print standard, shared resources). +/// in the runs' order: its links between its own pages and its form fields stay whole across its +/// runs, its attachments come along when its runs use all of its pages, and it gets one top-level +/// bookmark (the title of its first run) at its first page, its own bookmarks nested. Otherwise as [`combine_selected`] (print standard, shared resources). pub fn combine_grouped(runs: &[Run<'_>]) -> Result { combine_grouped_with(runs, FieldConflicts::Rename).map(|o| o.doc) } @@ -281,7 +286,10 @@ pub fn combine_grouped_with(runs: &[Run<'_>], conflicts: FieldConflicts) -> Resu if let Some(first) = imported.pages.first() { marks.push(Mark { title: file.title.to_string(), page: *first, outline: source_outline(file.src, imported.page_map) }); } - collect_attachments(&mut out, file.src, &mut attachments)?; + // Attachments come along with all of a file's pages, not with a selection of them. + if file.pages.iter().collect::>().len() == crate::page_count(file.src)? { + collect_attachments(&mut out, file.src, &mut attachments)?; + } let declared = crate::pdfx::PrintStandard::of(file.src); match &standard { None => standard = Some((declared, file.src, imported.objects)), diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index d5ff2116ff..768b22e21d 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -1591,13 +1591,12 @@ fn inserting_a_file_brings_its_bookmarks_under_one_named_after_it() { let page_of = |title: &str| marks.iter().find(|b| b.title == title).and_then(|b| b.page); assert_eq!((page_of("annex"), page_of("Chapter one"), page_of("Section"), page_of("End")), (Some(1), Some(1), Some(2), Some(4))); assert!(!marks.iter().find(|b| b.title == "annex").unwrap().open, "the file's bookmarks start collapsed"); - // Inserted last, after every bookmark; only the pages chosen keep a destination. + // Inserted last, after every bookmark; a bookmark to a page left out stays behind with it. let mut host = open(fixture()); add_bookmark(&mut host, &[], 0, "Start", 0).unwrap(); insert_file_pages(&mut host, &doc_c(), "annex", &[0], 9, FieldConflicts::Rename).unwrap(); let marks = bookmarks(&host); - assert_eq!(titles(&marks), ["Start", "annex[Chapter one[Section]]"]); - assert_eq!(marks.iter().find(|b| b.title == "Section").unwrap().page, None, "its page was left out"); + assert_eq!(titles(&marks), ["Start", "annex[Chapter one]"], "Section's page was left out"); // A file without bookmarks adds none, and a host without an outline gets one only when needed. let mut host = open(fixture()); insert_file_pages(&mut host, &doc_b(), "plain", &[0], 0, FieldConflicts::Rename).unwrap(); @@ -3985,3 +3984,42 @@ fn an_outline_deeper_than_the_listing_is_exported_as_truncated() { crate::import_outline(&mut target, &[], 0, &tree.nodes, false).unwrap(); assert_eq!(crate::bookmarks(&target).len(), 33); } + +/// The titles of a bookmark's children, each with its own children's titles. +fn bookmark_titles(doc: &Document, first: Option) -> Vec<(String, Vec)> { + let mut out = Vec::new(); + let mut at = first; + while let Some(r) = at { + let item = doc.get(r).as_dict().cloned().unwrap(); + let title = item.get(b"Title").and_then(|t| t.as_string()).map(|t| t.to_text()).unwrap_or_default(); + let children = bookmark_titles(doc, item.reference(b"First")).into_iter().map(|(t, _)| t).collect(); + out.push((title, children)); + at = item.reference(b"Next"); + } + out +} + +/// Combining some of a file's pages brings only the bookmarks that lead to them (and those above +/// them), and none of the file's attachments. +#[test] +fn combining_some_pages_brings_only_their_bookmarks_and_no_attachments() { + let source_bookmarks = |out: &Document| { + let outlines = out.get(catalog(out).reference(b"Outlines").unwrap()).as_dict().cloned().unwrap(); + let file = out.get(outlines.reference(b"First").unwrap()).as_dict().cloned().unwrap(); + bookmark_titles(out, file.reference(b"First")) + }; + let attachments = |out: &Document| { + catalog(out).get(b"Names").map(|n| out.resolve(n)).is_some_and(|n| n.as_dict().is_some_and(|d| d.contains(b"EmbeddedFiles"))) + }; + // Page 1 only: "Section" leads to page 2. + let first = full_roundtrip(&combine_selected(&[("C", &doc_c(), Some(&[0]))]).unwrap()); + assert_eq!(source_bookmarks(&first), [("Chapter one".to_string(), vec![])]); + assert!(!attachments(&first), "an attachment came along with a selection of pages"); + // Page 2 only: "Chapter one" stays, as the way to "Section". + let second = full_roundtrip(&combine_selected(&[("C", &doc_c(), Some(&[1]))]).unwrap()); + assert_eq!(source_bookmarks(&second), [("Chapter one".to_string(), vec!["Section".to_string()])]); + // Every page: all of it, attachments too. + let all = full_roundtrip(&combine_selected(&[("C", &doc_c(), Some(&[0, 1]))]).unwrap()); + assert_eq!(source_bookmarks(&all), [("Chapter one".to_string(), vec!["Section".to_string()])]); + assert!(attachments(&all)); +}