M6: a joined radio button stays selected, keeps its actions, and always gets a free value

Review follow-ups on #4977:

- ui-egui: after a rename, the Properties dialog re-selects the edited
  widget by object, not by its old index. A button that joins a group is
  appended after the group's own buttons, so the old index selected the
  group's first button and a nudge moved the wrong one.
- split_merged (join and Duplicate): the new widget takes /A and the
  widget triggers of /AA (E X D U Fo Bl PO PC PV PI); the field keeps
  K F V C.
- join: a clashing export value searches at most taken + 1 ChoiceN
  names, so one is always free, and the names are chosen before
  anything is written.

Tests: a_joined_merged_groups_button_keeps_its_actions,
a_joining_button_gets_a_free_export_value_when_choice_names_run_long,
a_radio_button_joining_a_group_by_name_stays_selected (ui-egui). Each
fails with its fix reverted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9

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<String> = 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<String, String> = 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<String> = 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<String, Option<String>> = 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());

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<Dict>, Option<Dict>) {
+    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.

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::<Vec<_>>(), ["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<String> = 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]

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() {

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<u8> {
-    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<u8> {
     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<u8> {
     out
 }
 
+/// Two one-button radio groups placed apart on page 1: "Group1" (export `A`) and "Group2" (`B`).
+fn two_radio_groups_fixture() -> Vec<u8> {
+    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::<Vec<_>>()
+    };
+    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;