diff --git a/crates/forms/src/read.rs b/crates/forms/src/read.rs index 34775ee75e..829c04ad12 100644 --- a/crates/forms/src/read.rs +++ b/crates/forms/src/read.rs @@ -486,7 +486,17 @@ fn walk( let field_kids: Vec = kids.iter().copied().filter(|k| doc.get(*k).as_dict().is_some_and(|kd| kd.contains(b"T"))).collect(); if !field_kids.is_empty() { for k in field_kids { - walk(doc, k, &inh, page_of, seen, out, depth + 1); + // A fully qualified name is built up the field's own `/Parent` chain (ISO 32000-2 + // §12.7.4.2). A kid that doesn't name this field as its `/Parent` is named from its + // own chain, as other readers name it; it still inherits this field's attributes. + let parent = doc.get(k).as_dict().and_then(|kd| kd.reference(b"Parent")); + if parent == Some(r) { + walk(doc, k, &inh, page_of, seen, out, depth + 1); + } else { + let mut own = inh.clone(); + own.name = parent.and_then(|p| qualified_name(doc, p)).unwrap_or_default(); + walk(doc, k, &own, page_of, seen, out, depth + 1); + } } return; } diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 3948e06e83..29062c5314 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2275,6 +2275,61 @@ fn fields_sharing_a_name_take_the_same_value() { } } +/// A radio group `Group1` whose two kids are fields of their own: each has `/T` (`kid_t`) and, +/// with `parent`, a `/Parent` naming the group. +fn radio_kids_with_names(kid_t: [&str; 2], parent: bool) -> Document { + let parent = if parent { " /Parent 4 0 R" } else { "" }; + let kid = |t: &str, state: &str, x: u32| { + format!( + "<< /Type /Annot /Subtype /Widget /FT /Btn /Ff 49152 /T ({t}){parent} /V /Off /AS /Off /Rect [{x} 700 {} 715] /P 3 0 R /AP << /N << /Off 7 0 R /{state} 7 0 R >> >> >>", + x + 15 + ) + }; + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [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 6 0 R] >>".into(), // 3 + "<< /FT /Btn /Ff 49152 /T (Group1) /V /Choice2 /Kids [6 0 R 5 0 R] >>".into(), // 4 + kid(kid_t[0], "Choice1", 50), // 5 + kid(kid_t[1], "Choice2", 100), // 6 + "<< /Length 0 >>\nstream\n\nendstream".into(), // 7 + ]; + document(&objs) +} + +/// A field's fully qualified name is built from its own `/Parent` chain (ISO 32000-2 §12.7.4.2, +/// Table 226): a kid with `/T` that a field's `/Kids` lists but that has no `/Parent` naming +/// that field is named on its own, not under the field that lists it. +#[test] +fn a_named_kid_without_a_parent_keeps_its_own_name() { + let names = |doc: &Document| fields(doc).iter().map(|f| (f.name.clone(), f.obj.num)).collect::>(); + let same = radio_kids_with_names(["Group1", "Group1"], false); + assert_eq!(names(&same), [("Group1".to_string(), 6), ("Group1".to_string(), 5)]); + let other = radio_kids_with_names(["A", "B"], false); + assert_eq!(names(&other), [("B".to_string(), 6), ("A".to_string(), 5)]); + // With /Parent the kids are the group's children, as before. + let nested = radio_kids_with_names(["A", "B"], true); + assert_eq!(names(&nested), [("Group1.B".to_string(), 6), ("Group1.A".to_string(), 5)]); +} + +/// Fields with the same fully qualified name are one field (ISO 32000-2 §12.7.4.2): a radio +/// option that only a later one of them draws can be chosen by the name, and the others turn off. +#[test] +fn a_radio_option_of_any_field_sharing_the_name_can_be_chosen() { + for parent in [false, true] { + let name = if parent { "Group1.Group1" } else { "Group1" }; + let mut doc = radio_kids_with_names(["Group1", "Group1"], parent); + for (choice, on, off) in [("Choice1", 5, 6), ("Choice2", 6, 5), ("Choice1", 5, 6)] { + set_value(&mut doc, name, &FieldValue::Radio(Some(choice.into()))).unwrap_or_else(|e| panic!("{name} = {choice}: {e}")); + doc = reopen(&doc); + let st = |n: u32| doc.get(ObjRef { num: n, generation: 0 }).as_dict().and_then(|d| d.name(b"AS").map(<[u8]>::to_vec)); + assert_eq!((st(on), st(off)), (Some(choice.as_bytes().to_vec()), Some(b"Off".to_vec())), "{name} = {choice}"); + } + let err = set_value(&mut doc, name, &FieldValue::Radio(Some("Choice3".into()))).unwrap_err(); + assert!(err.to_string().contains("Choice3"), "{err}"); + } +} + #[test] fn locked_fields_only_take_unlocking() { let mut doc = one_page(); diff --git a/crates/forms/src/values.rs b/crates/forms/src/values.rs index 9d670e62db..a681287cba 100644 --- a/crates/forms/src/values.rs +++ b/crates/forms/src/values.rs @@ -89,7 +89,19 @@ impl NamedFields { /// Write `value` into the field `name`, without recalculating: `named` are the positions in /// `all` (the form as read before this write) of the fields with that name, in field order. fn fill(doc: &mut Document, all: &[Field], named: &[usize], name: &str, value: &FieldValue, scripts: &mut dyn Scripts) -> Result<(), FormError> { - let f = named.first().and_then(|&i| all.get(i)).ok_or_else(|| FormError::NoSuchField(name.into()))?; + let mut same = named.iter().filter_map(|&i| all.get(i)); + let first = same.clone().next().ok_or_else(|| FormError::NoSuchField(name.into()))?; + // Fields with the same fully qualified name are one field (ISO 32000-2 §12.7.4.2), so a radio + // option any of them draws is an option of the name: it is written through the first one + // that draws it, and the others turn off below. + let choice = match value { + FieldValue::Radio(Some(c)) | FieldValue::Text(c) => Some(c.as_str()), + _ => None, + }; + let f = match choice { + Some(c) if first.kind == FieldKind::Radio => same.find(|t| t.kind == FieldKind::Radio && t.state_for(c).is_some()).unwrap_or(first), + _ => first, + }; if f.read_only() { return Err(FormError::ReadOnly(name.into())); } diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index 603aa88110..d8b756aa23 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -366,8 +366,24 @@ impl<'a> Inspector<'a> { }; let field_kids: Vec<_> = kids.iter().filter(|k| self.dict(k).is_some_and(|kd| kd.get(b"T").is_ok())).collect(); if !field_kids.is_empty() { + let here = match o { + Object::Reference(id) => Some(*id), + _ => None, + }; for k in field_kids { - self.field(k, Some((&full, ft.clone(), ff)), out, seen, depth + 1); + // A fully qualified name is built up the field's own `/Parent` chain (ISO 32000-2 + // §12.7.4.2): a kid that doesn't name this field as its `/Parent` is named from its + // own chain, as the form reader names it. + let parent = match self.dict(k).map(|kd| kd.get(b"Parent")) { + Some(Ok(Object::Reference(p))) => Some(*p), + _ => None, + }; + if here.is_none() || parent == here { + self.field(k, Some((&full, ft.clone(), ff)), out, seen, depth + 1); + } else { + let own = parent.map(|p| self.qualified_name(p)).unwrap_or_default(); + self.field(k, Some((&own, ft.clone(), ff)), out, seen, depth + 1); + } } return; } @@ -426,6 +442,29 @@ impl<'a> Inspector<'a> { pages } + /// The fully qualified name of the field `id`: the partial names (`/T`) up its `/Parent` + /// chain, joined with periods; empty when none has one or the chain reaches an object that + /// isn't a dictionary, as the form reader has it. A `/Parent` loop ends the walk. + fn qualified_name(&self, id: ObjectId) -> String { + let mut parts = Vec::new(); + let mut visited = HashSet::new(); + let mut cur = Some(id); + while let Some(c) = cur { + if visited.len() > 64 || !visited.insert(c) { + break; + } + let o = Object::Reference(c); + let Some(d) = self.dict(&o) else { return String::new() }; + parts.extend(self.text(d, b"T")); + cur = match d.get(b"Parent") { + Ok(Object::Reference(p)) => Some(*p), + _ => None, + }; + } + parts.reverse(); + parts.join(".") + } + /// 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. pub(super) fn unlisted_field(&self, id: ObjectId, seen: &HashSet) -> Option { diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 7221e97d5a..03357a34fe 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1912,3 +1912,26 @@ fn inspection_keeps_no_private_page_data() { }; assert_eq!(report(with), report(without), "what inspection reports does not change"); } + +/// A kid with `/T` that a field's `/Kids` lists but whose `/Parent` doesn't name that field is +/// named up its own `/Parent` chain (ISO 32000-2 §12.7.4.2), as the form reader names it. +#[test] +fn a_named_kid_without_a_parent_keeps_its_own_name() { + let pdf = |parent: &str| { + format!( + "%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [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 << /FT /Btn /Ff 49152 /T (Group1) /Kids [6 0 R 5 0 R] >> endobj +5 0 obj << /Type /Annot /Subtype /Widget /T (Group1){parent} /Rect [10 100 30 120] >> endobj +6 0 obj << /Type /Annot /Subtype /Widget /T (Group1){parent} /Rect [40 100 60 120] >> endobj +trailer << /Root 1 0 R >> +%%EOF" + ) + .into_bytes() + }; + let names = |bytes: Vec| inspect(Arc::new(bytes), None).expect("opens").fields.iter().map(|f| (f.name.clone(), f.kind)).collect::>(); + assert_eq!(names(pdf("")), [("Group1".into(), FieldKind::Radio), ("Group1".into(), FieldKind::Radio)]); + assert_eq!(names(pdf(" /Parent 4 0 R")), [("Group1.Group1".into(), FieldKind::Radio), ("Group1.Group1".into(), FieldKind::Radio)]); +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7582b3f6bc..301b560c11 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4590,8 +4590,8 @@ milestone = "M3" status = "shipped" commands = ["form.fields"] tools = ["doc_info"] -evidence = ["crates/ui-egui/tests/it/view.rs::field_list_jumps_to_a_fields_page", "crates/render/src/inspect/tests.rs::fields_listed_only_on_the_pages_are_listed", "crates/ui-egui/tests/it/bulk_fields.rs::fields_panel_selects_multiple_fields_and_opens_their_shared_properties", "crates/ui-egui/tests/it/bulk_fields.rs::double_clicking_a_field_in_the_panel_opens_its_individual_properties"] -notes = "list with jump-to-field; in Prepare a form, Shift/Ctrl/Command-click selects several fields and Properties edits their shared properties; double-click opens field properties; fields a file lists only as page widgets (empty /Fields) are adopted too" +evidence = ["crates/ui-egui/tests/it/view.rs::field_list_jumps_to_a_fields_page", "crates/render/src/inspect/tests.rs::fields_listed_only_on_the_pages_are_listed", "crates/ui-egui/tests/it/bulk_fields.rs::fields_panel_selects_multiple_fields_and_opens_their_shared_properties", "crates/ui-egui/tests/it/bulk_fields.rs::double_clicking_a_field_in_the_panel_opens_its_individual_properties", "crates/render/src/inspect/tests.rs::a_named_kid_without_a_parent_keeps_its_own_name", "crates/forms/src/tests.rs::a_named_kid_without_a_parent_keeps_its_own_name", "crates/forms/src/tests.rs::a_radio_option_of_any_field_sharing_the_name_can_be_chosen"] +notes = "list with jump-to-field; in Prepare a form, Shift/Ctrl/Command-click selects several fields and Properties edits their shared properties; double-click opens field properties; fields a file lists only as page widgets (empty /Fields) are adopted too; a named kid whose /Parent does not name the field listing it is named up its own /Parent chain; fields sharing a name are listed apart, and a radio option any of them draws is chosen by the name" [[feature]] id = "form.highlight-fields"