From 2336b247460285c2fa10a51bcc3d6324e1d8eb09 Mon Sep 17 00:00:00 2001 From: storybored379 Date: Sat, 3 Oct 2026 21:14:59 -0700 Subject: [PATCH] M4: Deleted pages leave the file even when bookmarks, links or widgets still point at them (#2) Delete Pages only took the pages out of the page tree, so a bookmark, link or form widget that referenced a deleted page kept it (and its content) in a full save. Deleted page objects are now freed; their widgets leave the form (with fields left without widgets), and bookmarks and links to them lose that destination. Co-Authored-By: Claude Opus 5.5 --- crates/organize/src/lib.rs | 94 +++++++++++++++++++++++++++++++++- crates/organize/src/outline.rs | 12 +++++ crates/organize/src/tests.rs | 29 +++++++++++ 3 files changed, 134 insertions(+), 1 deletion(-) diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 137f50d6..9f57f9cd 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -157,7 +157,99 @@ pub fn delete_pages(doc: &mut Document, indices: &[usize]) -> Result<(), Organiz if keep.is_empty() { return Err(OrganizeError::WouldRemoveAllPages); } - rebuild(doc, &keep) + rebuild(doc, &keep)?; + // The deleted pages go for good: bookmarks, links and widgets that still point at one would + // otherwise keep it, with its content, in the saved file. Their form widgets leave the form, + // and bookmarks and links that went to them lose that destination. + let gone: Vec = all.iter().map(|(r, _)| *r).filter(|r| !keep.iter().any(|(k, _)| k == r)).collect(); + let mut widgets = Vec::new(); + for r in &gone { + let annots = doc.get(*r).as_dict().and_then(|d| d.get(b"Annots").cloned()).map(|a| doc.resolve(&a).as_array().cloned().unwrap_or_default()); + for a in annots.unwrap_or_default() { + if let Some(w) = a.as_ref().filter(|w| doc.get(*w).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))) { + widgets.push(w); + } + } + } + drop_widgets(doc, widgets)?; + drop_destinations_to(doc, &gone, &keep)?; + for r in gone { + doc.free(r); + } + Ok(()) +} + +/// Bookmarks, and links on the `keep` pages, that go to one of the `gone` pages lose that +/// destination (/Dest, or a GoTo /A), rather than pointing at nothing. +fn drop_destinations_to(doc: &mut Document, gone: &[ObjRef], keep: &[(ObjRef, Dict)]) -> Result<(), OrganizeError> { + let mut holders = outline::items(doc); + for (p, _) in keep { + let annots = doc.get(*p).as_dict().and_then(|d| d.get(b"Annots").cloned()).map(|a| doc.resolve(&a).as_array().cloned().unwrap_or_default()); + holders.extend( + annots + .unwrap_or_default() + .iter() + .filter_map(Object::as_ref) + .filter(|a| doc.get(*a).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Link"))), + ); + } + let to_gone = |o: Option<&Object>| { + o.map(|o| doc.resolve(o)).and_then(|o| o.as_array().and_then(|a| a.first()).and_then(Object::as_ref)).is_some_and(|r| gone.contains(&r)) + }; + let mut dead: Vec<(ObjRef, &[u8])> = Vec::new(); + for h in holders { + let Some(d) = doc.get(h).as_dict().cloned() else { continue }; + if to_gone(d.get(b"Dest")) { + dead.push((h, b"Dest")); + } + let action = d.get(b"A").map(|a| doc.resolve(a)).and_then(|a| a.as_dict().cloned()); + if action.is_some_and(|a| a.name(b"S") == Some(b"GoTo") && to_gone(a.get(b"D"))) { + dead.push((h, b"A")); + } + } + for (h, key) in dead { + doc.update_dict(h, |d| { + d.remove(key); + })?; + } + Ok(()) +} + +/// Take widgets out of the form: out of their field's /Kids, or out of /AcroForm /Fields when +/// they are fields themselves. A field left without kids goes too. +fn drop_widgets(doc: &mut Document, mut widgets: Vec) -> Result<(), OrganizeError> { + let unlink = |list: Option<&Object>, w: ObjRef| -> Option> { + let list = list.and_then(Object::as_array)?; + list.iter().any(|o| o.as_ref() == Some(w)).then(|| list.iter().filter(|o| o.as_ref() != Some(w)).cloned().collect()) + }; + while let Some(w) = widgets.pop() { + if let Some(parent) = doc.get(w).as_dict().and_then(|d| d.reference(b"Parent")) { + let kids = doc.get(parent).as_dict().and_then(|d| unlink(d.get(b"Kids").map(|k| doc.resolve(k)).as_deref(), w)); + if let Some(kids) = kids { + if kids.is_empty() { + widgets.push(parent); + } + doc.update_dict(parent, |d| d.set(b"Kids".to_vec(), Object::Array(kids)))?; + } + continue; + } + let Some(root) = doc.root() else { continue }; + match doc.get(root).as_dict().and_then(|c| c.get(b"AcroForm").cloned()) { + Some(Object::Ref(form)) => { + if let Some(fields) = doc.get(form).as_dict().and_then(|f| unlink(f.get(b"Fields").map(|x| doc.resolve(x)).as_deref(), w)) { + doc.update_dict(form, |f| f.set(b"Fields".to_vec(), Object::Array(fields)))?; + } + } + Some(Object::Dict(mut form)) => { + if let Some(fields) = unlink(form.get(b"Fields").map(|x| doc.resolve(x)).as_deref(), w) { + form.set(b"Fields".to_vec(), Object::Array(fields)); + doc.update_dict(root, |c| c.set(b"AcroForm".to_vec(), Object::Dict(form)))?; + } + } + _ => {} + } + } + Ok(()) } /// Move the pages at `indices` (kept in their relative order) so they start at position `to` diff --git a/crates/organize/src/outline.rs b/crates/organize/src/outline.rs index 6379b143..82e78195 100644 --- a/crates/organize/src/outline.rs +++ b/crates/organize/src/outline.rs @@ -65,6 +65,18 @@ fn children_of(doc: &Document, parent: ObjRef, seen: &mut HashSet) -> Ve out } +/// Every bookmark item, parents before their children. Cycle-safe. +pub(crate) fn items(doc: &Document) -> Vec { + let Some(root) = outline_root(doc) else { return Vec::new() }; + let (mut seen, mut out, mut stack) = (HashSet::new(), Vec::new(), vec![root]); + while let Some(parent) = stack.pop() { + let kids = children_of(doc, parent, &mut seen); + stack.extend(kids.iter().copied()); + out.extend(kids); + } + out +} + /// The bookmark tree. pub fn bookmarks(doc: &Document) -> Vec { let Some(root) = outline_root(doc) else { return Vec::new() }; diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 9889f886..5d105a77 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -117,6 +117,35 @@ fn delete_keeps_inherited_attributes_on_survivors() { assert_eq!(root.as_dict().unwrap().int(b"Count"), Some(2)); } +#[test] +fn deleted_pages_are_not_kept_by_what_points_at_them() { + // A1 links to A3, a bookmark goes to A3, and A2 holds the form's only field. After deleting + // A2 and A3 neither page, its content nor the field may stay in the saved file. + let mut doc = doc_a(); + add_bookmark(&mut doc, &[], 0, "To A3", 2).unwrap(); + delete_pages(&mut doc, &[1, 2]).unwrap(); + let out = full_roundtrip(&doc); + assert_eq!(labels(&out), ["A1"]); + // The bookmark and A1's link now go nowhere instead of to a missing page. + let mark = out.get(bookmarks(&out)[0].obj).as_dict().cloned().unwrap(); + assert!(!mark.contains(b"Dest") && !mark.contains(b"A"), "{mark:?}"); + let link = &annots(&out, 0)[0]; + assert!(!link.contains(b"A") && !link.contains(b"Dest"), "{link:?}"); + let mut page_objects = 0; + for n in out.object_numbers() { + let o = out.get(ObjRef::new(n, out.generation(n))); + if let Object::Stream(s) = &*o { + let data = String::from_utf8_lossy(&s.decoded().unwrap()).into_owned(); + assert!(!data.contains("(A2)") && !data.contains("(A3)"), "object {n} still holds a deleted page's content"); + } + page_objects += usize::from(o.as_dict().is_some_and(|d| d.name(b"Type") == Some(b"Page"))); + } + assert_eq!(page_objects, 1); + let cat = out.get(out.root().unwrap()).as_dict().cloned().unwrap(); + let form = out.resolve(cat.get(b"AcroForm").unwrap()); + assert_eq!(form.as_dict().and_then(|f| f.get(b"Fields")).and_then(Object::as_array).map(Vec::len), Some(0), "the field went with its page"); +} + #[test] fn cannot_delete_every_page_or_missing_pages() { let mut doc = open(fixture());