Skip to content

organize: split a combined field/widget so every copied page owns its widget #2289

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
midasmatterhorn wants to merge 1 commit into
base: main
Choose a base branch
from
Open

organize: split a combined field/widget so every copied page owns its widget #2289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 74 additions & 16 deletions crates/organize/src/import.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`].
Expand Down Expand Up @@ -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.
Expand All @@ -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));
}
Expand Down Expand Up @@ -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() {
Expand Down
64 changes: 51 additions & 13 deletions crates/organize/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<ObjRef> = 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<ObjRef> = 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]
Expand All @@ -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");
}
}
Expand All @@ -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]
Expand Down Expand Up @@ -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");
Expand All @@ -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
Expand Down
Loading