From d39fe5b2336d36c5801904cb6f4b90a11d332df7 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 27 Oct 2026 09:37:36 -0400 Subject: [PATCH 1/3] M6: clearing a field's default leaves it with none, not its parent field's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /DV is inheritable (ISO 32000-2 §12.7.4.1). set_props removed a cleared /DV, which brought back a parent field's, so Reset Form restored the parent's default. When an ancestor sets /DV, a cleared default is now written empty (/Off for buttons, () otherwise); with none it is removed as before. New tree::ancestor_sets, cycle-safe and capped. Adapted from pdfcer (MIT, same author) 6af2773ccf599b416fab379673119c6b37c55a2c. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/automation/tests/it/automation.rs | 25 +++++++++++ crates/forms/src/author/edit.rs | 9 +++- crates/forms/src/author/tree.rs | 18 ++++++++ crates/forms/src/tests.rs | 54 ++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 5 files changed, 107 insertions(+), 3 deletions(-) diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 23351db9aa..7bbfee1d5d 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -9175,3 +9175,28 @@ fn page_render_draws_stamps_without_an_appearance() { let grey = custom.iter().filter(|p| p[0].abs_diff(p[1]) < 24 && p[1].abs_diff(p[2]) < 24).count(); assert!(grey > 100 && grey == custom.len(), "an unknown name is a grey placeholder: {grey} of {}", custom.len()); } + +/// Clearing a field's default through `form_set_props` (`"default": ""`) leaves it with none, +/// even when its parent field has one it would otherwise inherit. +#[test] +fn clearing_a_kids_default_through_tools_does_not_bring_back_its_parents() { + let objs = [ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 400 300] >>", + "<< /Type /Page /Parent 2 0 R /Annots [6 0 R] >>", + "<< /Fields [5 0 R] /DA (/Helv 12 Tf 0 g) /DR << /Font << /Helv 7 0 R >> >> >>", + "<< /T (address) /FT /Tx /DV (Main St) /Kids [6 0 R] >>", + "<< /Type /Annot /Subtype /Widget /T (city) /Parent 5 0 R /DV (Elm) /V (Elm) /Rect [10 250 150 268] /P 3 0 R >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>", + ]; + let dir = workdir("kid-default"); + std::fs::write(dir.join("form.pdf"), pdf_of(&objs)).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "form.pdf" }))["doc"].as_u64().unwrap(); + ok(&mut a, "form_set_props", json!({ "doc": doc, "field": "address.city", "default": "" })); + ok(&mut a, "form_fill", json!({ "doc": doc, "values": { "address.city": "Oak" } })); + ok(&mut a, "form_reset", json!({ "doc": doc })); + let fields = ok(&mut a, "form_fields", json!({ "doc": doc })); + let city = fields["fields"].as_array().unwrap().iter().find(|f| f["name"] == "address.city").unwrap().clone(); + assert!(city["value"].is_null() || city["value"] == "", "reset clears it rather than restoring the parent's Main St: {}", city["value"]); +} diff --git a/crates/forms/src/author/edit.rs b/crates/forms/src/author/edit.rs index dc7d91d4fc..0e45f12086 100644 --- a/crates/forms/src/author/edit.rs +++ b/crates/forms/src/author/edit.rs @@ -1,5 +1,5 @@ use super::look::{BorderStyle, da_string, patch_da, patch_widget_look, redraw_widgets}; -use super::tree::{attach, check_place, child_named, clash, detach, ensure_form, inherited, name_parts, own_inherited}; +use super::tree::{ancestor_sets, attach, check_place, child_named, clash, detach, ensure_form, inherited, name_parts, own_inherited}; use super::*; /// Swap width and height, keeping the center. `None` when that is not a usable widget. @@ -173,6 +173,7 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result Result d.set(b"DV".to_vec(), Object::Name(crate::name_bytes(v))), Some(Some(v)) => d.set(b"DV".to_vec(), PdfString::text(v)), + // `/DV` is inheritable: with an ancestor's in place, removing the field's own would + // bring that one back, so the field keeps an empty default of its own instead. + Some(None) if parent_default && matches!(f.kind, FieldKind::CheckBox | FieldKind::Radio) => { + d.set(b"DV".to_vec(), Object::Name(b"Off".to_vec())) + } + Some(None) if parent_default => d.set(b"DV".to_vec(), PdfString::text("")), Some(None) => { d.remove(b"DV"); } diff --git a/crates/forms/src/author/tree.rs b/crates/forms/src/author/tree.rs index 4f7db4d8e1..605ce95c35 100644 --- a/crates/forms/src/author/tree.rs +++ b/crates/forms/src/author/tree.rs @@ -317,6 +317,24 @@ pub(super) fn inherited(doc: &Document, field: ObjRef) -> Dict { add } +/// Whether an ancestor of `field` sets the inheritable `key`, which `field` would take on without +/// its own entry. +pub(super) fn ancestor_sets(doc: &Document, field: ObjRef, key: &[u8]) -> bool { + let mut seen = std::collections::HashSet::new(); + let mut parent = doc.get(field).as_dict().and_then(|d| d.reference(b"Parent")); + while let Some(p) = parent { + if seen.len() > 64 || !seen.insert(p) { + return false; + } + let Some(d) = doc.get(p).as_dict().cloned() else { return false }; + if d.contains(key) { + return true; + } + parent = d.reference(b"Parent"); + } + false +} + /// Copy onto `field` what it inherits (`add`, from [`inherited`]), so it keeps it when it moves. pub(super) fn own_inherited(doc: &mut Document, field: ObjRef, add: &Dict) -> Result<(), FormError> { if add.iter().next().is_some() { diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 61dab131e0..43f933fe0d 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -4577,3 +4577,57 @@ fn set_values_reads_the_form_once_not_per_value() { let reads = form_reads() - before; assert_eq!(reads, 3, "one to fill, one when f0 comes again, one to recalculate"); } + +#[test] +fn clearing_a_kids_default_does_not_bring_back_its_parents() { + let mut doc = fixture(); + let all = fields(&doc); + let city = field(&all, "address.city").clone(); + let parent = doc.get(city.obj).as_dict().unwrap().reference(b"Parent").unwrap(); + doc.update_dict(parent, |d| d.set(b"DV".to_vec(), PdfString::text("Main St"))).unwrap(); + set_props(&mut doc, "address.city", &FieldProps { default_value: Some(Some("Elm".into())), ..FieldProps::default() }).unwrap(); + assert_eq!(field(&fields(&doc), "address.city").default, ["Elm"]); + set_props(&mut doc, "address.city", &FieldProps { default_value: Some(None), ..FieldProps::default() }).unwrap(); + let mut doc = reopen(&doc); + let f = field(&fields(&doc), "address.city").clone(); + assert!(f.default.iter().all(String::is_empty), "no default, not the parent's: {:?}", f.default); + let own = doc.get(f.obj).as_dict().and_then(|d| d.get(b"DV")).and_then(|o| o.as_string()).map(PdfString::to_text); + assert_eq!(own.as_deref(), Some(""), "the kid holds an empty default of its own"); + set_value(&mut doc, "address.city", &FieldValue::Text("Oak".into())).unwrap(); + reset(&mut doc, None).unwrap(); + assert!(field(&fields(&doc), "address.city").value.iter().all(String::is_empty), "reset clears it"); + // Without a parent default, clearing still removes the key. + set_props(&mut doc, "name", &FieldProps { default_value: Some(None), ..FieldProps::default() }).unwrap(); + let name = field(&fields(&doc), "name").clone(); + assert!(doc.get(name.obj).as_dict().unwrap().get(b"DV").is_none()); +} + +#[test] +fn clearing_a_buttons_default_under_a_parent_with_one_leaves_it_off() { + // A check box, then a radio button (flags 49152, inherited from the parent). + for flags in ["", "/Ff 49152"] { + let mut doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 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 [6 0 R] >>".into(), + "<< /Fields [5 0 R] /DA (/Helv 0 Tf 0 g) >>".into(), + format!("<< /T (opts) /FT /Btn {flags} /DV /Yes /Kids [6 0 R] >>"), + "<< /Type /Annot /Subtype /Widget /T (a) /Parent 5 0 R /V /Off /AS /Off /Rect [10 10 25 25] /P 3 0 R /AP << /N << /Yes 7 0 R /Off 7 0 R >> >> >>".into(), + "<< /Length 0 >> +stream + +endstream".into(), + ]); + let kind = field(&fields(&doc), "opts.a").kind; + assert_eq!(kind == FieldKind::Radio, !flags.is_empty()); + set_props(&mut doc, "opts.a", &FieldProps { default_value: Some(None), ..FieldProps::default() }).unwrap(); + let f = field(&fields(&doc), "opts.a").clone(); + assert_eq!(doc.get(f.obj).as_dict().and_then(|d| d.get(b"DV")).and_then(|o| o.as_name()), Some(&b"Off"[..]), "{kind:?}"); + let on = if kind == FieldKind::Radio { FieldValue::Radio(Some("Yes".into())) } else { FieldValue::Check(true) }; + set_value(&mut doc, "opts.a", &on).unwrap(); + assert_eq!(field(&fields(&doc), "opts.a").value, ["Yes"]); + reset(&mut doc, None).unwrap(); + let f = field(&fields(&doc), "opts.a").clone(); + assert!(f.value.iter().all(String::is_empty), "{kind:?}: reset leaves it off, not the parent's Yes: {:?}", f.value); + } +} diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index bcba7a0ecd..8dc8ce17fd 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4991,8 +4991,8 @@ milestone = "M6" status = "shipped" commands = ["form.field.properties"] tools = ["form_set_props"] -evidence = ["crates/forms/src/tests.rs::button_caption_preserves_other_widgets_and_shared_dictionaries", "crates/forms/src/tests.rs::a_button_caption_is_drawn_in_its_fields_font_size_and_color", "crates/ui-egui/tests/it/prepare.rs::button_label_is_editable_without_renaming_the_field", "crates/automation/tests/it/button_caption.rs::button_caption_can_be_changed_after_creation", "crates/forms/src/tests.rs::properties_rename_and_delete", "crates/automation/tests/it/automation.rs::preparing_a_form_through_tools", "crates/forms/src/tests.rs::options_tab_flags_alignment_and_defaults", "crates/ui-egui/tests/it/prepare.rs::options_tab_sets_flags_alignment_and_defaults", "crates/automation/tests/it/form_bulk.rs::bulk_options_apply_per_field_and_refuse_an_invalid_comb_atomically"] -notes = "push buttons: per-widget normal label independent of the field name, including clearing and save/reopen, drawn in the font, size and color of the button's /DA (the form's /DR font, else Helvetica; an automatic size fits the button) and centered by Helvetica's widths; icon-only layout retained, rollover/down labels and icon layout editing remain unavailable; text: alignment, default, multi-line, scroll, rich text flag, limit, password, file selection, spell check, comb; choices: items, default, sort, custom text, multiple selection, commit immediately; check boxes and radios: checked by default, radios in unison. Shared GUI offers applicable text/choice flags, alignment, text limits and check styles; choices/defaults remain individual GUI edits" +evidence = ["crates/forms/src/tests.rs::button_caption_preserves_other_widgets_and_shared_dictionaries", "crates/forms/src/tests.rs::a_button_caption_is_drawn_in_its_fields_font_size_and_color", "crates/ui-egui/tests/it/prepare.rs::button_label_is_editable_without_renaming_the_field", "crates/automation/tests/it/button_caption.rs::button_caption_can_be_changed_after_creation", "crates/forms/src/tests.rs::properties_rename_and_delete", "crates/automation/tests/it/automation.rs::preparing_a_form_through_tools", "crates/forms/src/tests.rs::options_tab_flags_alignment_and_defaults", "crates/ui-egui/tests/it/prepare.rs::options_tab_sets_flags_alignment_and_defaults", "crates/automation/tests/it/form_bulk.rs::bulk_options_apply_per_field_and_refuse_an_invalid_comb_atomically", "crates/forms/src/tests.rs::clearing_a_kids_default_does_not_bring_back_its_parents", "crates/forms/src/tests.rs::clearing_a_buttons_default_under_a_parent_with_one_leaves_it_off", "crates/automation/tests/it/automation.rs::clearing_a_kids_default_through_tools_does_not_bring_back_its_parents"] +notes = "push buttons: per-widget normal label independent of the field name, including clearing and save/reopen, drawn in the font, size and color of the button's /DA (the form's /DR font, else Helvetica; an automatic size fits the button) and centered by Helvetica's widths; icon-only layout retained, rollover/down labels and icon layout editing remain unavailable; text: alignment, default (clearing it leaves none, not a parent field's), multi-line, scroll, rich text flag, limit, password, file selection, spell check, comb; choices: items, default, sort, custom text, multiple selection, commit immediately; check boxes and radios: checked by default, radios in unison. Shared GUI offers applicable text/choice flags, alignment, text limits and check styles; choices/defaults remain individual GUI edits" [[feature]] id = "form.props-actions" From 12c9051fd8b717858b725d7d0826ddb1fbbfd5ee Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 27 Oct 2026 11:01:12 -0400 Subject: [PATCH 2/3] M6: clearing a field's value leaves it empty, not its parent field's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /V is inheritable like /DV (ISO 32000-2 §12.7.4.1). Clearing a text or choice field's value removed its own /V, so a parent field's /V showed through: Reset Form on a kid with no default refilled it with the parent's value. When an ancestor sets /V, an empty value is now written instead of removing the key. Check boxes and radio buttons already write /Off. Test clearing_a_kids_value_does_not_bring_back_its_parents covers a text kid and a list box kid through reset and through set_value; each half of the fix was switched off in turn and the test failed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/forms/src/author.rs | 2 ++ crates/forms/src/author/tree.rs | 2 +- crates/forms/src/tests.rs | 28 ++++++++++++++++++++++++++++ crates/forms/src/values.rs | 6 +++++- parity/acrobat-features.toml | 2 +- 5 files changed, 37 insertions(+), 3 deletions(-) diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs index ce89fad806..76ae937249 100644 --- a/crates/forms/src/author.rs +++ b/crates/forms/src/author.rs @@ -15,6 +15,8 @@ mod lifecycle; mod look; mod tree; +pub(crate) use tree::ancestor_sets; + pub use edit::set_props; pub use lifecycle::{add_field, delete_field, delete_field_object, duplicate_field}; pub(crate) use look::button_appearance; diff --git a/crates/forms/src/author/tree.rs b/crates/forms/src/author/tree.rs index 605ce95c35..9eb0791b75 100644 --- a/crates/forms/src/author/tree.rs +++ b/crates/forms/src/author/tree.rs @@ -319,7 +319,7 @@ pub(super) fn inherited(doc: &Document, field: ObjRef) -> Dict { /// Whether an ancestor of `field` sets the inheritable `key`, which `field` would take on without /// its own entry. -pub(super) fn ancestor_sets(doc: &Document, field: ObjRef, key: &[u8]) -> bool { +pub(crate) fn ancestor_sets(doc: &Document, field: ObjRef, key: &[u8]) -> bool { let mut seen = std::collections::HashSet::new(); let mut parent = doc.get(field).as_dict().and_then(|d| d.reference(b"Parent")); while let Some(p) = parent { diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 43f933fe0d..b9153ff6e3 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -4602,6 +4602,34 @@ fn clearing_a_kids_default_does_not_bring_back_its_parents() { assert!(doc.get(name.obj).as_dict().unwrap().get(b"DV").is_none()); } +#[test] +fn clearing_a_kids_value_does_not_bring_back_its_parents() { + // A text kid, then a list box kid, under a parent with both a value and a default. + for (ft, set) in [("/FT /Tx", FieldValue::Text("Oak".into())), ("/FT /Ch /Opt [(Main St) (Oak)]", FieldValue::Choice(vec!["Oak".into()]))] { + let mut doc = document(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm 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 [6 0 R] >>".into(), + "<< /Fields [5 0 R] /DA (/Helv 0 Tf 0 g) >>".into(), + format!("<< /T (address) {ft} /V (Main St) /DV (Main St) /Kids [6 0 R] >>"), + "<< /Type /Annot /Subtype /Widget /T (city) /Parent 5 0 R /Rect [10 10 110 60] /P 3 0 R >>".into(), + ]); + set_props(&mut doc, "address.city", &FieldProps { default_value: Some(None), ..FieldProps::default() }).unwrap(); + set_value(&mut doc, "address.city", &set).unwrap(); + assert_eq!(field(&fields(&doc), "address.city").value, ["Oak"], "{ft}"); + reset(&mut doc, None).unwrap(); + let doc = reopen(&doc); + let f = field(&fields(&doc), "address.city").clone(); + assert!(f.value.iter().all(String::is_empty), "{ft}: reset leaves it empty, not the parent's: {:?}", f.value); + let mut doc = doc; + set_value(&mut doc, "address.city", &set).unwrap(); + let clear = if ft.contains("Ch") { FieldValue::Choice(Vec::new()) } else { FieldValue::Text(String::new()) }; + set_value(&mut doc, "address.city", &clear).unwrap(); + let f = field(&fields(&reopen(&doc)), "address.city").clone(); + assert!(f.value.iter().all(String::is_empty), "{ft}: clearing leaves it empty: {:?}", f.value); + } +} + #[test] fn clearing_a_buttons_default_under_a_parent_with_one_leaves_it_off() { // A check box, then a radio button (flags 49152, inherited from the parent). diff --git a/crates/forms/src/values.rs b/crates/forms/src/values.rs index 9dd79f0a70..738c3b7428 100644 --- a/crates/forms/src/values.rs +++ b/crates/forms/src/values.rs @@ -207,8 +207,9 @@ pub(crate) fn write_value(doc: &mut Document, f: &Field, value: &FieldValue, scr let t = &af::keystroke(&f.actions.format, &f.name, t).map_err(FormError::Format)?; af::validate(&f.actions.validate, t).map_err(FormError::Invalid)?; let t = &scripting::accept(doc, f, t, scripts)?; + let shadow = t.is_empty() && crate::author::ancestor_sets(doc, f.obj, b"V"); doc.update_dict(f.obj, |d| { - if t.is_empty() { + if t.is_empty() && !shadow { d.remove(b"V"); } else { d.set(b"V".to_vec(), PdfString::text(t)); @@ -296,8 +297,11 @@ pub(crate) fn write_value(doc: &mut Document, f: &Field, value: &FieldValue, scr }), _ => None, }; + // An empty value of its own keeps a parent field's /V from showing through. + let shadow = exports.is_empty() && crate::author::ancestor_sets(doc, f.obj, b"V"); doc.update_dict(f.obj, |d| { match exports.as_slice() { + [] if shadow => d.set(b"V".to_vec(), PdfString::text("")), [] => { d.remove(b"V"); } diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 8dc8ce17fd..977a1503fb 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4727,7 +4727,7 @@ milestone = "M6" status = "shipped" commands = ["form.clear"] tools = ["form_reset"] -evidence = ["crates/forms/src/tests.rs::reset_restores_defaults", "crates/ui-egui/tests/it/forms.rs::check_boxes_radios_and_choices"] +evidence = ["crates/forms/src/tests.rs::reset_restores_defaults", "crates/ui-egui/tests/it/forms.rs::check_boxes_radios_and_choices", "crates/forms/src/tests.rs::clearing_a_kids_value_does_not_bring_back_its_parents"] notes = "back to /DV; undoable" [[feature]] From ac40f62294a593b2fcc4224b259c58685c3aa5e3 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 27 Oct 2026 22:54:50 -0400 Subject: [PATCH 3/3] C012 test: a cleared choice kid has no selected value, exactly Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/forms/src/tests.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 4634d2bdf2..dea60c4e80 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -4669,7 +4669,11 @@ fn clearing_a_kids_value_does_not_bring_back_its_parents() { let clear = if ft.contains("Ch") { FieldValue::Choice(Vec::new()) } else { FieldValue::Text(String::new()) }; set_value(&mut doc, "address.city", &clear).unwrap(); let f = field(&fields(&reopen(&doc)), "address.city").clone(); - assert!(f.value.iter().all(String::is_empty), "{ft}: clearing leaves it empty: {:?}", f.value); + if ft.contains("Ch") { + assert!(f.value.is_empty(), "{ft}: clearing leaves no choice selected: {:?}", f.value); + } else { + assert!(f.value.iter().all(String::is_empty), "{ft}: clearing leaves it empty: {:?}", f.value); + } } }