fix(forms): a read-only radio button is not picked or left through set_value (#6038)

* fix(forms): a read-only radio button is not picked or left through set_value

* address review: use /V as the selection in the read-only radio check, keep reset going

* address review: radio read-only checks judge each button by the state it would get, and cover FieldValue::Widget
This commit is contained in:
Dylan M. Taylor authored and GitHub committed 2026-11-11 14:48:24 -05:00
1 parent ee367a4a1c
commit 1e91e8a93e
2 files changed
+117 -10

No files matched your search

+62
View File
@@ -6802,6 +6802,48 @@ fn locked_button_export_edits_leave_the_document_unchanged() {
}
}
/// A radio button that is read-only on its own (its own `/Ff`) can't be picked, and a selected one
/// can't be switched away from, though the group as a whole is writable.
#[test]
fn a_read_only_radio_button_is_neither_picked_nor_left() {
let lock = |doc: &mut Document, num: u32| doc.update_dict(pdfcraft_cos::ObjRef::new(num, 0), |d| d.set(b"Ff".to_vec(), Object::Int(1))).unwrap();
let mut doc = radio_group_form();
lock(&mut doc, 7); // Medium
assert!(matches!(set_value(&mut doc, "size", &FieldValue::Radio(Some("M".into()))), Err(FormError::ReadOnly(_))));
set_value(&mut doc, "size", &FieldValue::Radio(Some("L".into()))).expect("an unlocked button is picked");
// Small (selected) locked: the group stays on it.
let mut doc = radio_group_form();
lock(&mut doc, 6);
assert!(matches!(set_value(&mut doc, "size", &FieldValue::Radio(Some("L".into()))), Err(FormError::ReadOnly(_))));
assert_eq!(field(&fields(&doc), "size").value, ["S"]);
}
/// The group's `/V` names its selection even when the button's `/AS` says nothing: a read-only
/// button that `/V` selects is not switched away from either.
#[test]
fn a_read_only_radio_button_selected_by_the_group_value_is_not_left() {
let mut doc = radio_group_form();
doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| {
d.set(b"Ff".to_vec(), Object::Int(1));
d.set(b"AS".to_vec(), Object::name("Off"));
})
.unwrap();
assert!(matches!(set_value(&mut doc, "size", &FieldValue::Radio(Some("L".into()))), Err(FormError::ReadOnly(_))));
assert_eq!(field(&fields(&doc), "size").value, ["S"]);
}
/// Clearing the form skips a group held by a read-only button and still clears the rest.
#[test]
fn clearing_the_form_goes_on_past_a_group_held_by_a_read_only_button() {
let mut doc = radio_group_form();
doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| d.set(b"Ff".to_vec(), Object::Int(1))).unwrap();
doc.update_dict(pdfcraft_cos::ObjRef::new(9, 0), |d| d.set(b"DV".to_vec(), pdfcraft_cos::PdfString::text("Bob"))).unwrap();
assert_eq!(reset(&mut doc, None).unwrap(), 1);
let all = fields(&doc);
assert_eq!(field(&all, "size").value, ["S"]);
assert_eq!(field(&all, "name").value, ["Bob"]);
}
#[test]
/// Cosmetic field updates skip calculations, while a value write still triggers the chain.
fn script_cosmetic_mutations_do_not_run_the_calculation_chain() {
@@ -6937,6 +6979,26 @@ fn radio_buttons_without_as_sharing_the_value_show_one_on() {
assert_eq!(on_widgets(&doc, "r").0, [false, true, false]);
}
/// Two buttons share the on state `Yes`; the first is on by its `/AS` and the second is read-only
/// and off. `/V` is `Yes` for both, but only the first is selected: picking another option switches
/// the first off, which is allowed, and the read-only one never changes.
#[test]
fn a_read_only_button_sharing_the_on_state_of_the_selected_one_does_not_block() {
let mut doc = radio_without_as(49152, "/Ff 1 /AS /Off");
doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| d.set(b"AS".to_vec(), Object::name("Yes"))).unwrap();
set_value(&mut doc, "r", &FieldValue::Radio(Some("No".into()))).expect("the writable selected button may be switched off");
assert_eq!(on_widgets(&doc, "r").0, [false, false, true]);
}
/// Selecting another button through `FieldValue::Widget` can't switch a read-only selected one off.
#[test]
fn a_read_only_selected_radio_button_is_not_left_by_picking_a_widget() {
let mut doc = radio_group_form();
doc.update_dict(pdfcraft_cos::ObjRef::new(6, 0), |d| d.set(b"Ff".to_vec(), Object::Int(1))).unwrap();
assert!(matches!(set_value(&mut doc, "size", &FieldValue::Widget(2)), Err(FormError::ReadOnly(_))));
assert_eq!(field(&fields(&doc), "size").value, ["S"]);
}
/// Table 227, ReadOnly: a processor "shall not allow a user to change the value". A button whose own
/// dictionary sets it can't be turned on by number.
#[test]
+55 -10
View File
@@ -356,6 +356,9 @@ fn write_value_impl(doc: &mut Document, f: &Field, value: &FieldValue, scripts:
let Some(state) = f.widgets.get(*i).and_then(|w| w.on_state.clone()) else {
return invalid(format!("{:?} has no button {} with an on state", f.name, i.saturating_add(1)));
};
if f.kind == FieldKind::Radio && radio_change_blocked(f, Some(&state), Some(*i)) {
return Err(FormError::ReadOnly(f.name.clone()));
}
set_states(doc, f, Some(&state), Some(*i))
}
(FieldKind::CheckBox, v) => {
@@ -406,6 +409,9 @@ fn write_value_impl(doc: &mut Document, f: &Field, value: &FieldValue, scripts:
if choice.is_none() && f.has(flags::NO_TOGGLE_TO_OFF) && !f.value.is_empty() {
return invalid(format!("{:?} must keep one option selected", f.name));
}
if radio_change_blocked(f, choice.as_deref(), None) {
return Err(FormError::ReadOnly(f.name.clone()));
}
set_states(doc, f, choice.as_deref(), None)
}
(FieldKind::Combo | FieldKind::List, FieldValue::Choice(_) | FieldValue::Text(_) | FieldValue::ChoiceIndices(_)) => {
@@ -541,17 +547,49 @@ pub(crate) fn scroll_widgets(doc: &mut Document, f: &Field, picked: &[usize], co
Ok(())
}
/// The one widget of an exclusive radio group that turns on when `on` is chosen (`pick` when it has
/// that state, else the one already on, else the first); `None` when `on` is `None` or the group
/// turns every widget with that state on together.
fn chosen_widget(f: &Field, on: Option<&str>, pick: Option<usize>) -> Option<usize> {
let exclusive = f.kind == FieldKind::Radio && !f.has(flags::RADIOS_IN_UNISON);
on.filter(|_| exclusive).and_then(|c| {
let has = |i: &usize| f.widgets.get(*i).is_some_and(|w| w.on_state.as_deref() == Some(c));
let n = f.widgets.len();
pick.filter(has).or_else(|| (0..n).filter(has).find(|&i| f.widget_on(i))).or_else(|| (0..n).find(has))
})
}
/// Whether setting a radio group to `on` (through widget `pick`, when the caller chose a widget)
/// would turn a read-only button on or off: such a button can't be picked, and a selected one can't
/// be switched off by picking another. Judged per widget, by the state `set_states` would give it,
/// so buttons sharing an on-state name are told apart by their `/AS`; the group's `/V` stands for
/// the selection only when no button's `/AS` says which is on.
fn radio_change_blocked(f: &Field, on: Option<&str>, pick: Option<usize>) -> bool {
let chosen = chosen_widget(f, on, pick);
let before = |i: usize, w: &Widget| {
let Some(s) = w.on_state.as_deref() else { return false };
if f.widget_on(i) {
return true;
}
// No button sharing this on state is on by its `/AS` and the group's `/V` names it: the
// first of them holds the selection, whatever its own `/AS` says (an `Off` among them).
let sharing = |x: &Widget| x.on_state.as_deref() == Some(s);
f.value.first().map(String::as_str) == Some(s)
&& !f.widgets.iter().any(|x| sharing(x) && x.state.as_deref() == Some(s))
&& f.widgets.iter().position(sharing) == Some(i)
};
f.widgets.iter().enumerate().any(|(i, w)| {
let after = on.is_some() && w.on_state.as_deref() == on && chosen.is_none_or(|k| k == i);
w.read_only && before(i, w) != after
})
}
/// Check boxes and radio buttons: `/V` on the field, `/AS` on each widget. The widgets whose on
/// state is `on` turn on together in a check box (ISO 32000-2 §12.7.5.2.3, EXAMPLE 2) and in a
/// radio group with RadiosInUnison. Without it, at most one radio button is on (Table 229,
/// §12.7.5.2.3): widget `pick` when it has that state, else the one already on, else the first.
fn set_states(doc: &mut Document, f: &Field, on: Option<&str>, pick: Option<usize>) -> Result<(), FormError> {
let exclusive = f.kind == FieldKind::Radio && !f.has(flags::RADIOS_IN_UNISON);
let chosen = on.filter(|_| exclusive).and_then(|c| {
let has = |i: &usize| f.widgets.get(*i).is_some_and(|w| w.on_state.as_deref() == Some(c));
let n = f.widgets.len();
pick.filter(has).or_else(|| (0..n).filter(has).find(|&i| f.widget_on(i))).or_else(|| (0..n).find(has))
});
let chosen = chosen_widget(f, on, pick);
// `/V` names the on state, the same for every widget that has it: which of them is on is
// their `/AS` alone (§12.7.5.2.4).
let v = on.unwrap_or("Off");
@@ -919,10 +957,17 @@ pub fn reset_with_formatting(
// A reset is not blocked by NoToggleToOff: go through the writer directly.
let mut tmp = f.clone();
tmp.flags &= !flags::NO_TOGGLE_TO_OFF;
write_value_impl(doc, &tmp, &v, scripts, true)?;
changed += 1;
if matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List) {
redraw_fields.insert(f.obj);
match write_value_impl(doc, &tmp, &v, scripts, true) {
Ok(()) => {
changed += 1;
if matches!(f.kind, FieldKind::Text | FieldKind::Combo | FieldKind::List) {
redraw_fields.insert(f.obj);
}
}
// a read-only button of the group holds the value: this field is left as it is, and the
// rest of the form is still cleared
Err(FormError::ReadOnly(_)) => {}
Err(e) => return Err(e),
}
}
let calculation_runner: &mut dyn Scripts = if calculate_with_scripts { scripts } else { &mut NoScripts };