From ca413b7e9f35145c1daf384dcffed196e1213247 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Thu, 29 Oct 2026 05:16:21 +0800 Subject: [PATCH 1/4] M12: XFA field names made up for repeats never take a name the template uses A repeated field gets a made-up PDF field name (`Cell4` -> `Cell4_2`). That name could be one another template field already has as written, so two different fields shared one PDF name and, per ISO 32000-2 12.7.4.2, one value. The layout now reserves the template's own names and skips them when making one up. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CBVyQEwtmEw3mS1MoAiwrK --- crates/xfa/src/layout.rs | 11 ++++++++++- crates/xfa/src/layout/common.rs | 35 ++++++++++++++++++++++++++++++++- crates/xfa/src/layout/draw.rs | 18 ++++++++++++----- crates/xfa/src/tests.rs | 28 ++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 7 deletions(-) diff --git a/crates/xfa/src/layout.rs b/crates/xfa/src/layout.rs index 73c6bde90f..076ebce118 100644 --- a/crates/xfa/src/layout.rs +++ b/crates/xfa/src/layout.rs @@ -28,9 +28,9 @@ mod items; mod measure; mod pages; -use common::collect_values; pub(crate) use common::date_pattern; pub use common::formatted_value; +use common::{collect_names, collect_values}; pub(crate) use items::UNDERLINE_DROP; pub use items::{Action, BorderShape, Form, Item, Page, Rounded, Widget, WidgetBorder, WidgetKind}; @@ -136,6 +136,11 @@ struct Layouter<'t> { pages: Vec, cur: Option, names: HashMap, + /// Every PDF field name handed out so far. + taken: HashSet, + /// The names the template's fields and groups take as written ([`common::collect_names`]), + /// which a made-up `_2`, `_3` name must not take. + template_names: HashSet, /// Field values by `id`, for embedded fields in rich text. values: HashMap, warnings: Vec, @@ -183,6 +188,8 @@ pub fn layout(tpl: &Template, data: Option<&DataNode>) -> Result // One copy of the root for the whole layout: measurements are cached by node address. let root_node = Node::Subform(Box::new(tpl.root.clone())); collect_values(std::slice::from_ref(&root_node), &mut values, 0); + let mut template_names = HashSet::new(); + collect_names(std::slice::from_ref(&root_node), &mut template_names, 0); let mut l = Layouter { tpl, data, @@ -191,6 +198,8 @@ pub fn layout(tpl: &Template, data: Option<&DataNode>) -> Result pages: Vec::new(), cur: None, names: HashMap::new(), + taken: HashSet::new(), + template_names, values, warnings: Vec::new(), warned: HashSet::new(), diff --git a/crates/xfa/src/layout/common.rs b/crates/xfa/src/layout/common.rs index 625f305a6b..de855219e3 100644 --- a/crates/xfa/src/layout/common.rs +++ b/crates/xfa/src/layout/common.rs @@ -1,5 +1,5 @@ use super::items::{Action, Item, Rounded}; -use super::{HashMap, Layout, MAX_DEPTH, Node, Rect, Script, date_letters, iso_to_pattern}; +use super::{HashMap, HashSet, Layout, MAX_DEPTH, Node, Rect, Script, date_letters, iso_to_pattern}; pub(super) fn collect_values(nodes: &[Node], out: &mut HashMap, depth: usize) { if depth > MAX_DEPTH { @@ -26,6 +26,39 @@ pub(super) fn collect_values(nodes: &[Node], out: &mut HashMap, } } +/// The PDF field names the template's fields and exclusion groups take as written +/// ([`clean_name`]): a name the layout makes up for a repeated field (`Cell4` → `Cell4_2`) must +/// not be one of these, or two PDF fields would share it. +pub(super) fn collect_names(nodes: &[Node], out: &mut HashSet, depth: usize) { + if depth > MAX_DEPTH { + return; + } + for n in nodes { + match n { + Node::Field(f) => { + if let Some(name) = &f.common.name { + out.insert(clean_name(name)); + } + } + Node::ExclGroup(g) => { + if let Some(name) = &g.common.name { + out.insert(clean_name(name)); + } + } + Node::Subform(s) => { + collect_names(&s.children, out, depth + 1); + if let Some(ps) = &s.page_set { + for a in &ps.areas { + collect_names(&a.items, out, depth + 1); + } + } + } + Node::Area(a) => collect_names(&a.children, out, depth + 1), + Node::Draw(_) => {} + } + } +} + pub(super) fn clamp_opt(v: f64, min: Option, max: Option) -> f64 { let mut v = v; if let Some(m) = min { diff --git a/crates/xfa/src/layout/draw.rs b/crates/xfa/src/layout/draw.rs index f6520c1628..b71e5114e8 100644 --- a/crates/xfa/src/layout/draw.rs +++ b/crates/xfa/src/layout/draw.rs @@ -316,12 +316,20 @@ impl Layouter<'_> { self.push(Item::Image { rect: fitted, data: Arc::new(data.to_vec()), content_type: content_type.to_string() }) } - /// A field name made from `base` that no earlier field of the form has: the first keeps it, - /// later ones get `_2`, `_3`, …. + /// A field name made from `base` that no other field of the form has: the first keeps it, + /// later ones get `_2`, `_3`, …, skipping a name another field already has or that the + /// template gives another field (a template field named `Cell4_2` keeps it, and the second + /// `Cell4` becomes `Cell4_3`). pub(super) fn unique_name(&mut self, base: &str) -> String { let base = clean_name(base); - let n = self.names.entry(base.clone()).or_insert(0); - *n += 1; - if *n == 1 { base } else { format!("{base}_{n}") } + loop { + let n = self.names.entry(base.clone()).or_insert(0); + *n = n.saturating_add(1); + let candidate = if *n == 1 { base.clone() } else { format!("{base}_{n}") }; + let reserved = *n > 1 && self.template_names.contains(&candidate); + if !reserved && self.taken.insert(candidate.clone()) { + return candidate; + } + } } } diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index ca129abceb..e6b562290f 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4089,3 +4089,31 @@ fn clear_password_values_clears_static_xfa_password_fields() { let packets = read_packets(&doc).unwrap().unwrap(); assert!(!packets.xdp.contains("hunter2"), "{}", packets.xdp); } + +/// Two PDF fields never share a name: a made-up `_2`, `_3` name skips one the template gives +/// another field. Whichever comes first in layout order, the field the template names `Cell4_2` +/// keeps that name and the second `Cell4` takes the next free one. Before, the second `Cell4` +/// became `Cell4_2` as well, so a script writing by that name hit the wrong cell. +#[test] +fn a_made_up_field_name_never_takes_one_the_template_uses() { + for rows in [["Cell4", "Cell4_2", "Cell4"], ["Cell4", "Cell4", "Cell4_2"]] { + let body: String = rows + .iter() + .enumerate() + .map(|(i, name)| { + format!(r#""#) + }) + .collect(); + let form = layout_xml(&one_area_form(r#"layout="tb""#, &body)).unwrap(); + let all = widgets(&form.pages[0]); + let names: Vec<&str> = all.iter().map(|w| w.name.as_str()).collect(); + let mut unique = names.clone(); + unique.sort_unstable(); + unique.dedup(); + assert_eq!(unique.len(), names.len(), "{rows:?}: {names:?}"); + let template_named = rows.iter().position(|n| *n == "Cell4_2").unwrap(); + let som = format!("form[0].row{template_named}[0].Cell4_2[0]"); + let got = all.iter().find(|w| w.som == som).map(|w| w.name.as_str()); + assert_eq!(got, Some("Cell4_2"), "{rows:?}: the template's own name is kept ({names:?})"); + } +} From a03446e8dd6f8788e0cd083145dc7ea18aa8ae0a Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Thu, 29 Oct 2026 07:21:22 +0800 Subject: [PATCH 2/4] M12: an unnamed XFA field's made-up name never takes one the template uses An unnamed field or group is named `field` or `group`. When it came before a field the template names `field`, it took that name and the named field became `field_2`. A fallback name now skips the template's own names too, and a field keeps the name the template gives it while no other field has it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01CBVyQEwtmEw3mS1MoAiwrK --- crates/xfa/src/layout/draw.rs | 14 ++++++++++---- crates/xfa/src/layout/field.rs | 2 +- crates/xfa/src/tests.rs | 15 +++++++++++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/crates/xfa/src/layout/draw.rs b/crates/xfa/src/layout/draw.rs index b71e5114e8..e3ecc41605 100644 --- a/crates/xfa/src/layout/draw.rs +++ b/crates/xfa/src/layout/draw.rs @@ -259,7 +259,7 @@ impl Layouter<'_> { let selected: Option = self.data.and_then(|d| d.text_at(&data_path(self.tpl, &ctx.som))).map(|t| t.trim().to_string()).filter(|t| !t.is_empty()); let inner = rect.inset(&g.common.margin); - let group = self.unique_name(g.common.name.as_deref().unwrap_or("group")); + let group = self.unique_name(g.common.name.as_deref().unwrap_or("group"), g.common.name.is_none()); let nodes: Vec = g.fields.iter().map(|f| Node::Field(Box::new(f.clone()))).collect(); let placed: Vec<(usize, Rect)> = match g.layout { Layout::Tb => { @@ -319,14 +319,20 @@ impl Layouter<'_> { /// A field name made from `base` that no other field of the form has: the first keeps it, /// later ones get `_2`, `_3`, …, skipping a name another field already has or that the /// template gives another field (a template field named `Cell4_2` keeps it, and the second - /// `Cell4` becomes `Cell4_3`). - pub(super) fn unique_name(&mut self, base: &str) -> String { + /// `Cell4` becomes `Cell4_3`). A `fallback` base, the name an unnamed field or group gets + /// (`field`, `group`), is made up too: it doesn't take a name the template gives another + /// field either, and a template field named `field` still gets it. + pub(super) fn unique_name(&mut self, base: &str, fallback: bool) -> String { let base = clean_name(base); + // The template's own name, while no other field has it. + if !fallback && self.taken.insert(base.clone()) { + return base; + } loop { let n = self.names.entry(base.clone()).or_insert(0); *n = n.saturating_add(1); let candidate = if *n == 1 { base.clone() } else { format!("{base}_{n}") }; - let reserved = *n > 1 && self.template_names.contains(&candidate); + let reserved = (fallback || *n > 1) && self.template_names.contains(&candidate); if !reserved && self.taken.insert(candidate.clone()) { return candidate; } diff --git a/crates/xfa/src/layout/field.rs b/crates/xfa/src/layout/field.rs index cc15f16788..e046ece482 100644 --- a/crates/xfa/src/layout/field.rs +++ b/crates/xfa/src/layout/field.rs @@ -152,7 +152,7 @@ impl Layouter<'_> { (Ui::Button, Some(cap)) => ctx.para.apply(&cap.para).h_align, _ => fctx.para.h_align, }; - let name = self.unique_name(f.common.name.as_deref().unwrap_or("field")); + let name = self.unique_name(f.common.name.as_deref().unwrap_or("field"), f.common.name.is_none()); let som = som_path(); let read_only = matches!(f.access, Access::ReadOnly | Access::Protected); // The data's value wins over the template's default. diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index e6b562290f..af33e46b12 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4117,3 +4117,18 @@ fn a_made_up_field_name_never_takes_one_the_template_uses() { assert_eq!(got, Some("Cell4_2"), "{rows:?}: the template's own name is kept ({names:?})"); } } + +/// An unnamed field's made-up name (`field`) doesn't take the name a later field has in the +/// template: the field the template names `field` keeps it, and the unnamed one before it takes +/// the next free name. +#[test] +fn an_unnamed_field_never_takes_a_name_the_template_uses() { + let body = r#" +"#; + let form = layout_xml(&one_area_form(r#"layout="tb""#, body)).unwrap(); + let all = widgets(&form.pages[0]); + // In layout order: the unnamed field, then the one the template names `field`. + let names: Vec<&str> = all.iter().map(|w| w.name.as_str()).collect(); + assert_eq!(names.get(1), Some(&"field"), "{names:?}"); + assert_ne!(names.first(), Some(&"field"), "{names:?}"); +} From 1defb91dbe7de1256ed739e733e6073a1a189e29 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:21:39 +0800 Subject: [PATCH 3/4] M12: test that a form without repeated fields keeps every template name (#5225 review) A form whose fields don't repeat keeps each template name as it is, one that looks made up (Total_2) included: reserving the template's names must not push a field off its own. Seen to fail with the template's names reserved for every field (Name_2, Total_3, Total_2_2, Date_2). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/xfa/src/tests.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index af33e46b12..ac619622f2 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4118,6 +4118,24 @@ fn a_made_up_field_name_never_takes_one_the_template_uses() { } } +/// #5225 review: a form whose fields don't repeat keeps every template name as it is, a name +/// that looks like a made-up one (`Total_2`) included, in every subform. Reserving the template's +/// names must not push a field off its own. +#[test] +fn a_form_without_repeats_keeps_every_template_name() { + let names = ["Name", "Total", "Total_2", "Date"]; + let body: String = names + .iter() + .enumerate() + .map(|(i, name)| { + format!(r#""#) + }) + .collect(); + let form = layout_xml(&one_area_form(r#"layout="tb""#, &body)).unwrap(); + let got: Vec = widgets(&form.pages[0]).iter().map(|w| w.name.clone()).collect(); + assert_eq!(got, names, "the template's names, unchanged"); +} + /// An unnamed field's made-up name (`field`) doesn't take the name a later field has in the /// template: the field the template names `field` keeps it, and the unnamed one before it takes /// the next free name. From 99bfb799944f2c7bde040caad543d823792079d5 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 13:08:37 +0800 Subject: [PATCH 4/4] M12: an exclusion group's member names are reserved too (#5225 review) collect_names recorded an exclusion group's name but not its members'. The members are drawn as fields with their own names (draw_field), so a repeated field drawn before the group could take a member's name as its made-up one (a repeated Cell4 took Cell4_2, and the member Cell4_2 became Cell4_2_2); the result depended on draw order. The members' names are now reserved as well, so a member keeps the name the template gives it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/xfa/src/layout.rs | 4 ++-- crates/xfa/src/layout/common.rs | 14 +++++++++++--- crates/xfa/src/tests.rs | 20 ++++++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/crates/xfa/src/layout.rs b/crates/xfa/src/layout.rs index 076ebce118..bfe61fc0f5 100644 --- a/crates/xfa/src/layout.rs +++ b/crates/xfa/src/layout.rs @@ -138,8 +138,8 @@ struct Layouter<'t> { names: HashMap, /// Every PDF field name handed out so far. taken: HashSet, - /// The names the template's fields and groups take as written ([`common::collect_names`]), - /// which a made-up `_2`, `_3` name must not take. + /// The names the template's fields, groups and groups' members take as written + /// ([`common::collect_names`]), which a made-up `_2`, `_3` name must not take. template_names: HashSet, /// Field values by `id`, for embedded fields in rich text. values: HashMap, diff --git a/crates/xfa/src/layout/common.rs b/crates/xfa/src/layout/common.rs index de855219e3..59948010e8 100644 --- a/crates/xfa/src/layout/common.rs +++ b/crates/xfa/src/layout/common.rs @@ -26,9 +26,10 @@ pub(super) fn collect_values(nodes: &[Node], out: &mut HashMap, } } -/// The PDF field names the template's fields and exclusion groups take as written -/// ([`clean_name`]): a name the layout makes up for a repeated field (`Cell4` → `Cell4_2`) must -/// not be one of these, or two PDF fields would share it. +/// The names the template's fields, exclusion groups and the groups' member fields take as +/// written ([`clean_name`]): a name the layout makes up for a repeated field (`Cell4` → +/// `Cell4_2`) must not be one of these, or two fields would share it. (A group's radio buttons +/// are the group's PDF field; a member that isn't one is a PDF field with its own name.) pub(super) fn collect_names(nodes: &[Node], out: &mut HashSet, depth: usize) { if depth > MAX_DEPTH { return; @@ -44,6 +45,13 @@ pub(super) fn collect_names(nodes: &[Node], out: &mut HashSet, depth: us if let Some(name) = &g.common.name { out.insert(clean_name(name)); } + // Its members are drawn as fields with their own names (`draw_field`), so a + // repeat drawn before the group must not take one. + for f in &g.fields { + if let Some(name) = &f.common.name { + out.insert(clean_name(name)); + } + } } Node::Subform(s) => { collect_names(&s.children, out, depth + 1); diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index ac619622f2..5ed3c18c84 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4118,6 +4118,26 @@ fn a_made_up_field_name_never_takes_one_the_template_uses() { } } +/// #5225 review: a member of an exclusion group keeps the name the template gives it, whatever +/// is drawn before it: a repeated field drawn first doesn't take it as its made-up name. +#[test] +fn a_made_up_field_name_never_takes_an_exclusion_group_members_name() { + let field = |name: &str| format!(r#""#); + let group = r#"AB"#; + // The repeated field first, then the group; and the other way round. + for body in [format!("{}{}{group}", field("Cell4"), field("Cell4")), format!("{group}{}{}", field("Cell4"), field("Cell4"))] { + let form = layout_xml(&one_area_form(r#"layout="tb""#, &body)).unwrap(); + let all = widgets(&form.pages[0]); + let shown: Vec<(String, String)> = all.iter().map(|w| (w.som.clone(), w.name.clone())).collect(); + let member = all.iter().find(|w| w.som.ends_with(".Choice[0].Cell4_2[0]")).map(|w| w.name.as_str()); + assert_eq!(member, Some("Cell4_2"), "the member keeps its name: {shown:?}"); + let mut names: Vec<&str> = all.iter().map(|w| w.name.as_str()).collect(); + names.sort_unstable(); + names.dedup(); + assert_eq!(names.len(), all.len(), "no two widgets share a name: {shown:?}"); + } +} + /// #5225 review: a form whose fields don't repeat keeps every template name as it is, a name /// that looks like a made-up one (`Total_2`) included, in every subform. Reserving the template's /// names must not push a field off its own.