collective
Browse / github.com/storytold/pdfcraft / patch
fix(forms): renaming a radio button to an existing group's name joins that group
fix(forms): renaming a radio button to an existing group's name joins that group ## What this changes Part of #4949: the group name. Today, renaming a radio button in Properties to a name another radio group already has is refused as a duplicate. So two buttons placed separately can never become one group. In a PDF, a radio group is one field whose widgets are its buttons (ISO 32000-2 §12.7.5.2.4). Two fields with one button each are two groups, so naming one after the other has to move its buttons into the other field. `set_props` now does that when both the renamed field and the field it is named after are radio groups. In `author/join.rs`: - the renamed field leaves
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- KenM76 (imported from pull request #4977, open)
- Base
1dd2737b47a9c1c9562f0af563053d1bf510e4bf- Size
- +590 −45 in 10 files
- Review
- pending
- Shared
- 2026-10-28
- Id
3d59695590e64b56eb85b5f0
How it went
No other operator has reported using it yet.
Diff
raw · crates/automation/tests/it/automation.rs crates/forms/src/author.rs crates/forms/src/author/edit.rs crates/forms/src/author/join.rs crates/forms/src/author/lifecycle.rs crates/forms/src/author/tree.rs crates/forms/src/tests.rs crates/ui-egui/src/dialogs.rs crates/ui-egui/tests/it/prepare.rs parity/acrobat-features.toml
@@ -4004,6 +4004,29 @@ fn preparing_a_form_through_tools() { assert!(matches!(a.call("form_set_image", &json!({ "doc": doc, "field": "size", "path": "pics/a_page_1.png" })), Err(ToolError::Failed(_)))); } +#[test]+fn renaming_a_radio_button_to_a_groups_name_joins_the_group_through_tools() {+ // #4949: two buttons placed separately are two groups; naming one after the other makes+ // them one group, and a clashing export value is renamed instead of shared.+ let dir = workdir("radio-join");+ let mut a = auto(&dir);+ let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();+ ok(&mut a, "form_add_field", json!({ "doc": doc, "page": 1, "type": "radio", "rect": [20, 100, 34, 114], "group": "size", "export": "S" }));+ ok(&mut a, "form_add_field", json!({ "doc": doc, "page": 1, "type": "radio", "rect": [40, 100, 54, 114], "group": "other", "export": "S" }));+ ok(&mut a, "form_fill", json!({ "doc": doc, "values": { "size": "S" } }));+ let r = ok(&mut a, "form_set_props", json!({ "doc": doc, "field": "other", "name": "size" }));+ assert_eq!(r["field"], "size");+ ok(&mut a, "doc_save", json!({ "doc": doc, "path": "joined.pdf" }));+ let doc = ok(&mut a, "doc_open", json!({ "path": "joined.pdf" }))["doc"].as_u64().unwrap();+ let fields = ok(&mut a, "form_fields", json!({ "doc": doc }));+ let f = fields["fields"].as_array().unwrap();+ assert_eq!(f.iter().map(|f| f["name"].as_str().unwrap()).collect::<Vec<_>>(), ["size"], "one group");+ assert_eq!(f[0]["options"], json!(["S", "Choice1"]), "the joining button takes a free export value");+ assert_eq!(f[0]["value"], "S", "the group keeps its selection");+ ok(&mut a, "form_fill", json!({ "doc": doc, "values": { "size": "Choice1" } }));+ assert_eq!(ok(&mut a, "form_fields", json!({ "doc": doc }))["fields"][0]["value"], "Choice1");+}+ #[test] fn rotating_a_field_through_tools() { let dir = workdir("rotate-field");@@ -11,6 +11,7 @@ use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString, Stream}; use crate::{Field, FieldKind, FormError, Widget, appearance, fields, flags, page_refs}; mod edit;+mod join; mod lifecycle; mod look; mod tree;@@ -1,3 +1,4 @@+use super::join::{check_join, join_radio_group}; 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::*;@@ -71,6 +72,9 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result<S crate::set_field_actions(doc, name, acts)?; } let mut new_name = name.to_string();+ // A radio button renamed to another radio group's name joins that group (#4949). The other+ // properties are written first, to this field, and its buttons then move.+ let mut join = None; if let Some(n) = props.name.as_deref().map(str::trim).filter(|n| *n != f.name) { // A full name under the same parents only renames the field's own part (the editor // shows nested fields by their full name): it isn't moved, so it keeps its place and@@ -79,7 +83,16 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result<S (Some((parents, own)), Some((current, _))) if parents == current => own, _ => n, };- if n.contains('.') {+ let full = match f.name.rsplit_once('.') {+ Some((parents, _)) if !n.contains('.') => format!("{parents}.{n}"),+ _ => n.to_string(),+ };+ let group = all.iter().find(|x| x.obj != f.obj && x.kind == FieldKind::Radio && x.name == full);+ if let Some(group) = group.filter(|_| f.kind == FieldKind::Radio) {+ check_join(doc, &f, group)?;+ join = Some(group.clone());+ new_name = full;+ } else if n.contains('.') { // A full name: the field moves under the parent fields it names (#2696). let parts = name_parts(n)?; if let Some(why) = all.iter().filter(|x| x.obj != f.obj).find_map(|x| clash(n, &x.name)) {@@ -385,7 +398,13 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result<S } // A label-only edit must not replace another widget's custom appearances. let only = props.caption.as_ref().filter(|_| *props == (FieldProps { caption: props.caption.clone(), ..Default::default() })).map(|(wi, _)| *wi);- redraw_widgets(doc, &new_name, only)?;+ if let Some(group) = &join {+ redraw_widgets(doc, name, only)?;+ join_radio_group(doc, &f, group)?;+ redraw_field(doc, &new_name)?;+ } else {+ redraw_widgets(doc, &new_name, only)?;+ } if props.calculate.is_some() { crate::recalculate(doc)?; }new file mode 100644@@ -0,0 +1,129 @@+//! Renaming a radio button to an existing radio group's name makes it a button of that group+//! (#4949): radio buttons are one group when they are widgets of one field (ISO 32000-2+//! §12.7.5.2.4), and a button's export value is the name of its "on" appearance state.++use std::collections::{HashMap, HashSet};++use super::lifecycle::{drop_from_calculation_order, split_merged, split_triggers};+use super::tree::{detach, list_of};+use super::*;++/// The keys a field that is also its own widget sheds when it becomes only a widget. Its `/AA`+/// keeps only the widget's own triggers.+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"];++/// 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"))+}++/// Refuse what [`join_radio_group`] can't do, before anything is written.+pub(super) fn check_join(doc: &Document, f: &Field, target: &Field) -> Result<(), FormError> {+ for x in [f, target] {+ if has_opt(doc, x.obj) {+ return invalid(format!("{} lists its export values in /Opt, so its buttons can't move between groups", x.name));+ }+ if x.widgets.is_empty() {+ return invalid(format!("{} has no button", x.name));+ }+ }+ if target.locked() {+ return invalid(format!("{} is locked: unlock it first", target.name));+ }+ Ok(())+}++/// Move every button of radio field `f` into radio field `target`. Each keeps its export value+/// unless a button of `target` already has it; then it takes the first free "ChoiceN", as a+/// button added to a group does, and buttons of `f` that shared a value still share the new one. A moved button shows selected only if it is `target`'s+/// 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()+ && only.obj == target.obj+ {+ split_merged(doc, target, only)?;+ }+ 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)?;+ let mut kids = Vec::with_capacity(f.widgets.len());+ for w in &f.widgets {+ let mut d = doc.get(w.obj).as_dict().cloned().unwrap_or_default();+ if w.obj == f.obj {+ for k in FIELD_ONLY_KEYS {+ d.remove(k);+ }+ let aa = d.get(b"AA").and_then(|a| doc.resolve(a).as_dict().cloned());+ match aa.as_ref().map(split_triggers) {+ Some((Some(widget_aa), _)) => d.set(b"AA".to_vec(), Object::Dict(widget_aa)),+ Some((None, _)) => {+ d.remove(b"AA");+ }+ None => {}+ }+ }+ let mut state = w.on_state.clone();+ if let Some(old) = w.on_state.as_deref()+ && let Some(fresh) = renamed.get(old)+ && rename_state(doc, &mut d, old, fresh)+ {+ state = Some(fresh.clone());+ }+ let on = state.is_some() && state == selected;+ let shown = match (&state, on) {+ (Some(s), true) => Object::Name(crate::name_bytes(s)),+ _ => Object::name("Off"),+ };+ d.set(b"AS".to_vec(), shown);+ d.set(b"Parent".to_vec(), Object::Ref(target.obj));+ doc.set(w.obj, Object::Dict(d));+ kids.push(Object::Ref(w.obj));+ }+ // `/Kids` may be an indirect array: read through it so the group's own buttons stay.+ let mut list = list_of(doc, target.obj, b"Kids");+ list.extend(kids);+ doc.update_dict(target.obj, |d| d.set(b"Kids".to_vec(), Object::Array(list)))?;+ Ok(())+}++/// Rename the appearance state `old` to `new` in the widget dictionary `d` (`/AP` `/N`, `/D`,+/// `/R`). A shared `/AP` or state dictionary is copied into `d` rather than renamed for its other+/// users. Whether `/N` had the state: when it didn't, nothing is changed.+fn rename_state(doc: &Document, d: &mut Dict, old: &str, new: &str) -> bool {+ let (old, new) = (crate::name_bytes(old), crate::name_bytes(new));+ let Some(mut ap) = d.get(b"AP").and_then(|a| doc.resolve(a).as_dict().cloned()) else { return false };+ let states = |ap: &Dict, key: &[u8]| ap.get(key).and_then(|s| doc.resolve(s).as_dict().cloned());+ if !states(&ap, b"N").is_some_and(|n| n.contains(&old)) {+ return false;+ }+ for key in [&b"N"[..], b"D", b"R"] {+ let Some(mut sub) = states(&ap, key) else { continue };+ if let Some(v) = sub.remove(&old) {+ sub.set(new.clone(), v);+ ap.set(key.to_vec(), Object::Dict(sub));+ }+ }+ d.set(b"AP".to_vec(), Object::Dict(ap));+ true+}@@ -215,7 +215,7 @@ fn delete_found_field(doc: &mut Document, f: Field) -> Result<(), FormError> { /// Remove the `/CO` entries `gone` matches, whether the form and the array are held directly or /// by reference.-fn drop_from_calculation_order(doc: &mut Document, gone: impl Fn(ObjRef) -> bool) -> Result<(), FormError> {+pub(super) fn drop_from_calculation_order(doc: &mut Document, gone: impl Fn(ObjRef) -> bool) -> Result<(), FormError> { let Some(root) = doc.root() else { return Ok(()) }; let Some(af_obj) = doc.get(root).as_dict().and_then(|d| d.get(b"AcroForm").cloned()) else { return Ok(()) }; let Some(mut af) = doc.resolve(&af_obj).as_dict().cloned() else { return Ok(()) };@@ -237,7 +237,23 @@ fn drop_from_calculation_order(doc: &mut Document, gone: impl Fn(ObjRef) -> bool } /// 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`.+pub(super) 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.@@ -258,40 +274,7 @@ pub fn duplicate_field(doc: &mut Document, name: &str, pages: &[usize]) -> Resul return Ok(0); } // A merged field/widget becomes a field with one widget kid.- let mut model_ref = model.obj;- if model.obj == f.obj {- let d = doc.get(f.obj).as_dict().cloned().unwrap_or_default();- let mut kid = Dict::new();- for k in WIDGET_KEYS {- if let Some(v) = d.get(k) {- kid.set(k.to_vec(), v.clone());- }- }- 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);- }- d.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(kr)]));- })?;- // The page lists the widget now, not the field.- if let Some(p) = model.page {- let pr = refs[p];- let annots = doc.get(pr).as_dict().and_then(|d| d.get(b"Annots").cloned());- let swap =- |a: Vec<Object>| -> Vec<Object> { a.into_iter().map(|o| if o.as_ref() == Some(f.obj) { Object::Ref(kr) } else { o }).collect() };- match annots {- Some(Object::Ref(ar)) => {- let a = doc.get(ar).as_array().cloned().unwrap_or_default();- doc.set(ar, Object::Array(swap(a)));- }- Some(Object::Array(a)) => doc.update_dict(pr, |d| d.set(b"Annots".to_vec(), Object::Array(swap(a))))?,- _ => {}- }- }- model_ref = kr;- }+ let model_ref = if model.obj == f.obj { split_merged(doc, &f, &model)? } else { model.obj }; let template = doc.get(model_ref).as_dict().cloned().unwrap_or_default(); let mut added = Vec::new(); for p in &targets {@@ -324,3 +307,52 @@ pub fn duplicate_field(doc: &mut Document, name: &str, pages: &[usize]) -> Resul redraw_field(doc, name)?; Ok(added.len()) }++/// Split field `f`, which is also its only widget `model`, into a field and one widget kid that+/// takes the widget's keys and its place in the page's `/Annots`. Returns the kid.+pub(super) fn split_merged(doc: &mut Document, f: &Field, model: &Widget) -> Result<ObjRef, FormError> {+ let refs = page_refs(doc);+ let d = doc.get(f.obj).as_dict().cloned().unwrap_or_default();+ let mut kid = Dict::new();+ for k in WIDGET_KEYS {+ if let Some(v) = d.get(k) {+ 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.+ if let Some(&pr) = model.page.and_then(|p| refs.get(p)) {+ let annots = doc.get(pr).as_dict().and_then(|d| d.get(b"Annots").cloned());+ let swap = |a: Vec<Object>| -> Vec<Object> { a.into_iter().map(|o| if o.as_ref() == Some(f.obj) { Object::Ref(kr) } else { o }).collect() };+ match annots {+ Some(Object::Ref(ar)) => {+ let a = doc.get(ar).as_array().cloned().unwrap_or_default();+ doc.set(ar, Object::Array(swap(a)));+ }+ Some(Object::Array(a)) => doc.update_dict(pr, |d| d.set(b"Annots".to_vec(), Object::Array(swap(a))))?,+ _ => {}+ }+ }+ Ok(kr)+}@@ -187,7 +187,7 @@ pub(super) fn clash(name: &str, other: &str) -> Option<String> { } /// The array `key` of `holder` (`/Fields` of the form, `/Kids` of a field).-fn list_of(doc: &Document, holder: ObjRef, key: &[u8]) -> Vec<Object> {+pub(super) fn list_of(doc: &Document, holder: ObjRef, key: &[u8]) -> Vec<Object> { let d = doc.get(holder).as_dict().cloned().unwrap_or_default(); d.get(key).map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default() }@@ -4577,3 +4577,275 @@ 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"); }++/// Two new radio groups on one page, "Group1" (export `a`) and "Group2" (export `b`).+fn two_radio_groups(a: &str, b: &str) -> Document {+ let mut doc = one_page();+ let radio = |export: &str| NewField::Radio { group: None, export: export.into() };+ assert_eq!(add_field(&mut doc, 0, [50.0, 600.0, 64.0, 614.0], &radio(a), None).unwrap(), "Group1");+ assert_eq!(add_field(&mut doc, 0, [80.0, 600.0, 94.0, 614.0], &radio(b), None).unwrap(), "Group2");+ doc+}++fn rename_to(doc: &mut Document, from: &str, to: &str) -> Result<String, FormError> {+ set_props(doc, from, &FieldProps { name: Some(to.into()), ..FieldProps::default() })+}++fn page_widgets(doc: &Document) -> Vec<ObjRef> {+ let page = page_refs(doc)[0];+ let annots = doc.get(page).as_dict().unwrap().get(b"Annots").cloned().unwrap();+ doc.resolve(&annots).as_array().unwrap().iter().filter_map(|o| o.as_ref()).collect()+}++/// #4949: a radio button given its group's name is one of the group's buttons, so the group can+/// be built up by naming buttons alike, as in Acrobat.+#[test]+fn a_radio_button_renamed_to_a_groups_name_joins_the_group() {+ let mut doc = two_radio_groups("Yes", "No");+ set_value(&mut doc, "Group1", &FieldValue::Radio(Some("Yes".into()))).unwrap();+ let before = page_widgets(&doc);+ assert_eq!(rename_to(&mut doc, "Group2", "Group1").unwrap(), "Group1");+ assert_eq!(page_widgets(&doc), before, "the buttons stay on the page where they were");+ let mut doc = reopen(&doc);+ let all = fields(&doc);+ assert_eq!(all.iter().map(|f| f.name.as_str()).collect::<Vec<_>>(), ["Group1"]);+ let group = field(&all, "Group1");+ assert_eq!(group.widgets.iter().filter_map(|w| w.on_state.clone()).collect::<Vec<_>>(), ["Yes", "No"]);+ assert_eq!(group.value, ["Yes"], "the group keeps its value");+ assert_eq!(group.widgets.iter().map(|w| w.state.clone()).collect::<Vec<_>>(), [Some("Yes".into()), Some("Off".into())]);+ assert_eq!(page_widgets(&doc), group.widgets.iter().map(|w| w.obj).collect::<Vec<_>>());+ // Choosing the moved button turns the other off: they are one group.+ set_value(&mut doc, "Group1", &FieldValue::Radio(Some("No".into()))).unwrap();+ let group = field(&fields(&doc), "Group1").clone();+ assert_eq!(group.widgets.iter().map(|w| w.state.clone()).collect::<Vec<_>>(), [Some("Off".into()), Some("No".into())]);+}++/// A joining button whose export value a button of the group already has takes the next free+/// "ChoiceN", in its appearance states too, so the two can be chosen apart.+#[test]+fn a_joining_radio_button_with_a_taken_export_value_takes_a_free_one() {+ let mut doc = two_radio_groups("Choice1", "Choice1");+ rename_to(&mut doc, "Group2", "Group1").unwrap();+ let doc = reopen(&doc);+ let all = fields(&doc);+ let group = field(&all, "Group1");+ assert_eq!(group.widgets.iter().filter_map(|w| w.on_state.clone()).collect::<Vec<_>>(), ["Choice1", "Choice2"]);+ let moved = doc.get(group.widgets[1].obj).as_dict().unwrap().clone();+ let normal = doc.resolve(moved.get(b"AP").unwrap()).as_dict().unwrap().get(b"N").cloned().unwrap();+ let normal = doc.resolve(&normal).as_dict().unwrap().clone();+ assert!(normal.contains(b"Choice2") && !normal.contains(b"Choice1"));+}++/// A group whose `/Kids` is an indirect array keeps its own buttons when another joins it.+#[test]+fn a_group_with_indirect_kids_keeps_its_buttons_when_joined() {+ let mut doc = two_radio_groups("Yes", "No");+ let group = field(&fields(&doc), "Group1").obj;+ let kids = doc.get(group).as_dict().unwrap().get(b"Kids").cloned().unwrap();+ let held = doc.add(kids);+ doc.update_dict(group, |d| d.set(b"Kids".to_vec(), Object::Ref(held))).unwrap();+ rename_to(&mut doc, "Group2", "Group1").unwrap();+ let doc = reopen(&doc);+ let all = fields(&doc);+ assert_eq!(field(&all, "Group1").widgets.iter().filter_map(|w| w.on_state.clone()).collect::<Vec<_>>(), ["Yes", "No"]);+}++/// Buttons of the joining group that share an export value (a button repeated on another page,+/// on together) still share it after the join, under one fresh name.+#[test]+fn joining_radio_buttons_that_share_a_value_keep_sharing_it() {+ let mut doc = two_radio_groups("Choice1", "Choice1");+ let pages = doc.get(doc.root().unwrap()).as_dict().unwrap().reference(b"Pages").unwrap();+ let mut p = pdfcraft_cos::Dict::new();+ p.set(b"Type".to_vec(), Object::name("Page"));+ p.set(b"Parent".to_vec(), Object::Ref(pages));+ p.set(b"MediaBox".to_vec(), Object::Array(vec![0.into(), 0.into(), 600.into(), 800.into()]));+ let r = doc.add(Object::Dict(p));+ doc.update_dict(pages, |d| {+ let mut kids = d.get(b"Kids").unwrap().as_array().unwrap().clone();+ kids.push(Object::Ref(r));+ d.set(b"Count".to_vec(), Object::Int(kids.len() as i64));+ d.set(b"Kids".to_vec(), Object::Array(kids));+ })+ .unwrap();+ assert_eq!(duplicate_field(&mut doc, "Group2", &[1]).unwrap(), 1);+ rename_to(&mut doc, "Group2", "Group1").unwrap();+ let doc = reopen(&doc);+ let all = fields(&doc);+ let group = field(&all, "Group1");+ assert_eq!(group.widgets.iter().filter_map(|w| w.on_state.clone()).collect::<Vec<_>>(), ["Choice1", "Choice2", "Choice2"]);+}++/// 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 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));+ 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 d = pdfcraft_cos::Dict::new();+ d.set(b"Type".to_vec(), Object::name("Annot"));+ d.set(b"Subtype".to_vec(), Object::name("Widget"));+ d.set(b"FT".to_vec(), Object::name("Btn"));+ d.set(b"Ff".to_vec(), Object::Int(i64::from(flags::RADIO)));+ d.set(b"T".to_vec(), pdfcraft_cos::PdfString::text(name));+ d.set(b"Rect".to_vec(), Object::Array(vec![x.into(), 600.into(), (x + 14.0).into(), 614.into()]));+ d.set(b"P".to_vec(), Object::Ref(page));+ d.set(b"AP".to_vec(), Object::Dict(ap));+ d.set(b"AS".to_vec(), Object::name("Off"));+ let r = doc.add(Object::Dict(d));+ doc.update_dict(af, |a| {+ let mut list = a.get(b"Fields").and_then(|l| l.as_array().cloned()).unwrap_or_default();+ list.push(Object::Ref(r));+ a.set(b"Fields".to_vec(), Object::Array(list));+ })+ .unwrap();+ 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);+ assert_eq!(all.iter().map(|f| f.name.as_str()).collect::<Vec<_>>(), ["solo"]);+ let group = field(&all, "solo");+ assert_eq!(group.widgets.iter().filter_map(|w| w.on_state.clone()).collect::<Vec<_>>(), ["A", "B"]);+ // The joined field's widget became a kid; the moved one shed its field entries.+ let kids: Vec<ObjRef> = group.widgets.iter().map(|w| w.obj).collect();+ assert!(!kids.contains(&group.obj));+ let moved = doc.get(group.widgets[1].obj).as_dict().unwrap().clone();+ assert!(!moved.contains(b"T") && !moved.contains(b"FT") && moved.reference(b"Parent") == Some(group.obj));+ let mut on_page = page_widgets(&doc);+ on_page.sort();+ let mut kids = kids;+ kids.sort();+ 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"));+}++/// A joining button that was its own field keeps its widget triggers and drops the field's.+#[test]+fn a_joining_merged_button_sheds_its_fields_triggers() {+ let mut doc = merged_radios(&[("solo", "A", 50.0), ("other", "B", 80.0)]);+ let other = field(&fields(&doc), "other").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"U".to_vec(), js("up();"));+ aa.set(b"V".to_vec(), js("validate();"));+ aa.set(b"C".to_vec(), js("calc();"));+ let aa = doc.add(Object::Dict(aa));+ doc.update_dict(other, |d| d.set(b"AA".to_vec(), Object::Ref(aa))).unwrap();+ rename_to(&mut doc, "other", "solo").unwrap();+ let doc = reopen(&doc);+ let group = field(&fields(&doc), "solo").clone();+ let moved = group.widgets.iter().find(|w| w.on_state.as_deref() == Some("B")).unwrap();+ let wd = doc.get(moved.obj).as_dict().unwrap().clone();+ let widget_aa = doc.resolve(wd.get(b"AA").unwrap()).as_dict().unwrap().clone();+ assert!(widget_aa.contains(b"U") && !widget_aa.contains(b"V") && !widget_aa.contains(b"C"), "only the widget's trigger stays");+ let mut once = merged_radios(&[("solo", "A", 50.0), ("other", "B", 80.0)]);+ let other = field(&fields(&once), "other").obj;+ let mut aa = pdfcraft_cos::Dict::new();+ aa.set(b"V".to_vec(), js("validate();"));+ once.update_dict(other, |d| d.set(b"AA".to_vec(), Object::Dict(aa))).unwrap();+ rename_to(&mut once, "other", "solo").unwrap();+ let once = reopen(&once);+ let group = field(&fields(&once), "solo").clone();+ let moved = group.widgets.iter().find(|w| w.on_state.as_deref() == Some("B")).unwrap();+ let wd = once.get(moved.obj).as_dict().unwrap().clone();+ assert!(!wd.contains(b"AA"), "a widget left with no trigger has no /AA");+}++/// 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]+fn joining_by_name_is_only_for_radio_buttons_and_unlocked_groups() {+ let mut doc = two_radio_groups("Yes", "No");+ add_field(&mut doc, 0, [50.0, 700.0, 250.0, 720.0], &NewField::Text { multiline: false }, Some("note")).unwrap();+ assert!(rename_to(&mut doc, "note", "Group1").is_err(), "a text field doesn't join a radio group");+ assert!(rename_to(&mut doc, "Group2", "note").is_err(), "a radio button doesn't join a text field");+ set_props(&mut doc, "Group1", &FieldProps { locked: Some(true), ..FieldProps::default() }).unwrap();+ assert!(rename_to(&mut doc, "Group2", "Group1").is_err(), "a locked group");+ assert_eq!(fields(&doc).len(), 3);+}@@ -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() {@@ -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;@@ -4847,8 +4847,8 @@ milestone = "M6" status = "shipped" commands = ["form.add.radio"] tools = ["form_add_field"]-evidence = ["crates/forms/src/tests.rs::every_field_type_can_be_added_named_and_drawn", "crates/automation/tests/it/automation.rs::preparing_a_form_through_tools"]-notes = "new group Group1… or join the selected group with the next ChoiceN"+evidence = ["crates/forms/src/tests.rs::every_field_type_can_be_added_named_and_drawn", "crates/automation/tests/it/automation.rs::preparing_a_form_through_tools", "crates/forms/src/tests.rs::a_radio_button_renamed_to_a_groups_name_joins_the_group", "crates/forms/src/tests.rs::a_joining_radio_button_with_a_taken_export_value_takes_a_free_one", "crates/forms/src/tests.rs::radio_fields_that_are_their_own_widget_join_and_are_joined", "crates/automation/tests/it/automation.rs::renaming_a_radio_button_to_a_groups_name_joins_the_group_through_tools"]+notes = "new group Group1… or join the selected group with the next ChoiceN; renaming a radio button (Properties or form_set_props) to another radio group's name moves its buttons into that group (#4949), a clashing export value becoming the next free ChoiceN; refused when either group lists its values in /Opt; a button's export value is not yet editable in Properties" [[feature]] id = "form.check-box-style"