diff --git a/crates/forms/src/author/join.rs b/crates/forms/src/author/join.rs index f98bd42cb9..0e74b94be7 100644 --- a/crates/forms/src/author/join.rs +++ b/crates/forms/src/author/join.rs @@ -12,9 +12,6 @@ use super::*; /// stays: a widget's own triggers live there. const FIELD_ONLY_KEYS: [&[u8]; 15] = [b"T", b"FT", b"Kids", b"Ff", b"V", b"DV", b"Opt", b"TU", b"TM", b"MaxLen", b"Q", b"DS", b"RV", b"TI", b"I"]; -/// The most "ChoiceN" names tried for a renamed export value. -const MAX_FRESH_STATES: usize = 10_000; - /// Whether `field` holds `/Opt`, which its buttons' export values index (ISO 32000-2 §12.7.5.2). fn has_opt(doc: &Document, field: ObjRef) -> bool { doc.get(field).as_dict().is_some_and(|d| d.contains(b"Opt")) @@ -42,6 +39,24 @@ pub(super) fn check_join(doc: &Document, f: &Field, target: &Field) -> Result<() /// selected value. `f` leaves the form; its own value and field-level properties go with it, /// and `target`'s stay. Call [`check_join`] first. pub(super) fn join_radio_group(doc: &mut Document, f: &Field, target: &Field) -> Result<(), FormError> { + let theirs: HashSet = target.widgets.iter().filter_map(|w| w.on_state.clone()).collect(); + let mut taken = theirs.clone(); + taken.extend(f.widgets.iter().filter_map(|w| w.on_state.clone())); + // The group's value counts only when one of its buttons has it. + let selected = target.value.first().filter(|v| theirs.contains(*v)).cloned(); + // Each clashing value of `f` gets one fresh name, shared by every button that had it. Of + // `taken.len() + 1` names at least one is free, so the search always ends in one. + let mut renamed: HashMap = HashMap::new(); + for old in f.widgets.iter().filter_map(|w| w.on_state.as_deref()).filter(|s| theirs.contains(*s)) { + if renamed.contains_key(old) { + continue; + } + let Some(fresh) = (1..=taken.len().saturating_add(1)).map(|i| format!("Choice{i}")).find(|c| !taken.contains(c)) else { + return invalid(format!("no free export value for {old}")); + }; + taken.insert(fresh.clone()); + renamed.insert(old.to_owned(), fresh); + } // A group that is its own one button gets a field with a separate widget first, so it can // hold more than one. if let [only] = target.widgets.as_slice() @@ -49,23 +64,6 @@ pub(super) fn join_radio_group(doc: &mut Document, f: &Field, target: &Field) -> { split_merged(doc, target, only)?; } - let theirs: HashSet = target.widgets.iter().filter_map(|w| w.on_state.clone()).collect(); - let mut taken = theirs.clone(); - taken.extend(f.widgets.iter().filter_map(|w| w.on_state.clone())); - // The group's value counts only when one of its buttons has it. - let selected = target.value.first().filter(|v| theirs.contains(*v)).cloned(); - // Each clashing value of `f` gets one fresh name, shared by every button that had it. - let mut renamed: HashMap> = HashMap::new(); - for old in f.widgets.iter().filter_map(|w| w.on_state.as_deref()).filter(|s| theirs.contains(*s)) { - if renamed.contains_key(old) { - continue; - } - let fresh = (1..=MAX_FRESH_STATES).map(|i| format!("Choice{i}")).find(|c| !taken.contains(c)); - if let Some(c) = &fresh { - taken.insert(c.clone()); - } - renamed.insert(old.to_owned(), fresh); - } detach(doc, f.obj)?; // `f` is no longer a field, even when it stays on as a widget. drop_from_calculation_order(doc, |r| r == f.obj)?; @@ -79,7 +77,7 @@ pub(super) fn join_radio_group(doc: &mut Document, f: &Field, target: &Field) -> } let mut state = w.on_state.clone(); if let Some(old) = w.on_state.as_deref() - && let Some(Some(fresh)) = renamed.get(old) + && let Some(fresh) = renamed.get(old) && rename_state(doc, &mut d, old, fresh) { state = Some(fresh.clone()); diff --git a/crates/forms/src/author/lifecycle.rs b/crates/forms/src/author/lifecycle.rs index c62b9bde2c..d23d28d95a 100644 --- a/crates/forms/src/author/lifecycle.rs +++ b/crates/forms/src/author/lifecycle.rs @@ -237,7 +237,23 @@ pub(super) fn drop_from_calculation_order(doc: &mut Document, gone: impl Fn(ObjR } /// Widget-only keys: they stay with each widget when a merged field is split. -const WIDGET_KEYS: [&[u8]; 12] = [b"Type", b"Subtype", b"Rect", b"P", b"AP", b"AS", b"MK", b"F", b"BS", b"Border", b"H", b"StructParent"]; +const WIDGET_KEYS: [&[u8]; 13] = [b"Type", b"Subtype", b"Rect", b"P", b"AP", b"AS", b"MK", b"F", b"BS", b"Border", b"H", b"StructParent", b"A"]; + +/// The `/AA` triggers of a widget annotation (ISO 32000-2 §12.6.3, Table 197). The rest (`K`, +/// `F`, `V`, `C`, Table 199) belong to the field. +const WIDGET_TRIGGERS: [&[u8]; 10] = [b"E", b"X", b"D", b"U", b"Fo", b"Bl", b"PO", b"PC", b"PV", b"PI"]; + +/// Split additional-actions dictionary `aa` into the widget's triggers and the field's. +/// An empty half is `None`. +fn split_triggers(aa: &Dict) -> (Option, Option) { + let (mut widget, mut field) = (Dict::new(), Dict::new()); + for (k, v) in aa.iter() { + let to = if WIDGET_TRIGGERS.contains(&k.as_slice()) { &mut widget } else { &mut field }; + to.set(k.clone(), v.clone()); + } + let some = |d: Dict| (!d.is_empty()).then_some(d); + (some(widget), some(field)) +} /// Duplicate a field onto other pages (Acrobat: right-click a field ▸ Duplicate): each page /// gets a widget at the same place, belonging to the same field, so they share one value. @@ -303,12 +319,26 @@ pub(super) fn split_merged(doc: &mut Document, f: &Field, model: &Widget) -> Res kid.set(k.to_vec(), v.clone()); } } + // The widget's own triggers go with it; the field's stay. + let aa = d.get(b"AA").and_then(|a| doc.resolve(a).as_dict().cloned()); + let (widget_aa, field_aa) = aa.as_ref().map(split_triggers).unwrap_or_default(); + if let Some(w) = widget_aa { + kid.set(b"AA".to_vec(), Object::Dict(w)); + } kid.set(b"Parent".to_vec(), Object::Ref(f.obj)); let kr = doc.add(Object::Dict(kid)); doc.update_dict(f.obj, |d| { for k in WIDGET_KEYS { d.remove(k); } + if aa.is_some() { + match field_aa { + Some(fa) => d.set(b"AA".to_vec(), Object::Dict(fa)), + None => { + d.remove(b"AA"); + } + } + } d.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(kr)])); })?; // The page lists the widget now, not the field. diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index 4648167c4f..4bda036c48 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -4676,15 +4676,14 @@ fn joining_radio_buttons_that_share_a_value_keep_sharing_it() { assert_eq!(group.widgets.iter().filter_map(|w| w.on_state.clone()).collect::>(), ["Choice1", "Choice2", "Choice2"]); } -/// A radio field that is its own (only) widget, as other writers make one-button groups, can -/// both join a group and be joined. -#[test] -fn radio_fields_that_are_their_own_widget_join_and_are_joined() { +/// One page holding radio fields that are each their own only widget, as other writers make +/// one-button groups: `(name, export value, x)`. +fn merged_radios(specs: &[(&str, &str, f64)]) -> Document { let mut doc = one_page(); let page = page_refs(&doc)[0]; let af = crate::author::ensure_form(&mut doc).unwrap(); let mut made = Vec::new(); - for (name, state, x) in [("solo", "A", 50.0), ("other", "B", 80.0)] { + for &(name, state, x) in specs { let look = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(pdfcraft_cos::Dict::new(), b"0 g".to_vec()))); let mut n = pdfcraft_cos::Dict::new(); n.set(state.as_bytes().to_vec(), Object::Ref(look)); @@ -4711,6 +4710,14 @@ fn radio_fields_that_are_their_own_widget_join_and_are_joined() { made.push(r); } doc.update_dict(page, |p| p.set(b"Annots".to_vec(), Object::Array(made.iter().map(|r| Object::Ref(*r)).collect()))).unwrap(); + doc +} + +/// A radio field that is its own (only) widget, as other writers make one-button groups, can +/// both join a group and be joined. +#[test] +fn radio_fields_that_are_their_own_widget_join_and_are_joined() { + let mut doc = merged_radios(&[("solo", "A", 50.0), ("other", "B", 80.0)]); assert_eq!(rename_to(&mut doc, "other", "solo").unwrap(), "solo"); let doc = reopen(&doc); let all = fields(&doc); @@ -4729,6 +4736,70 @@ fn radio_fields_that_are_their_own_widget_join_and_are_joined() { assert_eq!(on_page, kids, "the page lists the widgets, not the field"); } +/// A group that is its own only widget keeps its button's actions when another button joins +/// it: the new widget takes `/A` and the widget triggers of `/AA`, and the field keeps its own. +#[test] +fn a_joined_merged_groups_button_keeps_its_actions() { + let mut doc = merged_radios(&[("solo", "A", 50.0), ("other", "B", 80.0)]); + let solo = field(&fields(&doc), "solo").obj; + let js = |code: &str| { + let mut a = pdfcraft_cos::Dict::new(); + a.set(b"S".to_vec(), Object::name("JavaScript")); + a.set(b"JS".to_vec(), pdfcraft_cos::PdfString::text(code)); + Object::Dict(a) + }; + let mut aa = pdfcraft_cos::Dict::new(); + aa.set(b"Fo".to_vec(), js("focus();")); + aa.set(b"V".to_vec(), js("validate();")); + let aa = doc.add(Object::Dict(aa)); + doc.update_dict(solo, |d| { + d.set(b"A".to_vec(), js("up();")); + d.set(b"AA".to_vec(), Object::Ref(aa)); + }) + .unwrap(); + rename_to(&mut doc, "other", "solo").unwrap(); + let doc = reopen(&doc); + assert_eq!( + field_actions(&doc, "solo").unwrap(), + [(Trigger::MouseUp, FieldAction::JavaScript("up();".into())), (Trigger::OnFocus, FieldAction::JavaScript("focus();".into()))] + ); + let group = field(&fields(&doc), "solo").clone(); + let fd = doc.get(group.obj).as_dict().unwrap().clone(); + let field_aa = doc.resolve(fd.get(b"AA").unwrap()).as_dict().unwrap().clone(); + assert!(!fd.contains(b"A") && field_aa.contains(b"V") && !field_aa.contains(b"Fo"), "the field keeps only its own trigger"); + let wd = doc.get(group.widgets[0].obj).as_dict().unwrap().clone(); + let widget_aa = doc.resolve(wd.get(b"AA").unwrap()).as_dict().unwrap().clone(); + assert!(widget_aa.contains(b"Fo") && !widget_aa.contains(b"V")); +} + +/// However many "ChoiceN" names a group already uses, a joining button with a clashing export +/// value gets a free one, so the two buttons can still be chosen apart. +#[test] +fn a_joining_button_gets_a_free_export_value_when_choice_names_run_long() { + let mut doc = two_radio_groups("Choice1", "Choice1"); + let group = field(&fields(&doc), "Group1").clone(); + let model = doc.get(group.widgets[0].obj).as_dict().unwrap().clone(); + let look = doc.add(Object::Stream(pdfcraft_cos::Stream::from_raw(pdfcraft_cos::Dict::new(), b"0 g".to_vec()))); + let mut kids = vec![Object::Ref(group.widgets[0].obj)]; + for i in 2..=10_000 { + let mut n = pdfcraft_cos::Dict::new(); + n.set(format!("Choice{i}").into_bytes(), Object::Ref(look)); + n.set(b"Off".to_vec(), Object::Ref(look)); + let mut ap = pdfcraft_cos::Dict::new(); + ap.set(b"N".to_vec(), Object::Dict(n)); + let mut w = model.clone(); + w.set(b"AP".to_vec(), Object::Dict(ap)); + w.remove(b"P"); + kids.push(Object::Ref(doc.add(Object::Dict(w)))); + } + doc.update_dict(group.obj, |d| d.set(b"Kids".to_vec(), Object::Array(kids))).unwrap(); + rename_to(&mut doc, "Group2", "Group1").unwrap(); + let group = field(&fields(&doc), "Group1").clone(); + let states: Vec = group.widgets.iter().filter_map(|w| w.on_state.clone()).collect(); + assert_eq!(states.len(), 10_001); + assert_eq!(states.last().map(String::as_str), Some("Choice10001")); +} + /// Only a radio button joins by name: other clashes are refused as before, and a locked group /// isn't changed. #[test] diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index d0e9482d06..2421af6573 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1527,6 +1527,9 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { let form = |app: &PdfCraftApp| app.active_ids().and_then(|(_, id)| app.session.get(id)).map(|doc| doc.form.clone()); // A rename also renames the field in the formulas that read it. let before = props.name.as_ref().and_then(|_| form(app)); + // The widget being edited, by object: a radio button that joins a group by its new name + // is appended after the group's own buttons, so its index changes. + let widget = before.as_ref().and_then(|all| all.iter().find(|f| f.name == d.field)?.widgets.get(d.widget).map(|w| w.obj)); if app.apply_edit(Edit::SetFieldProps { name: d.field.clone(), props: Box::new(props) }) && let Some(i) = app.active { @@ -1535,7 +1538,10 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { let typed = d.name.trim(); let prefix = d.field.rsplit_once('.').filter(|_| !typed.contains('.')).map(|(p, _)| format!("{p}.")).unwrap_or_default(); let name = format!("{prefix}{typed}"); - if let (Some(before), Some(after)) = (before, form(app)) { + let after = form(app); + let index = + widget.and_then(|obj| after.as_ref()?.iter().find(|f| f.name == name)?.widgets.iter().position(|w| w.obj == obj)).unwrap_or(d.widget); + if let (Some(before), Some(after)) = (before, after) { let n = pdfcraft_engine::form_calculations_changed(&before, &after).iter().filter(|f| **f != name).count(); if n > 0 { app.notify(crate::i18n::trn( @@ -1546,7 +1552,7 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) { )); } } - app.views[i].prepare.selected = Some((name, d.widget)); + app.views[i].prepare.selected = Some((name, index)); } } if props_now && let Some(d) = app.comment_props.take() { diff --git a/crates/ui-egui/tests/it/prepare.rs b/crates/ui-egui/tests/it/prepare.rs index 8d565ab696..e8deba404a 100644 --- a/crates/ui-egui/tests/it/prepare.rs +++ b/crates/ui-egui/tests/it/prepare.rs @@ -775,14 +775,18 @@ fn actions_tab_edits_the_script_of_an_existing_action() { /// One page with a push button "go": Mouse Up runs a script with a link chained after it, On Blur /// runs another script. fn chained_action_fixture() -> Vec { - let objs = [ + pdf_of(&[ "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] /DA (/Helv 0 Tf 0 g) >> >>", "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>", "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>", "<< /Type /Annot /Subtype /Widget /FT /Btn /Ff 65536 /T (go) /Rect [20 300 100 320] /P 3 0 R /A 5 0 R /AA << /Bl << /S /JavaScript /JS (this.dirty = true;) >> >> >>", "<< /S /JavaScript /JS (app.alert\\(1\\);) /Next 6 0 R >>", "<< /S /URI /URI (https://example.org) >>", - ]; + ]) +} + +/// A PDF of `objs`, numbered from 1, with the first as its catalog. +fn pdf_of(objs: &[&str]) -> Vec { let mut out = b"%PDF-1.7\n".to_vec(); let mut offs = Vec::new(); for (i, o) in objs.iter().enumerate() { @@ -798,6 +802,65 @@ fn chained_action_fixture() -> Vec { out } +/// Two one-button radio groups placed apart on page 1: "Group1" (export `A`) and "Group2" (`B`). +fn two_radio_groups_fixture() -> Vec { + let widget = |parent: u32, x: u32, state: &str| { + format!( + "<< /Type /Annot /Subtype /Widget /Parent {parent} 0 R /Rect [{x} 300 {} 314] /P 3 0 R /AS /Off /AP << /N << /{state} 8 0 R /Off 8 0 R >> >> >>", + x + 14 + ) + }; + let (a, b) = (widget(4, 40, "A"), widget(6, 120, "B")); + pdf_of(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 6 0 R] >> >>", + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 300 400] >>", + "<< /Type /Page /Parent 2 0 R /Annots [5 0 R 7 0 R] >>", + "<< /FT /Btn /Ff 49152 /T (Group1) /Kids [5 0 R] >>", + &a, + "<< /FT /Btn /Ff 49152 /T (Group2) /Kids [7 0 R] >>", + &b, + "<< /Length 3 >>\nstream\n0 g\nendstream", + ]) +} + +/// #4949: renaming a radio button to another group's name moves it into that group, after the +/// group's own buttons. The moved button stays the selected one, so a nudge moves it and not the +/// group's first button, and undo and redo take the rename and the nudge back and forth. +#[test] +fn a_radio_button_joining_a_group_by_name_stays_selected() { + let mut h = harness_with("radios.pdf", two_radio_groups_fixture()); + h.state_mut().execute("form.prepare"); + h.run_steps(2); + let rects = |h: &Harness<'static, PdfCraftApp>| { + let s = h.state(); + let form = &s.session.get(s.views[0].id).unwrap().form; + form.iter().flat_map(|f| f.widgets.iter().map(|w| w.rect)).collect::>() + }; + let start = rects(&h); + h.state_mut().open_field_props("Group2", 0); + h.run_steps(2); + h.state_mut().field_props.as_mut().expect("open").name = "Group1".into(); + h.get_by_label("OK").click(); + h.run_steps(3); + assert_eq!(names(&h), ["Group1"]); + assert_eq!(h.state().views[0].prepare.selected, Some(("Group1".to_string(), 1)), "the moved button"); + h.key_press(egui::Key::ArrowRight); + h.run_steps(2); + let nudged = rects(&h); + assert_eq!(nudged[0], start[0], "the group's own button stays"); + assert!((nudged[1][0] - start[1][0] - 1.0).abs() < 0.01, "{:?} → {:?}", start[1], nudged[1]); + h.state_mut().execute("edit.undo"); + h.state_mut().execute("edit.undo"); + h.run_steps(2); + assert_eq!(names(&h), ["Group1", "Group2"]); + assert_eq!(rects(&h), start); + h.state_mut().execute("edit.redo"); + h.state_mut().execute("edit.redo"); + h.run_steps(2); + assert_eq!(names(&h), ["Group1"]); + assert_eq!(rects(&h), nudged); +} + #[test] fn actions_tab_shows_a_chain_and_keeps_it_when_the_first_action_is_replaced() { use egui_kittest::kittest::NodeT;