From 68dff686fec2019714b06b4bf6ed7b69c47e7070 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sat, 7 Nov 2026 23:54:22 +0800 Subject: [PATCH 1/6] M6: a named kid is named up its own /Parent chain, and a radio option of any same-named field is chosen by the name (form.field-list) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A field's fully qualified name is built from the partial names of the field and its ancestors, and a child names its parent with /Parent (ISO 32000-2 §12.7.4.2, Table 226). A /Kids entry with /T whose /Parent does not name the field listing it is now named up its own /Parent chain, in the form reader and in document inspection, as PDFium and MuPDF name it: pdf.js's issue15096.pdf lists its two radio fields as Group1, not Group1.Group1. Fields with the same fully qualified name are one field (§12.7.4.2), so a radio option that a later one of them draws is now chosen by the name: it is written through the first field that draws it and the others turn off. Before, only the first field's options were accepted. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA --- crates/forms/src/read.rs | 12 +++++- crates/forms/src/tests.rs | 55 ++++++++++++++++++++++++ crates/forms/src/values.rs | 14 +++++- crates/render/src/inspect/annotations.rs | 41 +++++++++++++++++- crates/render/src/inspect/tests.rs | 23 ++++++++++ parity/acrobat-features.toml | 4 +- 6 files changed, 144 insertions(+), 5 deletions(-) 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" From 401a64fd605543f3f7d68665808a2aed7a48db57 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sun, 8 Nov 2026 01:43:01 +0800 Subject: [PATCH 2/6] M6: a radio choice by the name leaves one button on in kids without /P saved both on (form.field-list) Adds the layout issue15096.pdf's last revision has: the two same-named radio kids carry no /P (their page comes from /Annots), and both are drawn on with /V that disagree (Off with /AS /Choice1, Choice2 with /AS /Choice2). Choosing Choice1, Choice2 and Choice1 again by the name leaves exactly the chosen button on after each save and reopen. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA --- crates/forms/src/tests.rs | 37 ++++++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 ++-- 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 29062c5314..2993e43ffa 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2330,6 +2330,43 @@ fn a_radio_option_of_any_field_sharing_the_name_can_be_chosen() { } } +/// The same group as a broken save leaves it: the kids carry no `/P` (their page is found through +/// the page's `/Annots`), and both are drawn on with values that disagree (`/V /Off` with +/// `/AS /Choice1`, `/V /Choice2` with `/AS /Choice2`). Choosing by the name leaves exactly one +/// button on, across a save, whichever option it is. +#[test] +fn a_radio_choice_by_the_name_turns_one_button_on_in_kids_without_a_page_that_disagree() { + let kid = |state: &str, v: &str, x: u32| { + format!( + "<< /Type /Annot /Subtype /Widget /FT /Btn /Ff 49152 /T (Group1) /V /{v} /AS /{state} /Rect [{x} 700 {} 715] /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(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R] >>".into(), + "<< /FT /Btn /Ff 49152 /T (Group1) /V /Choice2 /Kids [6 0 R 5 0 R] >>".into(), + kid("Choice1", "Off", 50), + kid("Choice2", "Choice2", 100), + "<< /Length 0 >>\nstream\n\nendstream".into(), + ]; + let mut doc = document(&objs); + let st = |doc: &Document, n: u32| doc.get(ObjRef { num: n, generation: 0 }).as_dict().and_then(|d| d.name(b"AS").map(<[u8]>::to_vec)); + let all = fields(&doc); + assert_eq!(all.iter().map(|f| f.name.as_str()).collect::>(), ["Group1", "Group1"]); + assert!(all.iter().flat_map(|f| &f.widgets).all(|w| w.page == Some(0)), "found on the page without /P"); + assert_eq!((st(&doc, 5), st(&doc, 6)), (Some(b"Choice1".to_vec()), Some(b"Choice2".to_vec())), "both drawn on as read"); + for (choice, on, off) in [("Choice1", 5, 6), ("Choice2", 6, 5), ("Choice1", 5, 6)] { + set_value(&mut doc, "Group1", &FieldValue::Radio(Some(choice.into()))).unwrap_or_else(|e| panic!("Group1 = {choice}: {e}")); + doc = reopen(&doc); + assert_eq!((st(&doc, on), st(&doc, off)), (Some(choice.as_bytes().to_vec()), Some(b"Off".to_vec())), "Group1 = {choice}"); + let values: Vec> = fields(&doc).into_iter().map(|f| f.value).collect(); + assert!(values.iter().all(|v| v.is_empty() || v == &[choice]), "Group1 = {choice}: {values:?}"); + assert!(values.iter().any(|v| v == &[choice]), "Group1 = {choice}: {values:?}"); + } +} + #[test] fn locked_fields_only_take_unlocking() { let mut doc = one_page(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 301b560c11..c188817ad0 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", "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" +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", "crates/forms/src/tests.rs::a_radio_choice_by_the_name_turns_one_button_on_in_kids_without_a_page_that_disagree"] +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, leaving one button on even when their kids have no /P and were saved both on" [[feature]] id = "form.highlight-fields" From 0ddcc60015d9a3260fc9369b3b0d01386434233d Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sun, 8 Nov 2026 01:52:28 +0800 Subject: [PATCH 3/6] M6: every field sharing a radio group's name takes the chosen /V (form.field-list) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ISO 32000-2 §12.7.4.2: fields with the same fully qualified name have the same value. Choosing a radio option by the name now writes the choice as /V of a same-named twin that draws no button for it (its buttons stay off), and of a field whose /Kids lists them without being their /Parent, so it shares their name (issue15096.pdf's group, whose /V was a stale Choice2). Exactly one button stays drawn on. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA --- crates/forms/src/read.rs | 32 ++++++++++++++++++++++++++++++++ crates/forms/src/tests.rs | 14 +++++++++++--- crates/forms/src/values.rs | 20 ++++++++++++++++++-- parity/acrobat-features.toml | 2 +- 4 files changed, 62 insertions(+), 6 deletions(-) diff --git a/crates/forms/src/read.rs b/crates/forms/src/read.rs index 829c04ad12..345c4d2179 100644 --- a/crates/forms/src/read.rs +++ b/crates/forms/src/read.rs @@ -112,6 +112,38 @@ fn qualified_name(doc: &Document, r: ObjRef) -> Option { (!parts.is_empty()).then(|| parts.join(".")) } +/// The fields of the form's tree whose `/Kids` list one of `kids` and whose own fully qualified +/// name is `name`: a field listing kids that don't name it as their `/Parent`, and so share its +/// name rather than extend it. Each node is visited once, at most 64 levels down. +pub(crate) fn listing_fields_named(doc: &Document, name: &str, kids: &[ObjRef]) -> Vec { + let mut out = Vec::new(); + let Some(af) = acroform(doc) else { return out }; + let top = af.get(b"Fields").map(|f| doc.resolve(f)).and_then(|f| f.as_array().cloned()).unwrap_or_default(); + let mut stack: Vec<(ObjRef, usize)> = top.iter().filter_map(Object::as_ref).map(|r| (r, 0)).collect(); + let mut seen = std::collections::HashSet::new(); + while let Some((r, depth)) = stack.pop() { + if depth > 64 || !seen.insert(r) { + continue; + } + let obj = doc.get(r); + let Some(d) = obj.as_dict() else { continue }; + let list: Vec = d + .get(b"Kids") + .map(|k| doc.resolve(k)) + .and_then(|k| k.as_array().cloned()) + .unwrap_or_default() + .iter() + .filter_map(Object::as_ref) + .collect(); + if list.iter().any(|k| kids.contains(k)) && qualified_name(doc, r).as_deref() == Some(name) { + out.push(r); + } + let next = depth.saturating_add(1); + stack.extend(list.into_iter().filter(|k| doc.get(*k).as_dict().is_some_and(|kd| kd.contains(b"T"))).map(|k| (k, next))); + } + out +} + /// JavaScript streams one reading of the form (its fields, or its document scripts) may decode /// in all, however many actions name the same stream. pub(crate) const MAX_SCRIPT_BYTES: usize = 64 << 20; diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 2993e43ffa..272b2a4243 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2333,7 +2333,8 @@ fn a_radio_option_of_any_field_sharing_the_name_can_be_chosen() { /// The same group as a broken save leaves it: the kids carry no `/P` (their page is found through /// the page's `/Annots`), and both are drawn on with values that disagree (`/V /Off` with /// `/AS /Choice1`, `/V /Choice2` with `/AS /Choice2`). Choosing by the name leaves exactly one -/// button on, across a save, whichever option it is. +/// button on, across a save, whichever option it is, and gives every field of the name the +/// chosen value: both kids and the group listing them. #[test] fn a_radio_choice_by_the_name_turns_one_button_on_in_kids_without_a_page_that_disagree() { let kid = |state: &str, v: &str, x: u32| { @@ -2361,9 +2362,16 @@ fn a_radio_choice_by_the_name_turns_one_button_on_in_kids_without_a_page_that_di set_value(&mut doc, "Group1", &FieldValue::Radio(Some(choice.into()))).unwrap_or_else(|e| panic!("Group1 = {choice}: {e}")); doc = reopen(&doc); assert_eq!((st(&doc, on), st(&doc, off)), (Some(choice.as_bytes().to_vec()), Some(b"Off".to_vec())), "Group1 = {choice}"); + // Every field named Group1 has the same value (ISO 32000-2 §12.7.4.2): both kids and the + // field listing them, whose /V was a stale Choice2. + let v = |n: u32| doc.get(ObjRef { num: n, generation: 0 }).as_dict().and_then(|d| d.name(b"V").map(<[u8]>::to_vec)); + assert_eq!( + [v(4), v(5), v(6)], + [Some(choice.as_bytes().to_vec()), Some(choice.as_bytes().to_vec()), Some(choice.as_bytes().to_vec())], + "Group1 = {choice}" + ); let values: Vec> = fields(&doc).into_iter().map(|f| f.value).collect(); - assert!(values.iter().all(|v| v.is_empty() || v == &[choice]), "Group1 = {choice}: {values:?}"); - assert!(values.iter().any(|v| v == &[choice]), "Group1 = {choice}: {values:?}"); + assert_eq!(values, [vec![choice.to_string()], vec![choice.to_string()]], "Group1 = {choice}"); } } diff --git a/crates/forms/src/values.rs b/crates/forms/src/values.rs index a681287cba..7e42f9bf8d 100644 --- a/crates/forms/src/values.rs +++ b/crates/forms/src/values.rs @@ -1,5 +1,5 @@ use super::appearances::{declares_pdfa, need_appearances}; -use super::read::{acroform, field_flags, fields, page_refs}; +use super::read::{acroform, field_flags, fields, listing_fields_named, page_refs}; use super::{ Dict, Document, Field, FieldEvent, FieldKind, FieldValue, FormError, NoScripts, ObjRef, Object, PdfString, Scripts, Widget, af, appearance, author, calc_order, flags, invalid, name_obj, scripting, @@ -117,6 +117,16 @@ fn fill(doc: &mut Document, all: &[Field], named: &[usize], name: &str, value: & share_value(doc, f.obj, t, &taken, scripts)?; } } + // A field whose `/Kids` lists radio fields that don't name it as their `/Parent` has their + // name too, so it takes the same value (§12.7.4.2) rather than keep a stale choice. + if f.kind == FieldKind::Radio { + let objs: Vec = named.iter().filter_map(|&i| all.get(i)).map(|t| t.obj).collect(); + if let Some(v) = doc.get(f.obj).as_dict().and_then(|d| d.get(b"V").cloned()) { + for p in listing_fields_named(doc, name, &objs) { + doc.update_dict(p, |d| d.set(b"V".to_vec(), v.clone()))?; + } + } + } Ok(()) } @@ -130,7 +140,13 @@ fn share_value(doc: &mut Document, source: ObjRef, twin: &Field, value: &[String } FieldKind::Radio => { let choice = value.first().filter(|c| twin.widgets.iter().any(|w| w.on_state.as_deref() == Some(c.as_str()))); - set_states(doc, twin, choice.map(String::as_str)) + set_states(doc, twin, choice.map(String::as_str))?; + // The same value (§12.7.4.2), even when none of the twin's buttons draws it: they all + // stay off, and only the field that draws the choice shows it. + match value.first() { + Some(v) if choice.is_none() => Ok(doc.update_dict(twin.obj, |d| d.set(b"V".to_vec(), name_obj(v)))?), + _ => Ok(()), + } } FieldKind::Text | FieldKind::Combo | FieldKind::List => { let src = doc.get(source).as_dict().cloned().unwrap_or_default(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index c188817ad0..86a9e1698b 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4591,7 +4591,7 @@ 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", "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", "crates/forms/src/tests.rs::a_radio_choice_by_the_name_turns_one_button_on_in_kids_without_a_page_that_disagree"] -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, leaving one button on even when their kids have no /P and were saved both on" +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, leaving one button on even when their kids have no /P and were saved both on; every field of the name, and a field listing them that shares it, takes the chosen /V" [[feature]] id = "form.highlight-fields" From 916ca60e8001b52edb8978fb2c07110d74d5846b Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sun, 8 Nov 2026 06:33:46 +0800 Subject: [PATCH 4/6] M6: a field merged with a widget reads its widget kids as its buttons, and nameless widgets of an unlisted field are options of its name (form.field-list) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A field dictionary that also carries a zero-area widget and lists widget kids now has the kids as its buttons (ISO 32000-2 §12.7.4: field and widget merge only for a single widget). Widget annotations listed beside named kids, and nameless /Fields entries whose /Parent /Fields doesn't list, are read as buttons of that parent's name. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA --- crates/forms/src/read.rs | 26 +++++++-- crates/forms/src/tests.rs | 101 +++++++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 3 files changed, 124 insertions(+), 7 deletions(-) diff --git a/crates/forms/src/read.rs b/crates/forms/src/read.rs index 345c4d2179..faca4140c3 100644 --- a/crates/forms/src/read.rs +++ b/crates/forms/src/read.rs @@ -392,7 +392,11 @@ pub(crate) fn enumerate(doc: &Document) -> (Vec, usize) { 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); + // A `/Fields` entry that is a nameless kid of a field `/Fields` doesn't list is + // read through that field, which names it (ISO 32000-2 §12.7.4.2). + let nameless_kid = doc.get(r).as_dict().is_some_and(|d| !d.contains(b"T") && d.contains(b"Parent")); + let start = if nameless_kid { unlisted_root(doc, r, &seen.fields).unwrap_or(r) } else { r }; + walk(doc, start, &base, &page_of, &mut seen, &mut out, 0); } } } @@ -512,10 +516,11 @@ fn walk( // An explicit zero also clears an inherited limit rather than inheriting it again. inh.max_len = usize::try_from(m).ok().filter(|limit| *limit > 0); } - let kids: Vec = + let mut kids: Vec = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_default().iter().filter_map(|k| k.as_ref()).collect(); // Kids with /T are fields; kids without are this field's widgets. - let field_kids: Vec = kids.iter().copied().filter(|k| doc.get(*k).as_dict().is_some_and(|kd| kd.contains(b"T"))).collect(); + let (field_kids, widget_kids): (Vec, Vec) = + kids.iter().copied().partition(|k| doc.get(*k).as_dict().is_some_and(|kd| kd.contains(b"T"))); if !field_kids.is_empty() { for k in field_kids { // A fully qualified name is built up the field's own `/Parent` chain (ISO 32000-2 @@ -530,7 +535,13 @@ fn walk( walk(doc, k, &own, page_of, seen, out, depth + 1); } } - return; + // "A field's children in the hierarchy may also include widget annotations" (ISO 32000-2 + // §12.7.4): widget annotations listed beside named kids are still this field's widgets, + // read as a field of this name next to the named kids. + kids = widget_kids.into_iter().filter(|k| doc.get(*k).as_dict().is_some_and(|kd| kd.name(b"Subtype") == Some(b"Widget"))).collect(); + if kids.is_empty() { + return; + } } // A field of no kind this reads is skipped before it claims widgets or scripts, which a later // field listing them then reads. @@ -545,7 +556,12 @@ fn walk( Some(b"Sig") => FieldKind::Signature, _ => return, }; - let widget_refs: Vec = if d.contains(b"Rect") { vec![r] } else { kids }; + // A field's widgets are its widget kids. The field dictionary is its own widget only when it + // has none: field and widget are merged "when a field has only a single associated widget + // annotation" (ISO 32000-2 §12.7.4), so a merged dictionary that also lists widget kids is a + // field whose buttons are those kids, not one more (often zero-area) button. + let has_widget_kids = kids.iter().any(|k| doc.get(*k).as_dict().is_some()); + let widget_refs: Vec = if d.contains(b"Rect") && !has_widget_kids { vec![r] } else { kids }; let widget_refs: Vec = widget_refs.into_iter().filter(|w| seen.widgets.insert(*w)).collect(); let actions = actions_of(doc, d, &widget_refs, &mut seen.scripts); let button = (inh.ft.as_deref() == Some(b"Btn") && inh.ff.unwrap_or(0) & flags::PUSH_BUTTON != 0) diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 272b2a4243..3566e86dfe 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2375,6 +2375,107 @@ fn a_radio_choice_by_the_name_turns_one_button_on_in_kids_without_a_page_that_di } } +/// A radio group whose field dictionary is merged with a zero-area widget and also lists four +/// widget kids, as some form exporters write it: field and widget may be merged only "when a field +/// has only a single associated widget annotation" (ISO 32000-2 §12.7.4), so the kids are the +/// buttons. Two such groups: each lists its four options, and choosing one by the name turns on +/// exactly that button, across a save, and leaves the other group alone. +#[test] +fn a_field_merged_with_a_widget_that_lists_widget_kids_has_the_kids_as_buttons() { + const OPTIONS: [&str; 4] = ["Not#20Satisfied", "Somewhat", "Satisfied", "Very#20Satisfied"]; + let group = |t: &str, first: u32| { + let kids: Vec = (first..first + 4).map(|n| format!("{n} 0 R")).collect(); + format!("<< /Type /Annot /Subtype /Widget /Rect [0 0 0 0] /FT /Btn /Ff 49152 /Kids [{}] /V /Off /T ({t}) >>", kids.join(" ")) + }; + let kid = |parent: u32, i: usize, y: u32| { + let state = OPTIONS.get(i).copied().unwrap_or("X"); + let x = 100 + 100 * u32::try_from(i).unwrap_or(0); + format!( + "<< /Type /Annot /Subtype /Widget /F 4 /FT /Btn /Ff 49152 /Parent {parent} 0 R /Rect [{x} {y} {} {}] /P 3 0 R /AS /Off /AP << /N << /{state} 14 0 R /Off 14 0 R >> >> >>", + x + 12, + y + 12 + ) + }; + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 5 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 [6 0 R 7 0 R 8 0 R 9 0 R 10 0 R 11 0 R 12 0 R 13 0 R] >>".into(), // 3 + group("typeA15[0]", 6), // 4 + group("typeA15[1]", 10), // 5 + ]; + objs.extend((0..4).map(|i| kid(4, i, 600))); // 6-9 + objs.extend((0..4).map(|i| kid(5, i, 560))); // 10-13 + objs.push("<< /Length 0 >>\nstream\n\nendstream".into()); // 14 + let mut doc = document(&objs); + let all = fields(&doc); + assert_eq!(all.iter().map(|f| f.name.as_str()).collect::>(), ["typeA15[0]", "typeA15[1]"]); + let want: Vec = ["Not Satisfied", "Somewhat", "Satisfied", "Very Satisfied"].map(String::from).to_vec(); + for f in &all { + assert_eq!(f.widgets.iter().filter_map(|w| w.on_state.clone()).collect::>(), want, "{}", f.name); + assert!(f.widgets.iter().all(|w| w.page == Some(0) && w.rect[2] > w.rect[0]), "{}: the kids, on the page", f.name); + } + let st = |doc: &Document, n: u32| doc.get(ObjRef { num: n, generation: 0 }).as_dict().and_then(|d| d.name(b"AS").map(<[u8]>::to_vec)); + for (name, choice, on) in [("typeA15[0]", "Satisfied", 8), ("typeA15[1]", "Very Satisfied", 13), ("typeA15[0]", "Not Satisfied", 6)] { + set_value(&mut doc, name, &FieldValue::Radio(Some(choice.into()))).unwrap_or_else(|e| panic!("{name} = {choice}: {e}")); + doc = reopen(&doc); + let first = if name == "typeA15[0]" { 6 } else { 10 }; + for n in first..first + 4 { + let want = if n == on { choice.as_bytes().to_vec() } else { b"Off".to_vec() }; + assert_eq!(st(&doc, n), Some(want), "{name} = {choice}: widget {n}"); + } + } + assert_eq!(st(&doc, 13), Some(b"Very Satisfied".to_vec()), "the other group keeps its choice"); + let values: Vec> = fields(&doc).into_iter().map(|f| f.value).collect(); + assert_eq!(values, [vec!["Not Satisfied".to_string()], vec!["Very Satisfied".to_string()]]); +} + +/// Radio buttons named `2` listed straight in `/Fields`, some fields of their own (`/T (2)`, no +/// `/Parent`) and some nameless widgets of an unlisted field `2` whose `/Kids` mix both: "A +/// field's children in the hierarchy may also include widget annotations" (ISO 32000-2 §12.7.4). +/// Every option is listed under the name `2`, and each, including the nameless widgets', can be +/// chosen by the name, leaving exactly that button on across a save. +#[test] +fn nameless_widgets_of_an_unlisted_field_are_options_of_its_name() { + let named = |state: &str, x: u32| { + format!( + "<< /Type /Annot /Subtype /Widget /FT /Btn /Ff 49152 /T (2) /V /Off /AS /Off /Rect [{x} 700 {} 712] /P 3 0 R /AP << /N << /{state} 9 0 R /Off 9 0 R >> >> >>", + x + 12 + ) + }; + let nameless = |state: &str, x: u32| { + format!( + "<< /Type /Annot /Subtype /Widget /Parent 4 0 R /AS /Off /Rect [{x} 700 {} 712] /P 3 0 R /AP << /N << /{state} 9 0 R /Off 9 0 R >> >> >>", + x + 12 + ) + }; + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 0 R 6 0 R 7 0 R 8 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 7 0 R 8 0 R] >>".into(), // 3 + "<< /FT /Btn /Ff 49152 /T (2) /Kids [5 0 R 6 0 R 7 0 R 8 0 R] >>".into(), // 4 unlisted + named("0", 50), // 5 + named("1", 100), // 6 + nameless("2", 150), // 7 + nameless("6", 200), // 8 + "<< /Length 0 >>\nstream\n\nendstream".into(), // 9 + ]; + let mut doc = document(&objs); + let all = fields(&doc); + assert!(all.iter().all(|f| f.name == "2"), "{:?}", all.iter().map(|f| &f.name).collect::>()); + let mut options: Vec = all.iter().flat_map(|f| &f.widgets).filter_map(|w| w.on_state.clone()).collect(); + options.sort(); + assert_eq!(options, ["0", "1", "2", "6"]); + let st = |doc: &Document, n: u32| doc.get(ObjRef { num: n, generation: 0 }).as_dict().and_then(|d| d.name(b"AS").map(<[u8]>::to_vec)); + for (choice, on) in [("6", 8), ("1", 6), ("2", 7), ("0", 5)] { + set_value(&mut doc, "2", &FieldValue::Radio(Some(choice.into()))).unwrap_or_else(|e| panic!("2 = {choice}: {e}")); + doc = reopen(&doc); + for n in 5..=8 { + let want = if n == on { choice.as_bytes().to_vec() } else { b"Off".to_vec() }; + assert_eq!(st(&doc, n), Some(want), "2 = {choice}: widget {n}"); + } + } +} + #[test] fn locked_fields_only_take_unlocking() { let mut doc = one_page(); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 86a9e1698b..381d2b00e0 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", "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", "crates/forms/src/tests.rs::a_radio_choice_by_the_name_turns_one_button_on_in_kids_without_a_page_that_disagree"] -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, leaving one button on even when their kids have no /P and were saved both on; every field of the name, and a field listing them that shares it, takes the chosen /V" +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", "crates/forms/src/tests.rs::a_radio_choice_by_the_name_turns_one_button_on_in_kids_without_a_page_that_disagree", "crates/forms/src/tests.rs::a_field_merged_with_a_widget_that_lists_widget_kids_has_the_kids_as_buttons", "crates/forms/src/tests.rs::nameless_widgets_of_an_unlisted_field_are_options_of_its_name"] +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, leaving one button on even when their kids have no /P and were saved both on; every field of the name, and a field listing them that shares it, takes the chosen /V; a field dictionary merged with a widget that also lists widget kids has those kids as its buttons; widget annotations listed beside named kids, or listed in /Fields under an unlisted /Parent, are read as buttons of that parent's name" [[feature]] id = "form.highlight-fields" From eee81da38bb47007f15977b38a52607d23bb9b97 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sun, 8 Nov 2026 11:21:59 +0800 Subject: [PATCH 5/6] M6: a radio choice by the name is refused when any radio field of it is read-only, a claimed widget is read once, and inspection lists widgets beside named kids (form.field-list) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - fill: the first same-named field's read-only flag still applies when a later one draws the choice, and no radio field of the name is written when any of them is read-only. - enumerate: a nameless /Fields widget its parent already claimed is not read again as a nameless field; one its parent's /Kids omits is still read on its own. - walk: only kids whose /Subtype is /Widget replace a merged field's own widget. - inspection: a kid whose /Parent is another field is named from that chain even under a direct listing field, and widget annotations beside named kids are listed as a field of the parent's name (ISO 32000-2 §12.7.4), as the form reader lists them. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA --- crates/forms/src/read.rs | 19 ++-- crates/forms/src/tests.rs | 108 +++++++++++++++++++++++ crates/forms/src/values.rs | 5 +- crates/render/src/inspect/annotations.rs | 28 ++++-- crates/render/src/inspect/tests.rs | 48 ++++++++++ 5 files changed, 194 insertions(+), 14 deletions(-) diff --git a/crates/forms/src/read.rs b/crates/forms/src/read.rs index faca4140c3..4137a6f4d0 100644 --- a/crates/forms/src/read.rs +++ b/crates/forms/src/read.rs @@ -393,10 +393,19 @@ pub(crate) fn enumerate(doc: &Document) -> (Vec, usize) { 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() { // A `/Fields` entry that is a nameless kid of a field `/Fields` doesn't list is - // read through that field, which names it (ISO 32000-2 §12.7.4.2). + // read through that field, which names it (ISO 32000-2 §12.7.4.2). A kid a field + // already read has claimed as its widget is not read again as a nameless field; + // one its parent's `/Kids` omits is still read on its own, as before. let nameless_kid = doc.get(r).as_dict().is_some_and(|d| !d.contains(b"T") && d.contains(b"Parent")); - let start = if nameless_kid { unlisted_root(doc, r, &seen.fields).unwrap_or(r) } else { r }; - walk(doc, start, &base, &page_of, &mut seen, &mut out, 0); + if nameless_kid { + if let Some(root) = unlisted_root(doc, r, &seen.fields) { + walk(doc, root, &base, &page_of, &mut seen, &mut out, 0); + } + if seen.widgets.contains(&r) { + continue; + } + } + walk(doc, r, &base, &page_of, &mut seen, &mut out, 0); } } } @@ -557,10 +566,10 @@ fn walk( _ => return, }; // A field's widgets are its widget kids. The field dictionary is its own widget only when it - // has none: field and widget are merged "when a field has only a single associated widget + // has none (no kid whose `/Subtype` is `/Widget`): field and widget are merged "when a field has only a single associated widget // annotation" (ISO 32000-2 §12.7.4), so a merged dictionary that also lists widget kids is a // field whose buttons are those kids, not one more (often zero-area) button. - let has_widget_kids = kids.iter().any(|k| doc.get(*k).as_dict().is_some()); + let has_widget_kids = kids.iter().any(|k| doc.get(*k).as_dict().is_some_and(|kd| kd.name(b"Subtype") == Some(b"Widget"))); let widget_refs: Vec = if d.contains(b"Rect") && !has_widget_kids { vec![r] } else { kids }; let widget_refs: Vec = widget_refs.into_iter().filter(|w| seen.widgets.insert(*w)).collect(); let actions = actions_of(doc, d, &widget_refs, &mut seen.scripts); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 3566e86dfe..cea2845faa 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2476,6 +2476,114 @@ fn nameless_widgets_of_an_unlisted_field_are_options_of_its_name() { } } +/// A kid with `/T` that a field's `/Kids` lists but whose `/Parent` names a different field is +/// named up that field's chain (ISO 32000-2 §12.7.4.2), not under the field listing it. +#[test] +fn a_named_kid_whose_parent_is_another_field_is_named_under_it() { + 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] >>".into(), // 3 + "<< /FT /Tx /T (Listing) /Kids [5 0 R] >>".into(), // 4 + "<< /Type /Annot /Subtype /Widget /T (k) /Parent 6 0 R /Rect [50 700 150 715] /P 3 0 R >>".into(), // 5 + "<< /T (Other) /Kids [5 0 R] >>".into(), // 6 unlisted + ]; + let names: Vec = fields(&document(&objs)).into_iter().map(|f| f.name).collect(); + assert_eq!(names, ["Other.k"]); +} + +/// The two kids of [`radio_kids_with_names`] (`Group1`, no `/Parent`), with the one drawing +/// `Choice2` (6, read first) or the one drawing `Choice1` (5) read-only. +fn radio_kids_one_read_only(read_only: u32) -> Document { + let kid = |n: u32, state: &str, x: u32| { + let ff = if n == read_only { 49153 } else { 49152 }; + format!( + "<< /Type /Annot /Subtype /Widget /FT /Btn /Ff {ff} /T (Group1) /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(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R 6 0 R] >>".into(), + "<< /FT /Btn /Ff 49152 /T (Group1) /Kids [6 0 R 5 0 R] >>".into(), + kid(5, "Choice1", 50), + kid(6, "Choice2", 100), + "<< /Length 0 >> +stream + +endstream" + .into(), + ]; + document(&objs) +} + +/// A radio choice by the name reaches every radio field of that name (ISO 32000-2 §12.7.4.2), so +/// it is refused when any of them is read-only: the first one, even when a later one draws the +/// choice, or a later one, even when the first draws it. Nothing is written. +#[test] +fn a_radio_choice_by_the_name_is_refused_when_a_field_of_the_name_is_read_only() { + for (read_only, choice) in [(6, "Choice1"), (5, "Choice2")] { + let mut doc = radio_kids_one_read_only(read_only); + let state = |doc: &Document| { + [4, 5, 6].map(|n| { + doc.get(ObjRef { num: n, generation: 0 }).as_dict().map(|d| (d.name(b"V").map(<[u8]>::to_vec), d.name(b"AS").map(<[u8]>::to_vec))) + }) + }; + let before = state(&doc); + let err = set_value(&mut doc, "Group1", &FieldValue::Radio(Some(choice.into()))).unwrap_err(); + assert!(matches!(err, FormError::ReadOnly(_)), "{read_only} read-only, {choice}: {err}"); + assert_eq!(state(&doc), before, "{read_only} read-only, {choice}: nothing written"); + } +} + +/// A nameless widget listed in `/Fields` that the field before it already claimed is not read a +/// second time as a nameless field with no buttons. +#[test] +fn a_listed_widget_its_parent_claimed_is_read_once() { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 5 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] >>".into(), // 3 + "<< /FT /Tx /T (p) /Kids [5 0 R] >>".into(), // 4 + "<< /Type /Annot /Subtype /Widget /FT /Tx /Parent 4 0 R /Rect [50 700 150 715] /P 3 0 R >>".into(), // 5 + ]; + let all = fields(&document(&objs)); + assert_eq!(all.iter().map(|f| (f.name.as_str(), f.widgets.len())).collect::>(), [("p", 1)]); +} + +/// A nameless widget listed in `/Fields` whose `/Parent` doesn't list it in its `/Kids` is still +/// read, as it was before such widgets were read through their parent. +#[test] +fn a_listed_widget_its_parent_omits_is_still_read() { + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [5 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] >>".into(), // 3 + "<< /FT /Tx /T (p) /Kids [] >>".into(), // 4 unlisted + "<< /Type /Annot /Subtype /Widget /FT /Tx /Parent 4 0 R /Rect [50 700 150 715] /P 3 0 R >>".into(), // 5 + ]; + let all = fields(&document(&objs)); + let widgets: Vec = all.iter().flat_map(|f| &f.widgets).map(|w| w.obj.num).collect(); + assert_eq!(widgets, [5]); +} + +/// A field merged with its widget whose `/Kids` holds a dictionary that isn't a widget keeps +/// itself as its one widget: only widget kids replace it (ISO 32000-2 §12.7.4). +#[test] +fn a_merged_field_with_a_kid_that_is_not_a_widget_keeps_itself_as_widget() { + 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 [4 0 R] >>".into(), // 3 + "<< /Type /Annot /Subtype /Widget /FT /Tx /T (t) /Rect [50 700 150 715] /P 3 0 R /Kids [5 0 R] >>".into(), // 4 + "<< /Foo 1 >>".into(), // 5 + ]; + let all = fields(&document(&objs)); + let widgets: Vec<(u32, [f64; 4])> = all.iter().flat_map(|f| &f.widgets).map(|w| (w.obj.num, w.rect)).collect(); + assert_eq!(widgets, [(4, [50.0, 700.0, 150.0, 715.0])]); +} + #[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 7e42f9bf8d..94c637fd52 100644 --- a/crates/forms/src/values.rs +++ b/crates/forms/src/values.rs @@ -102,7 +102,10 @@ fn fill(doc: &mut Document, all: &[Field], named: &[usize], name: &str, value: & 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() { + // The name is read-only when the first field of it is, whichever field draws the choice; and + // a radio choice reaches every radio field of the name, so none of them may be read-only. + let radio_locked = f.kind == FieldKind::Radio && named.iter().filter_map(|&i| all.get(i)).any(|t| t.kind == FieldKind::Radio && t.read_only()); + if f.read_only() || first.read_only() || radio_locked { return Err(FormError::ReadOnly(name.into())); } write_value(doc, f, value, scripts)?; diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index d8b756aa23..6004e43c2a 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -360,9 +360,9 @@ impl<'a> Inspector<'a> { let ft = self.name(d, b"FT").or(parent_ft); let ff = d.get(b"Ff").ok().and_then(|x| x.as_i64().ok()).or(parent_ff); // Non-terminal: has kids that are fields (they carry /T). Widgets-only kids are terminal. - let kids = match d.get(b"Kids").map(|k| self.resolve(k)) { - Ok(Object::Array(k)) => k.as_slice(), - _ => &[], + let mut kids: Vec = match d.get(b"Kids").map(|k| self.resolve(k)) { + Ok(Object::Array(k)) => k.clone(), + _ => Vec::new(), }; 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() { @@ -378,14 +378,20 @@ impl<'a> Inspector<'a> { Some(Ok(Object::Reference(p))) => Some(*p), _ => None, }; - if here.is_none() || parent == here { + if 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; + // "A field's children in the hierarchy may also include widget annotations" (ISO + // 32000-2 §12.7.4): widget annotations listed beside named kids are still this + // field's widgets, listed as a field of this name, as the form reader reads them. + kids.retain(|k| self.dict(k).is_some_and(|kd| kd.get(b"T").is_err() && self.name(kd, b"Subtype").as_deref() == Some("Widget"))); + if kids.is_empty() { + return; + } } let flags = ff.unwrap_or(0); let kind = match ft.as_deref() { @@ -413,9 +419,15 @@ impl<'a> Inspector<'a> { Ok(Object::Dictionary(_)) if kind == FieldKind::Signature => Some("signed".into()), _ => None, }; - // The widget is either this dict (merged field/widget) or its first kid. - let widget = if d.get(b"Rect").is_ok() { Some(d) } else { kids.first().and_then(|k| self.dict(k)) }; - let page = widget.and_then(|w| w.get(b"P").ok()).and_then(|p| self.page_of(p)).or_else(|| self.find_widget_page(o, kids)); + // The widget is this dict (merged field/widget) or its first kid. A merged dict that also + // lists widget kids has them as its widgets (ISO 32000-2 §12.7.4), as the form reader has it. + let widget_kid = kids.iter().filter_map(|k| self.dict(k)).find(|kd| self.name(kd, b"Subtype").as_deref() == Some("Widget")); + let widget = match widget_kid { + Some(w) => Some(w), + None if d.get(b"Rect").is_ok() => Some(d), + None => kids.first().and_then(|k| self.dict(k)), + }; + let page = widget.and_then(|w| w.get(b"P").ok()).and_then(|p| self.page_of(p)).or_else(|| self.find_widget_page(o, &kids)); let has_actions = d.get(b"AA").is_ok() || widget.is_some_and(|w| w.get(b"AA").is_ok() || w.get(b"A").is_ok()); let rect = widget.and_then(|w| w.get(b"Rect").ok()).map(|r| rect4(self.resolve(r))); out.push(Field { name: full, kind, value, page, tooltip: self.text(d, b"TU"), has_actions, rect }); diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 03357a34fe..4088eec98b 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1935,3 +1935,51 @@ trailer << /Root 1 0 R >> 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)]); } + +/// A kid named from its own `/Parent` chain (ISO 32000-2 §12.7.4.2) when that `/Parent` is a field +/// other than the one listing it: an indirect one, and one listed as a direct dictionary in +/// `/Fields` (which no `/Parent` can name). +#[test] +fn a_named_kid_whose_parent_is_another_field_is_named_under_it() { + let pdf = |listing: &str, fields: &str| { + format!( + "%PDF-1.7 +1 0 obj << /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [{fields}] >> >> 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] >> endobj +4 0 obj {listing} endobj +5 0 obj << /Type /Annot /Subtype /Widget /T (k) /Parent 6 0 R /Rect [10 100 30 120] >> endobj +6 0 obj << /T (Other) /Kids [5 0 R] >> 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()).collect::>(); + let listing = "<< /FT /Tx /T (Listing) /Kids [5 0 R] >>"; + assert_eq!(names(pdf(listing, "4 0 R")), ["Other.k"]); + assert_eq!(names(pdf("null", listing)), ["Other.k"], "listed as a direct dictionary"); +} + +/// "A field's children in the hierarchy may also include widget annotations" (ISO 32000-2 +/// §12.7.4): a nameless widget beside a named kid is listed as a field of the parent's name, at +/// the widget's place, as the form reader lists it. +#[test] +fn a_widget_beside_a_named_kid_is_a_field_of_the_parents_name() { + let pdf = b"%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 (2) /Kids [5 0 R 6 0 R] >> endobj +5 0 obj << /Type /Annot /Subtype /Widget /T (2) /Rect [10 100 30 120] >> endobj +6 0 obj << /Type /Annot /Subtype /Widget /Parent 4 0 R /Rect [40 100 60 120] >> endobj +trailer << /Root 1 0 R >> +%%EOF" + .to_vec(); + let info = inspect(Arc::new(pdf), None).expect("opens"); + let got: Vec<_> = info.fields.iter().map(|f| (f.name.clone(), f.kind, f.rect)).collect(); + assert_eq!( + got, + [("2".into(), FieldKind::Radio, Some([10.0, 100.0, 30.0, 120.0])), ("2".into(), FieldKind::Radio, Some([40.0, 100.0, 60.0, 120.0]))] + ); +} From 8bb2d920a0a330b1d2b48fb88ab286487448f128 Mon Sep 17 00:00:00 2001 From: mtlcl410 Date: Sun, 8 Nov 2026 12:49:40 +0800 Subject: [PATCH 6/6] M6: a /Parent given through an object that is only a reference is followed in the field name (form.field-list) qualified_name reads each ancestor through Document::resolve, which follows an object that is nothing but a reference to another, as document inspection does, so both name the kid the same. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA --- crates/forms/src/read.rs | 3 ++- crates/forms/src/tests.rs | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/forms/src/read.rs b/crates/forms/src/read.rs index 4137a6f4d0..90472ffdf7 100644 --- a/crates/forms/src/read.rs +++ b/crates/forms/src/read.rs @@ -101,7 +101,8 @@ fn qualified_name(doc: &Document, r: ObjRef) -> Option { if seen.len() > 64 || !seen.insert(c) { break; } - let obj = doc.get(c); + // An object that is only a reference to another is followed, as `Document::resolve` does. + let obj = doc.resolve(&Object::Ref(c)); let d = obj.as_dict()?; if let Some(t) = d.get(b"T").and_then(|o| text_of(&doc.resolve(o))) { parts.push(t); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index cea2845faa..ae47d72d4a 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2492,6 +2492,23 @@ fn a_named_kid_whose_parent_is_another_field_is_named_under_it() { assert_eq!(names, ["Other.k"]); } +/// A `/Parent` naming an object that is only a reference to the parent field is followed to it, +/// as document inspection follows it, so both name the kid the same. +#[test] +fn a_parent_given_through_an_object_that_is_a_reference_names_the_kid() { + 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] >>".into(), // 3 + "<< /FT /Tx /T (Listing) /Kids [5 0 R] >>".into(), // 4 + "<< /Type /Annot /Subtype /Widget /T (k) /Parent 6 0 R /Rect [50 700 150 715] /P 3 0 R >>".into(), // 5 + "7 0 R".into(), // 6 + "<< /T (Other) /Kids [5 0 R] >>".into(), // 7 + ]; + let names: Vec = fields(&document(&objs)).into_iter().map(|f| f.name).collect(); + assert_eq!(names, ["Other.k"]); +} + /// The two kids of [`radio_kids_with_names`] (`Group1`, no `/Parent`), with the one drawing /// `Choice2` (6, read first) or the one drawing `Choice1` (5) read-only. fn radio_kids_one_read_only(read_only: u32) -> Document {