From 0b0519f5df2056b9a9430d72021b88bd7b779cd6 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 10:57:05 -0500 Subject: [PATCH] fix(forms): deleting a field removes ancestors it leaves without kids (#6997) * forms: deleting a field removes ancestors it leaves without kids * forms test: full-save check, widget only in the sibling case --- crates/forms/src/author/lifecycle.rs | 18 +++++++++++++++- crates/forms/src/tests.rs | 32 ++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/forms/src/author/lifecycle.rs b/crates/forms/src/author/lifecycle.rs index 50d552ccdb..28b56749d9 100644 --- a/crates/forms/src/author/lifecycle.rs +++ b/crates/forms/src/author/lifecycle.rs @@ -262,10 +262,26 @@ pub fn delete_widget(doc: &mut Document, name: &str, widget: usize) -> Result<() fn delete_found_field(doc: &mut Document, f: Field) -> Result<(), FormError> { let widgets: std::collections::HashSet = f.widgets.iter().map(|w| w.obj).collect(); drop_from_pages(doc, &widgets)?; + let mut parent = doc.get(f.obj).as_dict().and_then(|d| d.reference(b"Parent")); detach(doc, f.obj)?; // Out of the calculation order too: a `/CO` entry would keep the field, its value and its // widgets reachable for a full save. - drop_from_calculation_order(doc, |r| r == f.obj || widgets.contains(&r)) + drop_from_calculation_order(doc, |r| r == f.obj || widgets.contains(&r))?; + // An ancestor left with no kids would become a field of its own, and keep the `/V` and `/DV` + // the deleted field inherited from it: it goes too, up the chain. + let mut gone = std::collections::HashSet::new(); + while let Some(p) = parent.filter(|p| gone.len() < 64 && !gone.contains(p)) { + let Some(d) = doc.get(p).as_dict().cloned() else { break }; + let empty = d.get(b"Kids").map(|o| doc.resolve(o)).and_then(|o| o.as_array().map(Vec::is_empty)) == Some(true); + if !empty { + break; + } + parent = d.reference(b"Parent"); + gone.insert(p); + drop_from_pages(doc, &std::iter::once(p).collect())?; + detach(doc, p)?; + } + drop_from_calculation_order(doc, |r| gone.contains(&r)) } /// Take the annotations `widgets` off every page's `/Annots`, held directly or by reference. diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index caeba9491d..75dd075889 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -1664,6 +1664,38 @@ fn properties_rename_and_delete() { assert_eq!(annots.as_array().unwrap().len(), 13 - 3, "the deleted widgets left the page"); } +/// A deleted field's ancestor left with no kids stays in `/Fields` as a field of its own, with the +/// `/V` the deleted field inherited from it: it leaves too. An ancestor a sibling still has stays. +#[test] +fn deleting_the_last_kid_removes_the_emptied_ancestors_and_their_values() { + let form = |kids: &str, annots: &str| { + document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + format!("<< /Type /Page /Parent 2 0 R /Annots [{annots}] >>"), + "<< /Fields [5 0 R] >>".into(), + "<< /FT /Tx /T (outer) /V (SECRET) /DV (SECRET) /Kids [6 0 R] >>".into(), + format!("<< /T (inner) /Parent 5 0 R /Kids [{kids}] >>"), + "<< /Type /Annot /Subtype /Widget /T (a) /Parent 6 0 R /Rect [50 700 250 720] /P 3 0 R >>".into(), + "<< /Type /Annot /Subtype /Widget /T (b) /Parent 6 0 R /Rect [50 650 250 670] /P 3 0 R >>".into(), + ]) + }; + let top = |doc: &Document| { + let af = doc.get(doc.root().unwrap()).as_dict().unwrap().get(b"AcroForm").map(|o| doc.resolve(o)).unwrap().as_dict().unwrap().clone(); + doc.resolve(af.get(b"Fields").unwrap()).as_array().unwrap().len() + }; + let mut doc = form("7 0 R", "7 0 R"); + delete_field(&mut doc, "outer.inner.a").unwrap(); + assert_eq!(top(&doc), 0, "the emptied ancestors left /Fields"); + let saved = pdfcraft_cos::write_full(&doc, &SaveOptions::default()).unwrap(); + assert!(!saved.windows(6).any(|w| w == b"SECRET"), "the inherited value is gone from a full save"); + + let mut doc = form("7 0 R 8 0 R", "7 0 R 8 0 R"); + delete_field(&mut doc, "outer.inner.a").unwrap(); + assert_eq!(top(&doc), 1, "an ancestor with a surviving kid stays"); + assert!(fields(&doc).iter().any(|f| f.name == "outer.inner.b")); +} + /// A parent `MASTER` with kids `a` and `b` (each a field merged with its widget) and `e` (a field /// with two widget kids), and the top-level fields `c` and `d`. fn dotted_form() -> Document {