diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index a8fcb6f7e..32e3cdc32 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -1127,6 +1127,57 @@ fn forms_through_tools() { assert_ne!(v, "Filled by an agent"); } +/// A form whose fields are only page widgets (the `/Fields` list is empty) still lists, fills and +/// resets through the tools, and the leniency shows up as a repair note. +#[test] +fn orphan_form_fields_through_tools() { + /// One page with two text fields that appear only as page widgets: the AcroForm lists none. + fn orphan_form() -> Vec { + let objs: Vec<&str> = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>", // 1 + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 300] >>", // 2 + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R] >>", // 3 + "<< /Fields [] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 7 0 R >> >> >>", // 4 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (alpha) /Rect [10 200 90 220] /P 3 0 R >>", // 5 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (beta) /Rect [10 150 90 170] /P 3 0 R >>", // 6 + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", // 7 + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + out + } + let dir = workdir("orphan-forms"); + std::fs::write(dir.join("orphan.pdf"), orphan_form()).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "orphan.pdf" }))["doc"].as_u64().unwrap(); + let list = ok(&mut a, "form_fields", json!({ "doc": doc })); + assert_eq!(list["count"], 2, "{list}"); + let r = ok(&mut a, "form_fill", json!({ "doc": doc, "values": { "alpha": "Filled by an agent" } })); + assert_eq!(r["undo"], "Fill in alpha"); + let after = ok(&mut a, "form_fields", json!({ "doc": doc })); + let alpha = after["fields"].as_array().unwrap().iter().find(|f| f["name"] == "alpha").unwrap(); + assert_eq!(alpha["value"], "Filled by an agent"); + // The page shows it. + assert_eq!(ok(&mut a, "text_find", json!({ "doc": doc, "query": "Filled by an agent" }))["count"], 1); + // The leniency is not silent: the repair note names the adopted fields. + let info = ok(&mut a, "doc_info", json!({ "doc": doc })); + assert!(info["repairs"].as_array().unwrap().iter().any(|r| r.as_str().unwrap_or("").contains("page annotations")), "{}", info["repairs"]); + ok(&mut a, "form_reset", json!({ "doc": doc })); + let reset = ok(&mut a, "form_fields", json!({ "doc": doc })); + let v = reset["fields"].as_array().unwrap().iter().find(|f| f["name"] == "alpha").unwrap()["value"].clone(); + assert_ne!(v, "Filled by an agent"); +} + #[test] fn duplicating_and_cropping_through_tools() { let dir = workdir("boxes"); diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 3ad5b678c..e2c2b219e 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -540,6 +540,14 @@ impl Document { &self.repair_log } + /// Add a line to [`Self::repair_log`]: a leniency applied while reading that the user should + /// know about (fidelity: never silent). + pub fn note_repair(&mut self, line: String) { + let mut log = self.repair_log.as_ref().clone(); + log.push(line); + self.repair_log = Arc::new(log); + } + /// `true` when there are unsaved edits. pub fn is_modified(&self) -> bool { !self.overlay.is_empty() diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c49c07511..4e38e8d32 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -2242,7 +2242,7 @@ impl Session { xfa: Option, ) -> Result { let config = RenderConfig { password: render_password.as_deref().map(Arc::from), ..Default::default() }; - let (editor, read_only_reason) = match cos { + let (mut editor, read_only_reason) = match cos { Ok(Ok(cos)) => { let keys = Keys { render: render_password.clone(), reopen: password.map(str::to_owned) }; (Some(Editor { cos, undo: Vec::new(), redo: Vec::new(), keys }), None) @@ -2253,8 +2253,21 @@ impl Session { let display = display_bytes(editor.as_ref(), &bytes); let renderer = RenderPool::new(display.clone(), render_threads(), config.clone()); let mut form = editor.as_ref().map(|e| pdfcraft_forms::fields(&e.cos)).unwrap_or_default(); + let mut adopted = 0; if let Some(e) = editor.as_ref() { xfa::mark_script_buttons(&e.cos, &mut form); + adopted = pdfcraft_forms::adopted_page_fields(&e.cos); + } + // Fields the file lists only as page widgets are adopted leniently; the file says so. + if adopted > 0 + && let Some(e) = &mut editor + { + let line = if adopted == 1 { + "the form's /Fields list omits 1 field; it was read from the page annotations".to_string() + } else { + format!("the form's /Fields list omits {adopted} fields; they were read from the page annotations") + }; + e.cos.note_repair(line); } let marks = editor.as_ref().map(|e| pdfcraft_edit::marks_present(&e.cos)).unwrap_or_default(); let added = editor.as_ref().map(|e| pdfcraft_edit::list_added(&e.cos)).unwrap_or_default(); diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 9f956db51..821d6366a 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2416,3 +2416,36 @@ fn placeholder_titles_are_recognised() { assert!(!is_placeholder_title(t), "{t:?}"); } } + +/// A form whose fields are only page widgets (the `/Fields` list is empty): they are adopted, and +/// the leniency is recorded in the repair log rather than applied silently. +#[test] +fn adopting_page_only_fields_is_noted_as_a_repair() { + let objs: Vec<&str> = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>", // 1 + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 300] >>", // 2 + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R] >>", // 3 + "<< /Fields [] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 7 0 R >> >> >>", // 4 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (alpha) /Rect [10 200 90 220] /P 3 0 R >>", // 5 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (beta) /Rect [10 150 90 170] /P 3 0 R >>", // 6 + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>", // 7 + ]; + let mut bytes = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(bytes.len()); + bytes.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = bytes.len(); + bytes.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + bytes.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + let mut s = Session::new().with_clock(|| 1_700_000_000); + let id = s.open("orphan.pdf", None, Arc::new(bytes), None).expect("opens"); + let doc = s.get(id).unwrap(); + let names: Vec<&str> = doc.form.iter().map(|f| f.name.as_str()).collect(); + assert_eq!(names, ["alpha", "beta"]); + assert!(doc.repair_log().iter().any(|l| l.contains("2 fields") && l.contains("page annotations")), "{:?}", doc.repair_log()); +} diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 0fadb2360..acbb7c6e9 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -484,9 +484,20 @@ struct Inherited { max_len: Option, } -/// Every terminal field, in tree order. +/// Every terminal field: the `/Fields` tree in order, then the fields reachable only through the +/// page annotations (see [`adopt_page_fields`]). pub fn fields(doc: &Document) -> Vec { - let Some(af) = acroform(doc) else { return Vec::new() }; + enumerate(doc).0 +} + +/// How many of [`fields`] were reachable only through the page annotations: the form's `/Fields` +/// list names none or only some of them, so the leniency that adopts them is worth recording. +pub fn adopted_page_fields(doc: &Document) -> usize { + enumerate(doc).1 +} + +fn enumerate(doc: &Document) -> (Vec, usize) { + let af = acroform(doc); let mut page_of = std::collections::HashMap::new(); let mut annot_index = std::collections::HashMap::new(); let pages = page_refs(doc); @@ -500,20 +511,84 @@ pub fn fields(doc: &Document) -> Vec { } } } - let base = Inherited { - da: af.get(b"DA").and_then(|o| text_of(&doc.resolve(o))), - q: af.get(b"Q").and_then(|o| doc.resolve(o).as_int()), - ..Default::default() + let base = match &af { + Some(af) => Inherited { + da: af.get(b"DA").and_then(|o| text_of(&doc.resolve(o))), + q: af.get(b"Q").and_then(|o| doc.resolve(o).as_int()), + ..Default::default() + }, + None => Inherited::default(), }; let mut out = Vec::new(); let mut seen = std::collections::HashSet::new(); - for f in af.get(b"Fields").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default() { - if let Some(r) = f.as_ref() { - walk(doc, r, &base, &page_of, &mut seen, &mut out, 0); + if let Some(af) = &af { + for f in af.get(b"Fields").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default() { + if let Some(r) = f.as_ref() { + walk(doc, r, &base, &page_of, &mut seen, &mut out, 0); + } } } + let listed = out.len(); + adopt_page_fields(doc, &pages, &base, &page_of, &mut seen, &mut out); + let adopted = out.len() - listed; rank_tabs(doc, &pages, &annot_index, &mut out); - out + (out, adopted) +} + +/// Widget annotations no `/Fields` entry reaches are adopted as fields, as Acrobat and the +/// browsers do: some writers list only some of the fields, or none at all. Each widget is adopted +/// through its topmost unlisted `/Parent`, so a field split across several widgets stays one +/// field. Tree-listed fields keep their order first; the rest follow in page-annotation order. +fn adopt_page_fields( + doc: &Document, + pages: &[ObjRef], + base: &Inherited, + page_of: &std::collections::HashMap, + seen: &mut std::collections::HashSet, + out: &mut Vec, +) { + for p in pages { + let Some(a) = doc.get(*p).as_dict().and_then(|d| d.get(b"Annots").cloned()) else { continue }; + let annots = doc.resolve(&a); + let Some(annots) = annots.as_array() else { continue }; + for e in annots { + let Some(r) = e.as_ref() else { continue }; + if seen.contains(&r) { + continue; + } + let obj = doc.get(r); + let Some(d) = obj.as_dict() else { continue }; + if d.name(b"Subtype") != Some(b"Widget") { + continue; + } + if !d.contains(b"FT") && !d.contains(b"T") && !d.contains(b"Parent") { + continue; + } + if let Some(root) = unlisted_root(doc, r, seen) { + walk(doc, root, base, page_of, seen, out, 0); + } + } + } +} + +/// The topmost field above `r` (itself without a `/Parent`), or `None` when `r` or an ancestor is +/// already reachable from the `/Fields` tree, or a `/Parent` loop hides the top. +fn unlisted_root(doc: &Document, r: ObjRef, seen: &std::collections::HashSet) -> Option { + let mut top = r; + let mut visited = std::collections::HashSet::from([r]); + loop { + let parent = doc.get(top).as_dict().and_then(|d| d.reference(b"Parent")); + let Some(p) = parent else { + return if seen.contains(&top) { None } else { Some(top) }; + }; + if !visited.insert(p) || visited.len() > 64 { + return None; + } + if seen.contains(&p) { + return None; + } + top = p; + } } /// A page's tab order (`/Tabs`). diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 94255eab4..ff3247390 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -95,6 +95,52 @@ fn the_field_tree_is_read_with_inheritance() { assert_eq!(field(&all, "address.city").quadding, 1); } +/// Some writers leave every field out of `/Fields` and put the widgets only in the page +/// annotations: the tree walk alone finds nothing, and Acrobat and the browsers fill them anyway. +/// The pages' widgets are adopted as fields, after the listed tree fields. +#[test] +fn fields_listed_only_on_the_pages_are_adopted() { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(), // 1 + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), // 2 + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R 10 0 R 11 0 R 13 0 R 14 0 R 15 0 R] >>".into(), // 3 + "<< /Fields [5 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 6 0 R >> >> >>".into(), // 4 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (listed) /V (keep) /Rect [50 700 250 720] /P 3 0 R >>".into(), // 5 + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(), // 6 + "null".into(), // 7 + "null".into(), // 8 + "null".into(), // 9 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (alpha) /V (one) /Rect [50 600 250 620] /P 3 0 R >>".into(), // 10 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (beta) /DA (/Helv 9 Tf 0 g) /Rect [50 560 250 580] /P 3 0 R >>".into(), // 11 + "<< /FT /Tx /T (pair) /Kids [13 0 R 14 0 R] >>".into(), // 12 unlisted parent field + "<< /Type /Annot /Subtype /Widget /Parent 12 0 R /Rect [50 500 150 520] /P 3 0 R >>".into(), // 13 its widget + "<< /Type /Annot /Subtype /Widget /Parent 12 0 R /Rect [50 460 150 480] /P 3 0 R >>".into(), // 14 its other widget + "<< /Type /Annot /Subtype /Widget /Rect [400 700 415 715] /P 3 0 R >>".into(), // 15 no /FT, /T or /Parent: not a field + ]; + let mut doc = document(&objs); + let all = fields(&doc); + let names: Vec<&str> = all.iter().map(|f| f.name.as_str()).collect(); + assert_eq!(names, ["listed", "alpha", "beta", "pair"]); + assert_eq!(adopted_page_fields(&doc), 3); + let alpha = field(&all, "alpha"); + assert_eq!((alpha.kind, alpha.value.as_slice(), alpha.widgets[0].page), (FieldKind::Text, &["one".to_string()][..], Some(0))); + assert_eq!(field(&all, "beta").da, "/Helv 9 Tf 0 g"); + assert_eq!(field(&all, "pair").widgets.len(), 2, "one field carries both widgets"); + // They are filled and keep the value across a save. + set_value(&mut doc, "beta", &FieldValue::Text("typed".into())).unwrap(); + assert!(ap(&doc, &field(&fields(&doc), "beta").widgets[0]).contains("typed")); + let doc = reopen(&doc); + assert_eq!(field(&fields(&doc), "beta").value, ["typed"]); + // Without a /Fields list at all, everything on the pages is adopted. + let mut objs = objs; + objs[3] = "<< /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv 6 0 R >> >> >>".into(); + let doc = document(&objs); + let all = fields(&doc); + let names: Vec<&str> = all.iter().map(|f| f.name.as_str()).collect(); + assert_eq!(names, ["listed", "alpha", "beta", "pair"]); + assert_eq!(adopted_page_fields(&doc), 4); +} + /// pdf-lib and other writers give radio groups and check boxes an `/Opt` array and name the on /// states by position (`/0`, `/1`): the export values select them, as in Acrobat. #[test] diff --git a/crates/render/src/inspect.rs b/crates/render/src/inspect.rs index bd77a0f61..995ec50cc 100644 --- a/crates/render/src/inspect.rs +++ b/crates/render/src/inspect.rs @@ -476,12 +476,33 @@ impl<'a> Inspector<'a> { info.outline = self.outline_siblings(first, &mut seen, 0); } self.annotations(info); + let mut field_seen = HashSet::new(); if let Some(form) = catalog.get(b"AcroForm").ok().and_then(|o| self.dict(o)) && let Ok(fields) = form.get(b"Fields").and_then(|o| self.resolve(o).as_array()) { - let mut seen = HashSet::new(); for f in fields { - self.field(f, None, &mut info.fields, &mut seen, 0); + self.field(f, None, &mut info.fields, &mut field_seen, 0); + } + } + // Widgets no `/Fields` entry reaches are fields too (Acrobat and the browsers fill them); + // each is adopted through its topmost unlisted `/Parent`, so a split field stays one. + let mut pages: Vec<_> = self.page_index.iter().collect(); + pages.sort_by_key(|(_, i)| **i); + for (&pid, _) in pages { + let Ok(page_dict) = self.doc.get_dictionary(pid) else { continue }; + let Ok(Object::Array(annots)) = page_dict.get(b"Annots").map(|o| self.resolve(o)) else { continue }; + for a in annots { + let Some(d) = self.dict(a) else { continue }; + if self.name(d, b"Subtype").as_deref() != Some("Widget") + || (d.get(b"FT").is_err() && d.get(b"T").is_err() && d.get(b"Parent").is_err()) + { + continue; + } + if let Object::Reference(id) = a + && let Some(root) = self.unlisted_field(*id, &field_seen) + { + self.field(&Object::Reference(root), None, &mut info.fields, &mut field_seen, 0); + } } } info.has_javascript |= info.fields.iter().any(|f| f.has_actions); @@ -1006,6 +1027,31 @@ impl<'a> Inspector<'a> { }) } + /// The topmost field above widget `id` (itself without a `/Parent`), or `None` when it or an + /// ancestor is already reachable from the `/Fields` tree, or a `/Parent` loop hides the top. + fn unlisted_field(&self, id: ObjectId, seen: &HashSet) -> Option { + let mut top = id; + let mut visited = HashSet::from([id]); + loop { + let o = Object::Reference(top); + let d = self.dict(&o)?; + let parent = match d.get(b"Parent") { + Ok(Object::Reference(p)) => Some(*p), + _ => None, + }; + let Some(p) = parent else { + return if seen.contains(&top) { None } else { Some(top) }; + }; + if !visited.insert(p) || visited.len() > 64 { + return None; + } + if seen.contains(&p) { + return None; + } + top = p; + } + } + // ── optional content ──────────────────────────────────────────────────────────────────── fn layers(&self, catalog: &Dictionary, out: &mut Vec) { @@ -1537,6 +1583,23 @@ trailer << /Root 1 0 R >> assert_eq!(info.layer_groups, [vec![(5, 0), (6, 0)], vec![(6, 0), (7, 0)]]); } + const ORPHAN_FIELDS: &[u8] = b"%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >> endobj +2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj +3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Annots [5 0 R 6 0 R] >> endobj +4 0 obj << /Fields [] >> endobj +5 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (alpha) /V (one) /Rect [10 100 90 120] >> endobj +6 0 obj << /Type /Annot /Subtype /Widget /FT /Tx /T (beta) /Rect [10 60 90 80] >> endobj +trailer << /Root 1 0 R >> +%%EOF"; + + #[test] + fn fields_listed_only_on_the_pages_are_listed() { + let info = inspect(Arc::new(ORPHAN_FIELDS.to_vec()), None).expect("opens"); + let listed: Vec<_> = info.fields.iter().map(|f| (f.name.as_str(), f.value.as_deref(), f.page)).collect(); + assert_eq!(listed, [("alpha", Some("one"), Some(0)), ("beta", None, Some(0))], "{:?}", info.fields); + } + #[test] fn lazy_structure_matches_the_compatibility_inspector() { // Each fixture as lopdf writes it, and with its objects in object streams. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index ae6477e79..0f65485ae 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4302,8 +4302,8 @@ milestone = "M3" status = "shipped" commands = ["form.fields"] tools = ["doc_info"] -evidence = ["crates/ui-egui/tests/view.rs::field_list_jumps_to_a_fields_page"] -notes = "read-only list with jump-to-field; filling and editing are separate features (M6)" +evidence = ["crates/ui-egui/tests/view.rs::field_list_jumps_to_a_fields_page", "crates/render/src/inspect.rs::fields_listed_only_on_the_pages_are_listed"] +notes = "read-only list with jump-to-field; filling and editing are separate features (M6); fields a file lists only as page widgets (empty /Fields) are adopted too" [[feature]] id = "form.highlight-fields" @@ -4338,7 +4338,7 @@ milestone = "M6" status = "shipped" commands = ["form.fields"] tools = ["form_fill", "form_fields"] -evidence = ["crates/ui-egui/tests/forms.rs::typing_into_a_text_field_and_tabbing_on", "crates/forms/src/tests.rs::text_fields_get_new_appearances", "crates/engine/src/tests.rs::filling_a_form_shows_saves_and_undoes"] +evidence = ["crates/ui-egui/tests/forms.rs::typing_into_a_text_field_and_tabbing_on", "crates/forms/src/tests.rs::text_fields_get_new_appearances", "crates/engine/src/tests.rs::filling_a_form_shows_saves_and_undoes", "crates/automation/tests/automation.rs::orphan_form_fields_through_tools"] notes = "in-place editor; Enter or clicking away commits, Escape cancels; no format/keystroke JavaScript yet (M6.4)" [[feature]]