diff --git a/crates/automation/src/forms.rs b/crates/automation/src/forms.rs index adf0bd0884..136b85ab5d 100644 --- a/crates/automation/src/forms.rs +++ b/crates/automation/src/forms.rs @@ -214,7 +214,8 @@ fn value_for(f: &FormField, v: &Value) -> Result { Ok(match (f.kind, v) { (FormFieldKind::CheckBox, Value::Bool(b)) => FieldValue::Check(*b), (FormFieldKind::Radio, Value::Null) => FieldValue::Radio(None), - (FormFieldKind::Radio, Value::String(s)) => FieldValue::Radio(Some(s.clone())), + // An option as `form_fields` lists it (an export value) names its button's on state. + (FormFieldKind::Radio, Value::String(s)) => FieldValue::Radio(Some(f.state_for_export(s).unwrap_or(s).to_owned())), (FormFieldKind::Combo | FormFieldKind::List, Value::Array(a)) => FieldValue::Choice( a.iter().map(|x| x.as_str().map(str::to_owned).ok_or_else(|| wrong("list values must be strings"))).collect::>()?, ), diff --git a/crates/automation/tests/it/automation.rs b/crates/automation/tests/it/automation.rs index 6a5765b911..39ef2bd44e 100644 --- a/crates/automation/tests/it/automation.rs +++ b/crates/automation/tests/it/automation.rs @@ -2630,6 +2630,40 @@ fn form_fill_warns_when_a_value_does_not_fit() { assert!(r.get("warnings").is_none(), "{r}"); } +/// `form_fill` takes a radio option as `form_fields` lists it: the export value. With `/Opt [(1) +/// (2) (3)]` and on states named by position (`/0`, `/1`, `/2`; ISO 32000-2 Table 230), `1` is the +/// first button. It used to be read as the state `/1` and turn the second one on. +#[test] +fn form_fill_picks_a_radio_option_by_its_export_value() { + 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 7 0 R 8 0 R] >>", + "<< /Fields [5 0 R] /DA (/Helv 0 Tf 0 g) >>", + "<< /FT /Btn /T (title) /Ff 49152 /Opt [(1) (2) (3)] /V /Off /Kids [6 0 R 7 0 R 8 0 R] >>", + "<< /Type /Annot /Subtype /Widget /Parent 5 0 R /AS /Off /Rect [10 250 22 262] /P 3 0 R /AP << /N << /0 9 0 R /Off 9 0 R >> >> >>", + "<< /Type /Annot /Subtype /Widget /Parent 5 0 R /AS /Off /Rect [40 250 52 262] /P 3 0 R /AP << /N << /1 9 0 R /Off 9 0 R >> >> >>", + "<< /Type /Annot /Subtype /Widget /Parent 5 0 R /AS /Off /Rect [70 250 82 262] /P 3 0 R /AP << /N << /2 9 0 R /Off 9 0 R >> >> >>", + "<< /Type /XObject /Subtype /Form /BBox [0 0 12 12] /Length 0 >>\nstream\n\nendstream", + ]; + let dir = workdir("radio-export"); + 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(); + assert_eq!(ok(&mut a, "form_fields", json!({ "doc": doc }))["fields"][0]["options"], json!(["1", "2", "3"])); + for pick in ["1", "2", "3"] { + ok(&mut a, "form_fill", json!({ "doc": doc, "values": { "title": pick } })); + assert_eq!(ok(&mut a, "form_fields", json!({ "doc": doc }))["fields"][0]["value"], pick); + // The button drawn on is the one picked. + let f = a.session().get(pdfcraft_engine::DocId(doc)).unwrap().form[0].clone(); + let on: Vec = f.widgets.iter().enumerate().filter(|(_, w)| w.state.as_deref().is_some_and(|s| s != "Off")).map(|(i, _)| i).collect(); + assert_eq!(on, [pick.parse::().unwrap() - 1], "picked {pick}"); + } + // An on-state name the options do not list still selects its button. + ok(&mut a, "form_fill", json!({ "doc": doc, "values": { "title": "0" } })); + assert_eq!(ok(&mut a, "form_fields", json!({ "doc": doc }))["fields"][0]["value"], "1"); +} + /// A form whose fields are only page widgets (the `/Fields` list is empty) still lists, fills and /// resets through the tools, and the leniency shows up as a repair note. #[test] diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 333d0fd498..50d46e1215 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6110,6 +6110,42 @@ fn static_xfa_forms_take_their_values_from_the_datasets_and_write_them_back() { assert!(!d2.info.warnings.iter().any(|w| w.contains("taken from this form's XFA data")), "nothing was rewritten"); } +/// A static XFA exclusion group laid out as a radio group with `/Opt [(1) (2)]` and on states +/// named by position (`/0`, `/1`; ISO 32000-2 Table 230): the data's item `1` is the first +/// button, not the second one's state `/1`, and a click stores the clicked button's item. +/// Public benefit forms are built this way; their "English" came back as "French". +#[test] +fn a_static_xfa_radio_group_with_numeric_items_keeps_the_picked_button() { + let lang = "form1[0].page1[0].lang[0]"; + let data_of = |bytes: Arc>| { + let cos = pdfcraft_cos::Document::open(bytes).unwrap(); + let data = pdfcraft_xfa::parse_datasets(&pdfcraft_xfa::read_packets(&cos).unwrap().unwrap().xdp).unwrap(); + data.text_at(&pdfcraft_xfa::som_to_path(lang)).map(str::to_owned) + }; + let states = |s: &Session, id: DocId| { + let f = s.get(id).unwrap().form.iter().find(|f| f.name == lang).unwrap().clone(); + (f.value.clone(), f.widgets.iter().map(|w| w.state.clone().unwrap_or_default()).collect::>()) + }; + let form = |data: &str| Arc::new(pdfcraft_xfa::fixtures::static_radio_shell(&format!("{data}"))); + let mut s = Session::new().with_clock(|| 1_700_000_000); + // Data filled elsewhere: item 1 (English) turns the first button on, item 2 the second. + let id = s.open("one.pdf", None, form("1"), None).unwrap(); + assert_eq!(states(&s, id), (vec!["0".to_string()], vec!["0".to_string(), "Off".to_string()])); + let id = s.open("two.pdf", None, form("2"), None).unwrap(); + assert_eq!(states(&s, id), (vec!["1".to_string()], vec!["Off".to_string(), "1".to_string()])); + // A click on a button (its on state, as the page sends it) stores that button's item. + let id = s.open("blank.pdf", None, form(""), None).unwrap(); + for (state, item) in [("0", "1"), ("1", "2")] { + s.apply(id, Edit::SetFieldValue { name: lang.into(), value: FieldValue::Radio(Some(state.into())) }).unwrap(); + let saved = s.save_bytes(id).unwrap(); + assert_eq!(data_of(saved.clone()).as_deref(), Some(item), "clicked {state}"); + // Reopened, the same button is on and nothing is rewritten. + let again = s.open("again.pdf", None, saved, None).unwrap(); + assert_eq!(states(&s, again).0, vec![state.to_string()], "clicked {state}"); + assert!(!s.get(again).unwrap().info.warnings.iter().any(|w| w.contains("taken from this form's XFA data")), "clicked {state}"); + } +} + /// A static XFA form whose values are only in its datasets takes every one on opening, as /// filling the fields one at a time would leave it: values, appearances and the note; a /// read-only field keeps its own value. diff --git a/crates/engine/src/xfa_form.rs b/crates/engine/src/xfa_form.rs index 5ceaba1cd4..debd45d5e2 100644 --- a/crates/engine/src/xfa_form.rs +++ b/crates/engine/src/xfa_form.rs @@ -133,7 +133,8 @@ pub(crate) fn strip_xfa(doc: &mut pdfcraft_cos::Document, cx: &mut EditCtx) -> R match value { pdfcraft_xfa::FieldData::Text(t) => f.value.join("\n") != *t && !t.trim().is_empty(), pdfcraft_xfa::FieldData::Check(on) => f.value.is_empty() == *on, - pdfcraft_xfa::FieldData::Radio(sel) => f.value.first() != sel.as_ref(), + // The data holds the export value, the field the on-state name. + pdfcraft_xfa::FieldData::Radio(sel) => f.value.first().map(|v| f.export_for_state(v)) != sel.as_deref(), pdfcraft_xfa::FieldData::None => false, } }) @@ -283,7 +284,12 @@ pub(crate) fn xfa_values_from_datasets(doc: &mut pdfcraft_cos::Document) -> Resu } pdfcraft_xfa::FieldData::Text(t) => (f.value.join("\n") != t).then_some(FieldValue::Text(t)), pdfcraft_xfa::FieldData::Check(on) => (f.value.is_empty() == on).then_some(FieldValue::Check(on)), - pdfcraft_xfa::FieldData::Radio(sel) => (f.value.first() != sel.as_ref()).then_some(FieldValue::Radio(sel)), + // The data holds the item (the export value); the field takes the on state that + // exports it, which with `/Opt` may be a position (`1` is the state `/0`). + pdfcraft_xfa::FieldData::Radio(sel) => { + let state = sel.map(|v| f.state_for_export(&v).map_or(v.clone(), str::to_owned)); + (f.value.first() != state.as_ref()).then_some(FieldValue::Radio(state)) + } pdfcraft_xfa::FieldData::None => None, }; if let Some(v) = new { diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 775306d865..5776e6d3e1 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -214,6 +214,19 @@ impl Field { }) } + /// The on-state name that selects `value` read as an export value: form data, or one of the + /// options [`Field::export_of`] lists. A field with one `/Opt` entry per widget matches those + /// entries first and its on-state names only after them: the names may be positions (`/0`, + /// `/1`; ISO 32000-2 Table 230), so the export value `1` is the first widget, not the second + /// one's state `/1`. Elsewhere this is [`Field::state_for`]. + pub fn state_for_export(&self, value: &str) -> Option<&str> { + let by_export = self.button_exports().and_then(|exports| { + let i = exports.iter().position(|(e, _)| e == value)?; + self.widgets.get(i)?.on_state.as_deref() + }); + by_export.or_else(|| self.state_for(value)) + } + /// The export value for an on-state name such as the field's value; the name itself when the /// field has no `/Opt`. pub fn export_for_state<'a>(&'a self, state: &'a str) -> &'a str { diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index cca4cb4881..fb568532f9 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -387,6 +387,31 @@ fn opt_export_values_select_button_states() { assert!(err.to_string().contains("options: Post, Pick-up"), "{err}"); } +/// Export values that are themselves numbers (`/Opt [(1) (2)]`, as static XFA forms write their +/// items) name the buttons in `/Opt` order when read as export values, though `1` is also the +/// second button's on-state name (`/0`, `/1` by position; ISO 32000-2 Table 230). +#[test] +fn numeric_opt_export_values_name_their_own_buttons() { + let objs: Vec = vec![ + "<< /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 7 0 R] >>".into(), + "<< /Fields [5 0 R] /DA (/Helv 0 Tf 0 g) >>".into(), + "<< /FT /Btn /T (lang) /Ff 49152 /Opt [(1) (2)] /V /Off /Kids [6 0 R 7 0 R] >>".into(), + "<< /Type /Annot /Subtype /Widget /Parent 5 0 R /AS /Off /Rect [100 700 115 715] /P 3 0 R /AP << /N << /0 8 0 R /Off 8 0 R >> >> >>".into(), + "<< /Type /Annot /Subtype /Widget /Parent 5 0 R /AS /Off /Rect [200 700 215 715] /P 3 0 R /AP << /N << /1 8 0 R /Off 8 0 R >> >> >>".into(), + "<< /Length 0 >>\nstream\n\nendstream".into(), + ]; + let doc = document(&objs); + let all = fields(&doc); + let lang = field(&all, "lang"); + assert_eq!((lang.state_for_export("1"), lang.state_for_export("2")), (Some("0"), Some("1"))); + // A state name no export value matches still names its button; anything else names none. + assert_eq!((lang.state_for_export("0"), lang.state_for_export("3")), (Some("0"), None)); + // On-state names keep their meaning where the caller has one (a click sends the state). + assert_eq!(lang.state_for("1"), Some("1")); +} + /// What fits a text field is measured as its appearance draws it: in the field's own font (its /// `/DR` font, Helvetica where that can't show the text), inside the border and padding, at its /// size, wrapped for multi-line, at the smallest automatic size, as asterisks for a password, diff --git a/crates/xfa/src/data/values.rs b/crates/xfa/src/data/values.rs index 8042e774fe..55cf864645 100644 --- a/crates/xfa/src/data/values.rs +++ b/crates/xfa/src/data/values.rs @@ -8,7 +8,8 @@ pub enum FieldData { Text(String), /// Checked or not. Check(bool), - /// The selected button's on value, if any. + /// The selected button, if any: written from its on-state name (the data gets its item, see + /// `radio_dataset_value`), read as the data's item, which is its export value. Radio(Option), /// Nothing to write (buttons, signatures). None, @@ -35,14 +36,31 @@ fn som_of(doc: &Document, f: &FieldDatum) -> String { /// The datasets text for a radio group. The AcroForm value is an appearance-state name /// (`New York` is stored as `New_York`); `/PCOn` on the selected widget is the original item. +/// Without it, a group with one `/Opt` entry per widget exports that entry: its state names may +/// be positions (`/0`, `/1`; ISO 32000-2 Table 230), and the data holds the item (`1`, `2`). fn radio_dataset_value(doc: &Document, f: &FieldDatum, sel: &Option) -> String { let Some(selected) = sel.as_deref().filter(|s| !s.is_empty() && *s != "Off") else { return String::new(); }; let Some(group) = doc.get(f.obj).as_dict().cloned() else { return selected.to_string() }; let kids = group.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()).unwrap_or_else(|| vec![Object::Ref(f.obj)]); - for kid in kids { - let Some(widget) = doc.resolve(&kid).as_dict().cloned() else { continue }; + // One export value per widget, as the field's `/Opt` lists them (an entry may be an + // `[export display]` pair, as a choice field's are). + let exports: Option>> = group + .get(b"Opt") + .map(|o| doc.resolve(o)) + .and_then(|o| o.as_array().cloned()) + .map(|opt| { + opt.iter() + .map(|o| match &*doc.resolve(o) { + Object::Array(pair) => pair.first().and_then(|e| doc.resolve(e).as_string().map(|s| s.to_text())), + other => other.as_string().map(|s| s.to_text()), + }) + .collect() + }) + .filter(|opt: &Vec>| opt.len() == kids.len()); + for (i, kid) in kids.iter().enumerate() { + let Some(widget) = doc.resolve(kid).as_dict().cloned() else { continue }; let original = text_of(doc, &widget, b"PCOn"); let state = widget .get(b"AP") @@ -51,7 +69,8 @@ fn radio_dataset_value(doc: &Document, f: &FieldDatum, sel: &Option) -> .and_then(|n| doc.resolve(&n).as_dict().cloned()) .and_then(|n| n.iter().map(|(k, _)| String::from_utf8_lossy(k).into_owned()).find(|k| k != "Off")); if state.as_deref() == Some(selected) || original.as_deref() == Some(selected) { - return original.unwrap_or_else(|| selected.to_string()); + let export = exports.as_ref().and_then(|e| e.get(i).cloned().flatten()); + return original.or(export).unwrap_or_else(|| selected.to_string()); } } selected.to_string() diff --git a/crates/xfa/src/fixtures.rs b/crates/xfa/src/fixtures.rs index 8f6e0f7ee8..c1fd0d9c44 100644 --- a/crates/xfa/src/fixtures.rs +++ b/crates/xfa/src/fixtures.rs @@ -144,6 +144,30 @@ pub fn static_shell(data: &str) -> Vec { assemble(objs) } +/// A static XFA form with one exclusion group `form1[0].page1[0].lang[0]` (items `1` English, +/// `2` French) bound to the data by name, laid out as an AcroForm radio group whose `/Opt` holds +/// the items and whose on states are the buttons' positions `/0` and `/1` (ISO 32000-2 Table 230), +/// as public static forms write it. Its datasets hold `data` (the children of `xfa:data`). +pub fn static_radio_shell(data: &str) -> Vec { + let xdp = format!( + "{data}" + ); + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R /AcroForm << /XFA 5 0 R /Fields [6 0 R] /DA (/Helv 0 Tf 0 g) >> >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R /Annots [9 0 R 10 0 R] >>".to_vec(), + b"<< /Length 0 >>\nstream\n\nendstream".to_vec(), + format!("<< /Length {} >>\nstream\n{xdp}\nendstream", xdp.len()).into_bytes(), + b"<< /T (form1[0]) /Kids [7 0 R] >>".to_vec(), + b"<< /T (page1[0]) /Parent 6 0 R /Kids [8 0 R] >>".to_vec(), + b"<< /FT /Btn /Ff 49152 /T (lang[0]) /Parent 7 0 R /Opt [(1) (2)] /V /Off /Kids [9 0 R 10 0 R] >>".to_vec(), + b"<< /Type /Annot /Subtype /Widget /Parent 8 0 R /Rect [72 700 84 712] /P 3 0 R /AS /Off /MK << /BC [0 0 0] >> /AP << /N << /0 11 0 R /Off 11 0 R >> >> >>".to_vec(), + b"<< /Type /Annot /Subtype /Widget /Parent 8 0 R /Rect [144 700 156 712] /P 3 0 R /AS /Off /MK << /BC [0 0 0] >> /AP << /N << /1 11 0 R /Off 11 0 R >> >> >>".to_vec(), + b"<< /Type /XObject /Subtype /Form /BBox [0 0 12 12] /Length 0 >>\nstream\n\nendstream".to_vec(), + ]; + assemble(objs) +} + /// A static XFA form of `n` text fields `form1[0].page1[0].f[0]` on one page, whose values /// (`v`) are only in its datasets, as when another viewer filled it in. The first field is /// read-only. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 0b6f658dd6..975f75e022 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4523,8 +4523,8 @@ milestone = "M6" status = "shipped" commands = ["form.fields"] tools = ["form_fill"] -evidence = ["crates/ui-egui/tests/it/forms.rs::check_boxes_radios_and_choices", "crates/forms/src/tests.rs::check_boxes_and_radios_switch_states", "crates/ui-egui/tests/it/forms.rs::tab_and_space_fill_in_a_check_box_between_text_fields", "crates/ui-egui/tests/it/forms.rs::clicking_a_check_box_while_typing_keeps_the_text_and_focuses_the_box", "crates/ui-egui/tests/it/forms.rs::radio_buttons_take_tab_space_and_arrows", "crates/forms/src/tests.rs::a_round_xfa_check_button_draws_a_round_background_and_border", "crates/xfa/src/tests.rs::round_check_buttons_are_marked_for_a_round_appearance", "crates/engine/src/tests.rs::round_xfa_check_buttons_open_drawn_round"] -notes = "NoToggleToOff honoured; widgets without appearances get our own, drawn round for an XFA check button with shape=\"round\" (its background and border a circle inside the widget, as another viewer draws them on a synthetic form). Tab and Shift+Tab stop on check boxes and on each radio button, Space toggles the focused one, and the arrows move within a radio group (#2223); a click focuses the box too" +evidence = ["crates/ui-egui/tests/it/forms.rs::check_boxes_radios_and_choices", "crates/forms/src/tests.rs::check_boxes_and_radios_switch_states", "crates/ui-egui/tests/it/forms.rs::tab_and_space_fill_in_a_check_box_between_text_fields", "crates/ui-egui/tests/it/forms.rs::clicking_a_check_box_while_typing_keeps_the_text_and_focuses_the_box", "crates/ui-egui/tests/it/forms.rs::radio_buttons_take_tab_space_and_arrows", "crates/forms/src/tests.rs::a_round_xfa_check_button_draws_a_round_background_and_border", "crates/xfa/src/tests.rs::round_check_buttons_are_marked_for_a_round_appearance", "crates/engine/src/tests.rs::round_xfa_check_buttons_open_drawn_round", "crates/forms/src/tests.rs::numeric_opt_export_values_name_their_own_buttons", "crates/automation/tests/it/automation.rs::form_fill_picks_a_radio_option_by_its_export_value", "crates/engine/src/tests.rs::a_static_xfa_radio_group_with_numeric_items_keeps_the_picked_button"] +notes = "NoToggleToOff honoured; widgets without appearances get our own, drawn round for an XFA check button with shape=\"round\" (its background and border a circle inside the widget, as another viewer draws them on a synthetic form). Tab and Shift+Tab stop on check boxes and on each radio button, Space toggles the focused one, and the arrows move within a radio group (#2223); a click focuses the box too. A radio group with one /Opt export value per button and on states named by position (/0, /1; ISO 32000-2 Table 230) takes an export value from form_fill and from a static XFA form's data as that button, also when the export values are numbers (1 is the first button, not the state /1), and a click writes the button's /Opt value to the XFA data" [[feature]] id = "form.fill-choice"