From ade6b150a3be62223ce9bed21c6aff081a2d18c5 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 15:58:23 +0800 Subject: [PATCH] M12: an XFA reset restores groups, saved list values and structured picks, in bounded time The reset of objects an opening script added before they have widgets missed three kinds of value, and its name matching grew with the product of objects and names (review): - Exclusion groups were skipped, so a radio value set and reset in a new row stayed in the data. A group now resets to the on value of its default button, or to nothing. - A choice list reset to the text it shows, not the item value it saves. The default now goes through Field::saved_value, as the live form reads it. - A multi-select list whose picks the data holds as elements compared as empty and could not be written, so its picks survived. A new DataOp::Reset compares and replaces those picks (none for an empty default); DataOp::Value still leaves nodes with children alone. - Reset names are indexed (ResetTargets), so matching an object costs the same however many names a script passes, and forms::reset looks named fields up through sets instead of scanning the list per field. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/engine/src/tests.rs | 56 +++++++++++++++ crates/engine/src/xfa.rs | 116 +++++++++++++++++++++++-------- crates/forms/src/tests.rs | 20 ++++++ crates/forms/src/values.rs | 7 +- crates/xfa/src/data.rs | 34 ++++++++- crates/xfa/src/data/import.rs | 2 +- crates/xfa/src/data/merge.rs | 27 +++++++ crates/xfa/src/data/passwords.rs | 2 +- crates/xfa/src/data/path.rs | 15 ++-- crates/xfa/src/data/values.rs | 5 +- crates/xfa/src/data/write.rs | 7 +- crates/xfa/src/script.rs | 29 ++++++-- crates/xfa/src/tests.rs | 73 +++++++++++++++++++ 13 files changed, 343 insertions(+), 50 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 2dd652263d..4734b52de3 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -9530,3 +9530,59 @@ fn an_opening_reset_reaches_a_row_added_while_opening() { assert_eq!(data.count(&pdfcraft_xfa::som_to_path("form[0].page1[0].table[0]"), "row"), 2, "the row was added"); assert_eq!(data.text_at(&pdfcraft_xfa::som_to_path("form[0].page1[0].table[0].row[1].item[0]")).unwrap_or(""), "", "reset in the data"); } + +/// A reset in an opening script reaches an exclusion group of a row an earlier opening script +/// added, before the row has widgets: the group's data goes back to the on value of its default +/// button, and stays so when the form is saved. +#[test] +fn an_opening_reset_reaches_an_exclusion_group_of_a_row_added_while_opening() { + let group = r#"AAB"#; + let tpl = pdfcraft_xfa::fixtures::scripted_template() + .replace( + "if (qty.rawValue === null) qty.rawValue = 2;", + r#"if (qty.rawValue === null) qty.rawValue = 2; table._row.addInstance(1);"#, + ) + .replace(r#""#, &format!(r#"{group}"#)) + .replace( + r#""#, + r#""#, + ); + let mut s = Session::new().with_clock(|| 1_700_000_000); + let id = s.open("rows.pdf", None, Arc::new(pdfcraft_xfa::fixtures::shell(&tpl)), None).unwrap(); + let out = s.take_js_output(id); + assert!(out.errors.is_empty(), "{:?}", out.errors); + let cos = pdfcraft_cos::Document::open(s.save_bytes(id).unwrap()).unwrap(); + let data = pdfcraft_xfa::data_of(&cos).unwrap(); + assert_eq!(data.count(&pdfcraft_xfa::som_to_path("form[0].page1[0].table[0]"), "row"), 2, "the row was added"); + assert_eq!(data.text_at(&pdfcraft_xfa::som_to_path("form[0].page1[0].table[0].row[1].pick[0]")).unwrap_or(""), "A", "group reset in the data"); +} + +/// A reset in an opening script clears a multi-select list of a hidden subform (no widget) +/// whose picks the data holds as `value` elements: its empty default leaves no pick in the +/// saved data, and the rest of the data stays as it was. +#[test] +fn an_opening_reset_clears_structured_picks_of_a_field_without_a_widget() { + let tpl = pdfcraft_xfa::fixtures::scripted_template() + .replace( + r#""#, + r#"abc"#, + ) + .replace( + "if (qty.rawValue === null) qty.rawValue = 2;", + r#"if (qty.rawValue === null) qty.rawValue = 2; xfa.host.resetData("form.page1.details.tags");"#, + ) + .replace( + "", + r#"
3
ackept
 +"#, + ); + let mut s = Session::new().with_clock(|| 1_700_000_000); + let id = s.open("tags.pdf", None, Arc::new(pdfcraft_xfa::fixtures::shell(&tpl)), None).unwrap(); + let out = s.take_js_output(id); + assert!(out.errors.is_empty(), "{:?}", out.errors); + let cos = pdfcraft_cos::Document::open(s.save_bytes(id).unwrap()).unwrap(); + let data = pdfcraft_xfa::data_of(&cos).unwrap(); + assert_eq!(data.value_at(&pdfcraft_xfa::som_to_path("form[0].page1[0].details[0].tags[0]")).unwrap_or_default(), "", "picks reset in the data"); + assert_eq!(data.text_at(&pdfcraft_xfa::som_to_path("form[0].page1[0].details[0].note[0]")), Some("kept")); + assert_eq!(data.text_at(&pdfcraft_xfa::som_to_path("form[0].page1[0].qty[0]")), Some("3")); +} diff --git a/crates/engine/src/xfa.rs b/crates/engine/src/xfa.rs index d0b674bc04..3d163ca5f7 100644 --- a/crates/engine/src/xfa.rs +++ b/crates/engine/src/xfa.rs @@ -233,15 +233,52 @@ fn plain_som(som: &str) -> String { out } -/// Does `name` from a script (`qty`, `page1.qty`, `xfa.form.form.page1.qty`, or a container -/// such as `table`) name this SOM or one of its ancestors? -fn som_matches(som: &str, name: &str) -> bool { - let plain = plain_som(som); - let n = plain_som(name.trim()); - let n = n.strip_prefix("xfa.form.").or_else(|| n.strip_prefix("$form.")).unwrap_or(&n); - let want: Vec<&str> = n.split('.').filter(|s| !s.is_empty()).collect(); - let have: Vec<&str> = plain.split('.').filter(|s| !s.is_empty()).collect(); - !want.is_empty() && have.windows(want.len()).any(|w| w == want.as_slice()) +/// The names a script passed to a reset (`qty`, `page1.qty`, `xfa.form.form.page1.qty`, or a +/// container such as `table`), indexed, so matching an object against them costs the same +/// however many names there are. +struct ResetTargets { + /// Each name's steps without indexes, joined by `.`. + wants: HashSet, + /// The most steps of a name. + longest: usize, +} + +impl ResetTargets { + fn new(names: &[String]) -> Self { + let mut wants = HashSet::new(); + let mut longest = 0; + for name in names { + let n = plain_som(name.trim()); + let n = n.strip_prefix("xfa.form.").or_else(|| n.strip_prefix("$form.")).unwrap_or(&n); + let steps: Vec<&str> = n.split('.').filter(|s| !s.is_empty()).collect(); + if !steps.is_empty() { + longest = longest.max(steps.len()); + wants.insert(steps.join(".")); + } + } + ResetTargets { wants, longest } + } + + /// Does a name name this SOM or one of its ancestors: is it a run of the SOM's steps? The + /// runs are looked up, at most [`Self::longest`] steps each, so the work depends on the + /// SOM's depth and not on how many names there are. + fn matches(&self, som: &str) -> bool { + let plain = plain_som(som); + let have: Vec<&str> = plain.split('.').filter(|s| !s.is_empty()).collect(); + for start in 0..have.len() { + let mut run = String::new(); + for (k, step) in have.iter().skip(start).take(self.longest).enumerate() { + if k > 0 { + run.push('.'); + } + run.push_str(step); + if self.wants.contains(&run) { + return true; + } + } + } + false + } } /// A generated field as the runner tracks it: kind, value and on state. @@ -585,22 +622,20 @@ impl<'a, 'b> Runner<'a, 'b> { fn reset(&mut self, names: &[String]) -> Result<(), String> { self.flush()?; let by_som = self.by_som().clone(); - let listed: Option> = if names.is_empty() { - None - } else { - Some(by_som.iter().filter(|(som, _)| names.iter().any(|n| som_matches(som, n))).map(|(_, f)| f.clone()).collect()) - }; - // Fields of the live form with no widget yet (a row an opening script added, before the - // form is laid out again): their data goes back to the template's default too, or the - // value would stay and be laid out and saved. + let targets = (!names.is_empty()).then(|| ResetTargets::new(names)); + let wanted = |som: &str| targets.as_ref().is_none_or(|t| t.matches(som)); + let listed: Option> = targets.as_ref().map(|_| by_som.iter().filter(|(som, _)| wanted(som)).map(|(_, f)| f.clone()).collect()); + // Fields and exclusion groups of the live form with no widget yet (a row an opening + // script added, before the form is laid out again): their data goes back to the + // template's default too, or the value would stay and be laid out and saved. let unshown: Vec = { let live = self.live(); let mut out = Vec::new(); let mut stack: Vec<&pdfcraft_xfa::FormNode> = vec![&live.root]; while let Some(n) = stack.pop() { - if n.kind == Some(pdfcraft_xfa::NodeKind::Field) + if matches!(n.kind, Some(pdfcraft_xfa::NodeKind::Field | pdfcraft_xfa::NodeKind::ExclGroup)) && !by_som.contains_key(&n.som) - && (names.is_empty() || names.iter().any(|name| som_matches(&n.som, name))) + && wanted(&n.som) { out.push(n.som.clone()); } @@ -611,10 +646,7 @@ impl<'a, 'b> Runner<'a, 'b> { for som in unshown { let Some(text) = pdfcraft_xfa::reset_default(self.tpl, &som) else { continue }; let path = pdfcraft_xfa::data_path(self.tpl, &som); - if self.data.as_ref().and_then(|d| d.text_at(&path)).unwrap_or("") != text { - self.data_op(pdfcraft_xfa::DataOp::Value { path, text }); - self.changes.values += 1; - } + self.reset_data(path, text); } if listed.as_ref().is_some_and(Vec::is_empty) { self.invalidate(); @@ -623,6 +655,7 @@ impl<'a, 'b> Runner<'a, 'b> { pdfcraft_forms::reset(self.doc, listed.as_deref()).map_err(|e| e.to_string())?; self.fields = None; let infos = self.fields().clone(); + let listed: Option> = listed.as_ref().map(|l| l.iter().collect()); for (som, name) in &by_som { if listed.as_ref().is_some_and(|l| !l.contains(name)) { continue; @@ -632,16 +665,21 @@ impl<'a, 'b> Runner<'a, 'b> { continue; } let path = pdfcraft_xfa::data_path(self.tpl, som); - let text = data_text(info); - if self.data.as_ref().and_then(|d| d.text_at(&path)).unwrap_or("") != text { - self.data_op(pdfcraft_xfa::DataOp::Value { path, text }); - self.changes.values += 1; - } + self.reset_data(path, data_text(info)); } self.invalidate(); Ok(()) } + /// Put a reset's `text` in the data at `path`, unless it already holds it: a multi-select + /// list's picks (`value` elements) are compared as its lines, and replaced. + fn reset_data(&mut self, path: pdfcraft_xfa::DataPath, text: String) { + if self.data.as_ref().and_then(|d| d.value_at(&path)).unwrap_or_default() != text { + self.data_op(pdfcraft_xfa::DataOp::Reset { path, text }); + self.changes.values += 1; + } + } + /// Apply what a script of `ev` did. Returns whether the form has to be laid out again. fn apply_effects(&mut self, ev: &ScriptEvent, effects: &[XfaEffect]) -> Result { let mut relayout = false; @@ -1079,14 +1117,36 @@ mod tests { assert!(started.elapsed() < std::time::Duration::from_secs(2), "took {:?}", started.elapsed()); } + /// Matching objects against a reset's names costs the same however many names there are: + /// 5,000 objects against 50,000 names that miss them all is 250 million comparisons when + /// each object is compared with every name. + #[test] + fn matching_many_objects_against_many_reset_names_is_bounded() { + let names: Vec = (0..50_000).map(|i| format!("form1.page1.other{i}")).collect(); + let started = std::time::Instant::now(); + let targets = ResetTargets::new(&names); + let hits = (0..5_000).filter(|i| targets.matches(&format!("form1[0].page1[0].table[0].row[{i}].item[0]"))).count(); + assert_eq!(hits, 0); + assert!(targets.matches("form1[0].page1[0].other49999[0]")); + assert!(started.elapsed() < std::time::Duration::from_secs(2), "took {:?}", started.elapsed()); + } + #[test] fn script_names_match_indexed_som_paths() { + let som_matches = |som: &str, name: &str| ResetTargets::new(&[name.to_string()]).matches(som); assert!(som_matches("form[0].page1[0].qty[0]", "qty")); assert!(som_matches("form[0].page1[0].qty[0]", "page1.qty")); assert!(som_matches("form[0].page1[0].qty[0]", "xfa.form.form.page1.qty")); + assert!(som_matches("form[0].page1[0].qty[0]", "$form.form.page1.qty")); assert!(som_matches("form[0].page1[0].table[0].row[2].amount[0]", "table")); + assert!(som_matches("form[0].page1[0].table[0].row[2].amount[0]", "table.row[0].amount")); assert!(!som_matches("form[0].page1[0].qty[0]", "qt")); assert!(!som_matches("form[0].page1[0].price[0]", "qty")); + assert!(!som_matches("form[0].page1[0].qty[0]", "page1.page1.qty")); + assert!(!som_matches("form[0].page1[0].qty[0]", " . ")); + assert!(!ResetTargets::new(&[]).matches("form[0].qty[0]")); + let many = ResetTargets::new(&["price".into(), "page1.qty".into()]); + assert!(many.matches("form[0].page1[0].qty[0]") && many.matches("form[0].page2[0].price[0]")); assert_eq!( split_instance(&Template::default(), "form[0].table[0].row[2]").map(|(p, ps, n, i)| (p.len(), ps, n, i)), Some((2, "form[0].table[0]".into(), "row".into(), 2)) diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index cca4cb4881..c62a260120 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -5485,3 +5485,23 @@ fn locked_button_export_edits_leave_the_document_unchanged() { assert_eq!(write_incremental(&doc, &SaveOptions::default()).unwrap(), before); } } + +/// A reset of named fields costs the same per field however many are named: resetting 60,000 +/// fields by name is not 3.6 billion name comparisons. +#[test] +fn resetting_many_named_fields_is_not_quadratic() { + const N: usize = 60_000; + let kids: String = (0..N).map(|i| format!("{} 0 R ", i + 4)).collect(); + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 3 0 R >>".into(), + "<< /Type /Pages /Kids [] /Count 0 >>".into(), + format!("<< /Fields [{kids}] >>"), + ]; + objs.extend((0..N).map(|i| format!("<< /FT /Tx /T (f{i}) /V (a) /DV (a) >>"))); + let mut doc = document(&objs); + let names: Vec = (0..N).map(|i| format!("f{i}")).collect(); + let started = std::time::Instant::now(); + assert_eq!(reset(&mut doc, Some(&names)).unwrap(), 0); + assert!(started.elapsed() < std::time::Duration::from_secs(5), "took {:?}", started.elapsed()); + assert!(matches!(reset(&mut doc, Some(&["f0".to_string(), "nope".to_string()])), Err(FormError::NoSuchField(n)) if n == "nope")); +} diff --git a/crates/forms/src/values.rs b/crates/forms/src/values.rs index e8615567b6..24fc61e2ce 100644 --- a/crates/forms/src/values.rs +++ b/crates/forms/src/values.rs @@ -631,13 +631,16 @@ pub fn reset(doc: &mut Document, names: Option<&[String]>) -> Result = names.map(|_| all.iter().map(|f| f.name.as_str()).collect()).unwrap_or_default(); if let Some(n) = names - && let Some(missing) = n.iter().find(|n| !all.iter().any(|f| &f.name == *n)) + && let Some(missing) = n.iter().find(|n| !have.contains(n.as_str())) { return Err(FormError::NoSuchField(missing.clone())); } + let named: Option> = names.map(|n| n.iter().map(String::as_str).collect()); let mut changed = 0; - for f in all.iter().filter(|f| names.is_none_or(|n| n.contains(&f.name))) { + for f in all.iter().filter(|f| named.as_ref().is_none_or(|n| n.contains(f.name.as_str()))) { if f.value == f.default { continue; } diff --git a/crates/xfa/src/data.rs b/crates/xfa/src/data.rs index 7c387c44ef..6fb29fd95a 100644 --- a/crates/xfa/src/data.rs +++ b/crates/xfa/src/data.rs @@ -34,8 +34,8 @@ pub use dates::{iso_to_pattern, pattern_to_iso}; pub(crate) use import::MAX_IMPORT_BYTES; pub use import::{DataImport, ImportNode, export_data, import_data, parse_data_file, write_data_image}; pub use passwords::{clear_password_values, has_password_values}; -pub(crate) use path::template_field; pub use path::{data_path, parse_datasets, som_to_path}; +pub(crate) use path::{template_field, template_object}; pub use values::{FieldData, FieldDatum, is_on, read_values, read_values_with}; #[cfg(test)] pub(crate) use write::ENSURED; @@ -83,11 +83,25 @@ impl DataNode { /// elements (one way a multi-select list's picks are stored), those on separate lines. pub fn value_at(&self, path: &[(String, usize)]) -> Option { let n = self.get(path)?; - if n.children.is_empty() || !n.children.iter().all(|c| c.name == "value") { + if !n.holds_picks() { return Some(n.text.clone()); } - Some(n.children.iter().map(|c| c.text.trim()).filter(|t| !t.is_empty()).take(1000).collect::>().join("\n")) + Some(picks_of(n.children.iter().map(|c| c.text.as_str())).collect::>().join("\n")) } + + /// Whether this node holds a multi-select list's picks: children, all `value` elements. + pub(crate) fn holds_picks(&self) -> bool { + !self.children.is_empty() && self.children.iter().all(|c| c.name == "value") + } +} + +/// Most picks a multi-select value is read or written with. +pub(crate) const MAX_PICKS: usize = 1000; + +/// The picks among `texts` (`value` elements, or the lines of a value): non-empty, trimmed, at +/// most [`MAX_PICKS`]. +pub(crate) fn picks_of<'a>(texts: impl Iterator) -> impl Iterator { + texts.map(str::trim).filter(|t| !t.is_empty()).take(MAX_PICKS) } /// One change to the data, for [`write_data_ops`]. @@ -100,6 +114,11 @@ pub enum DataOp { Instances { parent: DataPath, name: String, count: usize }, /// Take the node at `path` out (a script removed a row). Remove(DataPath), + /// A reset to `text`: as [`DataOp::Value`], and also into a node holding a multi-select + /// list's picks (children, all `value` elements), which then holds one `value` per line of + /// `text`, none when it is empty. A node with other children is left as it is, as + /// [`DataOp::Value`] leaves it. + Reset { path: DataPath, text: String }, } impl DataNode { @@ -131,6 +150,15 @@ impl DataNode { n.text = text.clone(); } } + DataOp::Reset { path, text } => { + let Some(n) = self.ensure(path) else { return }; + if n.children.is_empty() { + n.text = text.clone(); + } else if n.holds_picks() { + n.children = + picks_of(text.lines()).map(|t| DataNode { name: "value".into(), text: t.to_string(), children: Vec::new() }).collect(); + } + } DataOp::Instances { parent, name, count } => { if let Some(&last) = count.checked_sub(1).as_ref() { let mut path = parent.clone(); diff --git a/crates/xfa/src/data/import.rs b/crates/xfa/src/data/import.rs index 006f4ff73b..0f3529bb6d 100644 --- a/crates/xfa/src/data/import.rs +++ b/crates/xfa/src/data/import.rs @@ -166,7 +166,7 @@ pub fn write_data_image( } let ct: String = content_type.chars().filter(|c| c.is_ascii_alphanumeric() || matches!(c, '/' | '-' | '.' | '+')).take(64).collect(); let b64 = base64::engine::general_purpose::STANDARD.encode(bytes); - let ensure = [Want { som: String::new(), path: path.clone(), text: String::new(), ensure: true }]; + let ensure = [Want { som: String::new(), path: path.clone(), text: String::new(), ensure: true, picks: false }]; rewrite(doc, &[Op::Values(&ensure), Op::Picture(path, &ct, &b64)], reuse) } diff --git a/crates/xfa/src/data/merge.rs b/crates/xfa/src/data/merge.rs index 0aefc634b5..f13f59f93f 100644 --- a/crates/xfa/src/data/merge.rs +++ b/crates/xfa/src/data/merge.rs @@ -311,6 +311,26 @@ fn merge_values(src: &str, data: roxmltree::Node, wants: &[Want], edits: &mut Ve continue; } let current: String = node.children().filter(|c| c.is_text()).filter_map(|c| c.text()).collect(); + if w.picks && holds_picks(node) { + // A reset of a multi-select list's picks: one `value` per line, none for an + // empty value, unless the node already holds those. + let have: Vec<&str> = super::picks_of(node.children().filter(|c| c.is_element()).map(|c| c.text().unwrap_or(""))).collect(); + let want: Vec<&str> = super::picks_of(w.text.lines()).collect(); + if have == want { + continue; + } + // The first pick's own tag, prefix included. + let tag = node.children().find(|c| c.is_element()).and_then(|c| qname(src, &c.range())).unwrap_or("value"); + let inner: String = want.iter().map(|p| format!("<{tag}>{}", escape(p).0)).collect(); + match replace_edit(src, node, &inner) { + Some(e) => edits.push(e), + None => skipped(warnings, STRUCTURED, &w.som), + } + if dropped { + skipped(warnings, CONTROL, &w.som); + } + continue; + } if node.children().any(|c| c.is_element()) { let all: String = node.descendants().filter(|c| c.is_text()).filter_map(|c| c.text()).collect(); // Ignore existing XML indentation, but not whitespace in the accepted value. @@ -368,6 +388,13 @@ fn merge_values(src: &str, data: roxmltree::Node, wants: &[Want], edits: &mut Ve } } +/// Whether `node` holds a multi-select list's picks: element children, all `value` elements +/// (as [`DataNode::holds_picks`](super::DataNode::holds_picks) reads the parsed data). +fn holds_picks(node: roxmltree::Node) -> bool { + let mut kids = node.children().filter(|c| c.is_element()).peekable(); + kids.peek().is_some() && kids.all(|c| c.tag_name().name() == "value") +} + /// Merge `ops` into `text` in order. `None`: nothing changed (or nothing could be merged). pub(super) fn merge_all(text: &str, within: Within, ops: &[Op], warnings: &mut Vec) -> Option { let mut current: Option = None; diff --git a/crates/xfa/src/data/passwords.rs b/crates/xfa/src/data/passwords.rs index 0f98a3f2b4..8674a4bd89 100644 --- a/crates/xfa/src/data/passwords.rs +++ b/crates/xfa/src/data/passwords.rs @@ -163,7 +163,7 @@ fn password_dataset_wants(doc: &Document) -> Vec { .into_iter() .filter_map(|(som, path, default)| { let cur = data.value_at(&path)?; - (!cur.is_empty() && cur != default).then_some(Want { som, path, text: default, ensure: false }) + (!cur.is_empty() && cur != default).then_some(Want { som, path, text: default, ensure: false, picks: false }) }) .collect() }) diff --git a/crates/xfa/src/data/path.rs b/crates/xfa/src/data/path.rs index d3568817d2..caa87cb3f0 100644 --- a/crates/xfa/src/data/path.rs +++ b/crates/xfa/src/data/path.rs @@ -131,6 +131,16 @@ fn named_at<'a>(nodes: &'a [Node], name: &str, index: usize, depth: usize) -> Op /// as the form counts it ([`named_at`]). `None` when the template has no such field, or it is /// a button of an exclusion group (whose buttons take their value from the group). pub(crate) fn template_field<'a>(tpl: &'a Template, som: &str) -> Option<&'a Field> { + match template_object(tpl, som)? { + Node::Field(f) => Some(f), + _ => None, + } +} + +/// The template object (field, exclusion group, subform...) the object at a SOM path is an +/// instance of, found as [`template_field`] finds a field. `None` for the root and for a button +/// of an exclusion group. +pub(crate) fn template_object<'a>(tpl: &'a Template, som: &str) -> Option<&'a Node> { let path = som_to_path(som); let ((root, _), rest) = path.split_first()?; if tpl.root.common.name.as_deref() != Some(root.as_str()) { @@ -145,10 +155,7 @@ pub(crate) fn template_field<'a>(tpl: &'a Template, som: &str) -> Option<&'a Fie _ => return None, }; } - match named_at(children, last, *last_index, 0)? { - Node::Field(f) => Some(f), - _ => None, - } + named_at(children, last, *last_index, 0) } pub(super) fn path_som(path: &DataPath) -> String { diff --git a/crates/xfa/src/data/values.rs b/crates/xfa/src/data/values.rs index 8042e774fe..8362960091 100644 --- a/crates/xfa/src/data/values.rs +++ b/crates/xfa/src/data/values.rs @@ -90,6 +90,9 @@ pub(super) struct Want { pub(super) text: String, /// Create the node even for an empty value (an instance of a repeating subform). pub(super) ensure: bool, + /// A reset ([`DataOp::Reset`](super::DataOp::Reset)): a node holding a multi-select list's picks + /// takes the text as its picks. + pub(super) picks: bool, } /// `tpl`, or the document's own template when none is given (parsed once). @@ -137,7 +140,7 @@ pub(super) fn wanted(doc: &Document, fields: &[FieldDatum], tpl: Option<&Templat FieldData::Radio(sel) => radio_dataset_value(doc, f, sel), FieldData::None => continue, }; - out.push(Want { som, path, text, ensure: false }); + out.push(Want { som, path, text, ensure: false, picks: false }); } out } diff --git a/crates/xfa/src/data/write.rs b/crates/xfa/src/data/write.rs index 4667036fe5..3dffa47b34 100644 --- a/crates/xfa/src/data/write.rs +++ b/crates/xfa/src/data/write.rs @@ -111,9 +111,10 @@ pub fn write_data_ops(doc: &mut Document, ops: &[DataOp], reuse: Option) }; for op in ops { match op { - DataOp::Value { path, text } if !path.is_empty() => { + DataOp::Value { path, text } | DataOp::Reset { path, text } if !path.is_empty() => { // An empty value clears an existing node and needs no new one. - run.push(Want { som: path_som(path), path: path.clone(), text: text.clone(), ensure: false }); + let picks = matches!(op, DataOp::Reset { .. }); + run.push(Want { som: path_som(path), path: path.clone(), text: text.clone(), ensure: false, picks }); } DataOp::Instances { parent, name, count } => { let done = queued.entry((parent, name.as_str())).or_insert(0); @@ -129,7 +130,7 @@ pub fn write_data_ops(doc: &mut Document, ops: &[DataOp], reuse: Option) *done += 1; let mut path = parent.clone(); path.push((name.clone(), k)); - run.push(Want { som: format!("{name}[{k}]"), path, text: String::new(), ensure: true }); + run.push(Want { som: format!("{name}[{k}]"), path, text: String::new(), ensure: true, picks: false }); } } DataOp::Remove(path) if !path.is_empty() => { diff --git a/crates/xfa/src/script.rs b/crates/xfa/src/script.rs index f0906a603a..ab4e7af208 100644 --- a/crates/xfa/src/script.rs +++ b/crates/xfa/src/script.rs @@ -575,15 +575,30 @@ pub fn fields_by_som(doc: &Document) -> HashMap { out } -/// What a reset gives the field at `som` in the data: its template default, or nothing when it has -/// none. `None` for an object a reset restores no value of: not a field of the template, or a -/// button, signature or picture field. +/// What a reset gives the field or exclusion group at `som` in the data: its template default +/// as saved (a choice list's item value, not its shown text, as [`Field::saved_value`] gives +/// it), or nothing when it has none. A group's default is the on value of the first of its +/// buttons whose own default is that on value. `None` for an object a reset restores no value +/// of: not a field or group of the template, or a button, signature or picture field. pub fn reset_default(tpl: &Template, som: &str) -> Option { - let f = crate::data::template_field(tpl, som)?; - if matches!(f.ui, crate::model::Ui::Button | crate::model::Ui::Signature | crate::model::Ui::ImageEdit) { - return None; + match crate::data::template_object(tpl, som)? { + Node::Field(f) => { + if matches!(f.ui, crate::model::Ui::Button | crate::model::Ui::Signature | crate::model::Ui::ImageEdit) { + return None; + } + Some(f.value.plain().map(|v| f.saved_value(v.trim())).unwrap_or_default()) + } + Node::ExclGroup(g) => Some( + g.fields + .iter() + .find_map(|f| { + let on = f.items.first().map(|s| s.trim()).filter(|s| !s.is_empty())?; + (f.value.plain()?.trim() == on).then(|| on.to_string()) + }) + .unwrap_or_default(), + ), + _ => None, } - Some(f.value.plain().unwrap_or_default()) } /// Lay the form out again from its template, data and overrides, replacing the pages and the diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index e424be5441..1ef8239823 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4145,3 +4145,76 @@ fn clear_password_values_clears_static_xfa_password_fields() { let packets = read_packets(&doc).unwrap().unwrap(); assert!(!packets.xdp.contains("hunter2"), "{}", packets.xdp); } + +/// A template for the reset defaults: a choice list showing `Label` and saving `code`, with +/// `Label` its default, and an exclusion group whose second button is on by default. +fn reset_defaults_template() -> Template { + let xml = r#""#; + crate::parse::parse(xml).unwrap().0 +} + +/// A reset gives a choice list the default it saves (the item value), not the text it shows, +/// as the live form reads it. +#[test] +fn a_choice_list_resets_to_its_saved_default() { + let tpl = reset_defaults_template(); + assert_eq!(reset_default(&tpl, "form[0].page1[0].kind[0]").as_deref(), Some("code")); +} + +/// An exclusion group resets to the on value of the button that is on by default, or to +/// nothing; its buttons, and push buttons, restore no value of their own. +#[test] +fn an_exclusion_group_resets_to_its_default_button() { + let tpl = reset_defaults_template(); + assert_eq!(reset_default(&tpl, "form[0].page1[0].pick[0]").as_deref(), Some("B")); + assert_eq!(reset_default(&tpl, "form[0].page1[0].none[0]").as_deref(), Some("")); + assert_eq!(reset_default(&tpl, "form[0].page1[0].pick[0].b[0]"), None); + assert_eq!(reset_default(&tpl, "form[0].page1[0].go[0]"), None); + assert_eq!(reset_default(&tpl, "form[0].page1[0]"), None); + assert_eq!(reset_default(&tpl, "form[0].page1[0].missing[0]"), None); +} + +/// A reset replaces a multi-select list's picks stored as `value` elements, in the packet and +/// in the copy scripts read; an empty default leaves no pick. An ordinary value write still +/// leaves a node with children alone. +#[test] +fn a_reset_replaces_structured_multi_select_picks() { + let xdp = template_with_data(1, "
ab

rich

"); + let path = som_to_path("form[0].tags[0]"); + let value_at = |doc: &Document| data_of(doc).and_then(|d| d.value_at(&path)).unwrap_or_default(); + + let mut doc = Document::open(Arc::new(shell(&xdp))).unwrap(); + write_data_ops(&mut doc, &[DataOp::Value { path: path.clone(), text: String::new() }], None).unwrap(); + assert_eq!(value_at(&doc), "a\nb", "an ordinary write leaves the picks"); + let mut copy = data_of(&doc).unwrap(); + copy.apply(&DataOp::Value { path: path.clone(), text: String::new() }); + assert_eq!(copy.value_at(&path).as_deref(), Some("a\nb")); + + let r = write_data_ops(&mut doc, &[DataOp::Reset { path: path.clone(), text: String::new() }], None).unwrap(); + assert!(r.warnings.is_empty(), "{:?}", r.warnings); + assert_eq!(value_at(&doc), "", "an empty default leaves no pick"); + copy.apply(&DataOp::Reset { path: path.clone(), text: String::new() }); + assert_eq!(copy.value_at(&path).as_deref(), Some("")); + + let mut doc = Document::open(Arc::new(shell(&xdp))).unwrap(); + write_data_ops(&mut doc, &[DataOp::Reset { path: path.clone(), text: "c\n\n d&e ".into() }], None).unwrap(); + assert_eq!(value_at(&doc), "c\nd&e"); + let packets = read_packets(&doc).unwrap().unwrap(); + assert!(packets.xdp.contains("cd&e"), "{}", packets.xdp); + let mut copy = data_of(&Document::open(Arc::new(shell(&xdp))).unwrap()).unwrap(); + copy.apply(&DataOp::Reset { path: path.clone(), text: "c\n\n d&e ".into() }); + assert_eq!(copy.value_at(&path).as_deref(), Some("c\nd&e")); + + // Other structured content (rich text) is not picks: a reset leaves it, with a warning. + let body = som_to_path("form[0].body[0]"); + let r = write_data_ops(&mut doc, &[DataOp::Reset { path: body.clone(), text: String::new() }], None).unwrap(); + assert!(!r.warnings.is_empty()); + assert_eq!(data_of(&doc).unwrap().get(&body).map(|n| n.children.len()), Some(1)); + copy.apply(&DataOp::Reset { path: body.clone(), text: String::new() }); + assert_eq!(copy.get(&body).map(|n| n.children.len()), Some(1)); +}