From 0cd6a197e40e31b1c0dd0cb84b047f6df1e295be Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 20:47:34 +0000 Subject: [PATCH] M6: a radio button without its /Parent link leaves its group's /Kids too Deleting one widget took it out of the field tree through its /Parent, which a damaged file can leave out or point at another field; the group kept the widget in its /Kids with no page showing it, and the delete reported success. The field that owns the widget drops it from its own /Kids as well (CodeRabbit on #4631). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01GbuSKAS2k5248LGFEapMUR --- crates/forms/src/author/lifecycle.rs | 9 +++++++++ crates/forms/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/crates/forms/src/author/lifecycle.rs b/crates/forms/src/author/lifecycle.rs index 4fb045d7b8..d5d7d14cdd 100644 --- a/crates/forms/src/author/lifecycle.rs +++ b/crates/forms/src/author/lifecycle.rs @@ -219,6 +219,15 @@ pub fn delete_widget(doc: &mut Document, name: &str, widget: usize) -> Result<() drop_from_calculation_order(doc, |r| r == w.obj)?; let mut d = doc.get(f.obj).as_dict().cloned().unwrap_or_default(); let mut changed = false; + // The field's own `/Kids` is what made the widget its: `detach` follows the widget's + // `/Parent`, which a damaged file can leave out or point elsewhere, so the field would keep + // a widget no page shows. + if let Some(kids) = d.get(b"Kids").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()) + && kids.iter().any(|k| k.as_ref() == Some(w.obj)) + { + d.set(b"Kids".to_vec(), Object::Array(kids.into_iter().filter(|k| k.as_ref() != Some(w.obj)).collect())); + changed = true; + } if per_widget_opt && let Some(opt) = d.get(b"Opt").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()) && opt.len() == f.widgets.len() diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index b85aeb726c..a0d57a1b5f 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -4693,6 +4693,29 @@ fn deleting_one_copy_of_a_field_keeps_the_field_and_its_value() { assert_eq!(annots(&doc), [6, 7, 8]); } +#[test] +fn deleting_a_radio_button_without_its_parent_link_takes_it_out_of_the_group() { + // A damaged file: the group lists Medium in its /Kids, but Medium has no /Parent. + for parent in [None, Some(Object::Ref(ObjRef::new(9, 0)))] { + let mut doc = radio_group_form(); + let medium = ObjRef::new(7, 0); + let mut w = doc.get(medium).as_dict().cloned().unwrap(); + w.remove(b"Parent"); + if let Some(p) = parent.clone() { + w.set(b"Parent".to_vec(), p); + } + doc.set(medium, Object::Dict(w)); + delete_widget(&mut doc, "size", 1).unwrap(); + let doc = reopen(&doc); + let all = fields(&doc); + assert_eq!(field(&all, "size").widgets.iter().map(|w| w.obj.num).collect::>(), [6, 8], "/Parent {parent:?}"); + let kids = doc.resolve(doc.get(ObjRef::new(5, 0)).as_dict().unwrap().get(b"Kids").unwrap()).as_array().cloned().unwrap(); + assert!(!kids.iter().any(|k| k.as_ref() == Some(medium)), "/Parent {parent:?}: {kids:?}"); + assert_eq!(field(&all, "name").widgets.len(), 2, "/Parent {parent:?}: the other field is untouched"); + assert_eq!(annots(&doc), [6, 8, 10, 11]); + } +} + #[test] fn deleting_a_widget_that_is_not_there_changes_nothing() { let mut doc = radio_group_form();