From 4f0493cc21e512087f6e625a5ddbd5426b777d21 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 00:09:53 +0300 Subject: [PATCH] organize: split a combined field/widget so every copied page owns its widget --- crates/organize/src/import.rs | 90 ++++++++++++++++++++++++++++------- crates/organize/src/tests.rs | 64 ++++++++++++++++++++----- 2 files changed, 125 insertions(+), 29 deletions(-) diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index 588539866a..ef18a121f0 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -34,6 +34,13 @@ const SKIP_ON_ANNOT: &[&[u8]] = &[b"StructParent", b"P"]; /// objects, each shallow enough for the parser, would otherwise exhaust the stack (#1800). const MAX_COPY_DEPTH: usize = 256; +/// Keys that stay on the widget when a combined field/widget is split (#1810): the annotation's +/// appearance, placement and page. Field state (name, value, flags, default appearance) moves to +/// the new parent field, where the widgets inherit it. +const WIDGET_KEYS: &[&[u8]] = &[ + b"Type", b"Subtype", b"Rect", b"P", b"AP", b"AS", b"MK", b"BS", b"Border", b"C", b"CA", b"F", b"H", b"R", b"NM", b"M", b"StructParent", b"StructParents", b"OC", b"Tabs", +]; + struct Copier<'a> { src: &'a Document, /// Current recursion depth, bounded by [`MAX_COPY_DEPTH`]. @@ -393,27 +400,32 @@ fn import_pages_mapped( let r = dst.add(Object::Null); let c = copier.copy_value(dst, &resolved, Some(r))?; dst.set(r, c); - // `/Kids` may be an array object of its own: add to it there, so the - // field keeps its other widgets. - match dst.get(copied).as_dict().and_then(|f| f.get(b"Kids").cloned()) { - Some(Object::Ref(list)) => { - let mut kids = dst.get(list).as_array().cloned().unwrap_or_default(); - kids.push(Object::Ref(r)); - dst.set(list, Object::Array(kids)); - } - _ => dst.update_dict(copied, |f| { - let mut kids = f.get(b"Kids").and_then(|k| k.as_array()).cloned().unwrap_or_default(); - kids.push(Object::Ref(r)); - f.set(b"Kids".to_vec(), Object::Array(kids)); - })?, - } + add_kid(dst, copied, r)?; annot_pages.insert(r, new); out.push(Object::Ref(r)); continue; } - // A widget that is its own field (no parent to hang a second widget on) stays - // the one object, shown on both pages, so the field stays one. + // A widget that is its own field (no parent to hang a second widget on): on + // its first copy it is split into a parent field and a widget child (#1810); + // every later copy of the page gets one more widget child of that field, so + // the pages no longer share one widget whose `/P` names only the last page. let own_field = resolved.as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget") && !d.contains(b"Parent")); + if own_field + && let Some(first) = a.as_ref().and_then(|r| copier.map.get(&r).copied()) + && let Some(parent) = dst.get(first).as_dict().and_then(|d| d.reference(b"Parent")) + { + let r = dst.add(Object::Null); + let c = copier.copy_value(dst, &resolved, Some(r))?; + let Object::Dict(mut d) = c else { continue }; + d.set(b"Parent".to_vec(), Object::Ref(parent)); + // The new widget is not a field root of its own. + copier.fields.retain(|f| *f != r); + dst.set(r, Object::Dict(d)); + add_kid(dst, parent, r)?; + annot_pages.insert(r, new); + out.push(Object::Ref(r)); + continue; + } let annots = if own_field { &mut copier } else { own.as_mut().unwrap_or(&mut copier) }; // Direct (inline) annotation dictionaries become indirect objects, so every // copied annotation can be fixed up the same way. @@ -427,6 +439,9 @@ fn import_pages_mapped( } _ => continue, }; + if own_field && copier.fields.contains(&r) { + split_own_field(dst, &mut copier.fields, r); + } annot_pages.insert(r, new); out.push(Object::Ref(r)); } @@ -805,6 +820,49 @@ fn qualified_optional(prefix: &str, name: &str) -> String { if name.is_empty() { prefix.to_string() } else { qualified(prefix, name) } } +/// Add `kid` to `field`'s `/Kids`, in the array object itself when `/Kids` is one, so the field +/// keeps its other widgets. +fn add_kid(dst: &mut Document, field: ObjRef, kid: ObjRef) -> Result<(), OrganizeError> { + match dst.get(field).as_dict().and_then(|f| f.get(b"Kids").cloned()) { + Some(Object::Ref(list)) => { + let mut kids = dst.get(list).as_array().cloned().unwrap_or_default(); + kids.push(Object::Ref(kid)); + dst.set(list, Object::Array(kids)); + } + _ => dst.update_dict(field, |f| { + let mut kids = f.get(b"Kids").and_then(|k| k.as_array()).cloned().unwrap_or_default(); + kids.push(Object::Ref(kid)); + f.set(b"Kids".to_vec(), Object::Array(kids)); + })?, + } + Ok(()) +} + +/// PdfCraft patch (#1810): split a widget that doubles as its own field into a parent field +/// holding the field keys (name, value, flags…) and a widget child holding the annotation keys +/// (rectangle, appearance, page). The parent takes the widget's place among the top-level fields; +/// later copies of the page add widget children of it, so every page has its own widget and the +/// field stays one. +fn split_own_field(dst: &mut Document, fields: &mut [ObjRef], widget: ObjRef) { + let Some(d) = dst.get(widget).as_dict().cloned() else { return }; + let (mut field, mut child) = (Dict::new(), Dict::new()); + for (k, v) in d.iter() { + if WIDGET_KEYS.contains(&k.as_slice()) { + child.set(k.clone(), v.clone()); + } else { + field.set(k.clone(), v.clone()); + } + } + let parent = dst.add(Object::Null); + field.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(widget)])); + child.set(b"Parent".to_vec(), Object::Ref(parent)); + dst.set(parent, Object::Dict(field)); + dst.set(widget, Object::Dict(child)); + if let Some(entry) = fields.iter_mut().find(|f| **f == widget) { + *entry = parent; + } +} + /// Add top-level fields to the destination's `/AcroForm /Fields` (creating the form if needed). fn register_fields(dst: &mut Document, fields: &[ObjRef]) -> Result<(), OrganizeError> { if fields.is_empty() { diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index d34b1e431d..c1e20a1327 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -672,9 +672,40 @@ fn links_are_rewired_to_copied_pages_or_dropped() { assert!(!link.contains(b"Dest") && !link.contains(b"A")); } -/// A widget annotation that is its own form field (no `/Parent`). -fn own_field(doc: &Document, r: ObjRef) -> bool { - doc.get(r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget") && !d.contains(b"Parent")) +/// Issue #1810: a widget that is its own field was shared between both copies of a repeated +/// page, so only the last page could identify it (its `/P`), and per-page annotation edits were +/// impossible. Each copy must get its own widget sharing one parent field (name and value). +#[test] +fn repeated_page_splits_a_combined_field_widget() { + let out = full_roundtrip(&extract_pages(&doc_a(), &[1, 1]).unwrap()); + let ps = pages(&out).unwrap(); + let widget = |i: usize| -> ObjRef { + let list = page_dict(&out, i).get(b"Annots").map(|a| out.resolve(a).as_array().cloned().unwrap_or_default()).unwrap_or_default(); + list.iter().filter_map(|a| a.as_ref()).find(|r| out.get(*r).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Widget"))).expect("a widget annotation") + }; + let (w1, w2) = (widget(0), widget(1)); + assert_ne!(w1, w2, "each copy of the page has its own widget"); + for (i, w) in [(0, w1), (1, w2)] { + let d = out.get(w).as_dict().unwrap().clone(); + assert_eq!(d.reference(b"P"), Some(ps[i].obj), "/P names the page holding the widget"); + let parent = d.reference(b"Parent").expect("the widget has a parent field"); + let f = out.get(parent).as_dict().unwrap().clone(); + assert_eq!(f.get(b"T").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("name")); + assert_eq!(f.get(b"V").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("Ada")); + let kids: Vec = f.get(b"Kids").and_then(|k| out.resolve(k).as_array().cloned()).unwrap_or_default().iter().filter_map(|k| k.as_ref()).collect(); + assert!(kids.contains(&w1) && kids.contains(&w2), "one field holds both widgets: {kids:?}"); + let fields: Vec = catalog(&out) + .get(b"AcroForm") + .map(|f| out.resolve(f)) + .and_then(|f| f.as_dict().cloned()) + .and_then(|f| f.get(b"Fields").map(|x| out.resolve(x))) + .and_then(|f| f.as_array().cloned()) + .unwrap_or_default() + .iter() + .filter_map(|f| f.as_ref()) + .collect(); + assert_eq!(fields, [parent], "the parent is the one top-level field"); + } } #[test] @@ -690,15 +721,11 @@ fn repeated_page_gets_independent_annotations() { assert!(!refs[i].is_empty()); assert_eq!(refs[i].len(), refs[i + 1].len()); for (a, b) in refs[i].iter().zip(&refs[i + 1]) { - // A widget that is its own field stays one object on both pages, so the field stays - // one (#790); every other annotation is copied. - if !own_field(&out, *a) { - assert_ne!(a, b, "each copy of the page has its own annotation objects"); - } + assert_ne!(a, b, "each copy of the page has its own annotation objects"); } } for (i, list) in refs.iter().enumerate() { - for a in list.iter().filter(|a| !own_field(&out, **a)) { + for a in list.iter() { assert_eq!(out.get(*a).as_dict().unwrap().reference(b"P"), Some(ps[i].obj), "/P names the page holding the copy"); } } @@ -719,7 +746,9 @@ fn form_fields_come_along_and_are_registered() { assert_eq!(fields.len(), 1); let field = out.resolve(&fields[0]).as_dict().cloned().unwrap(); assert_eq!(field.get(b"V").and_then(|v| v.as_string()).map(|s| s.to_text()).as_deref(), Some("Ada")); - assert_eq!(field.reference(b"P"), Some(pages(&out).unwrap()[0].obj)); + // The widget child, not the field, names the page (#1810). + let kid = field.get(b"Kids").map(|k| out.resolve(k)).and_then(|k| k.as_array().cloned()).and_then(|k| k.first().and_then(|k| k.as_ref())).unwrap(); + assert_eq!(out.get(kid).as_dict().unwrap().reference(b"P"), Some(pages(&out).unwrap()[0].obj)); } #[test] @@ -2547,8 +2576,9 @@ fn import_renames_a_field_colliding_with_another_incoming_field() { assert_eq!(form_field_names(&dst), ["Other", "Text1", "Text1-1"]); } -/// A page listed twice shows its widget on both copies — also when the field was renamed: both -/// pages' widgets belong to the one renamed field, and no second Text1 appears. +/// A page listed twice shows its widget on both copies — also when the field was renamed: each +/// copy has its own widget, both widgets belong to the one renamed field (#1810), and no second +/// Text1 appears. #[test] fn a_repeated_page_joins_the_renamed_field() { let mut dst = doc_with_field("Text1"); @@ -2565,10 +2595,18 @@ fn a_repeated_page_joins_the_renamed_field() { .and_then(|l| l.as_array().cloned()) .unwrap(); let renamed = fields[1].clone(); + let mut widgets = Vec::new(); for page in [1, 2] { let list = page_dict(&dst, page).get(b"Annots").map(|a| dst.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap(); - assert!(list.contains(&renamed), "page {page}'s widget belongs to the renamed field"); + let widget = list.iter().find_map(|a| a.as_ref()).expect("a widget on the page"); + widgets.push(widget); + assert_eq!( + dst.get(widget).as_dict().unwrap().reference(b"Parent"), + renamed.as_ref(), + "page {page}'s widget belongs to the renamed field" + ); } + assert_ne!(widgets[0], widgets[1], "each copy has its own widget"); } /// Combining forms keeps same-named fields separate — the second file's colliding field is