collective
Browse / github.com/storytold/pdfcraft / patch
fix(forms): editing or sorting a list's items keeps its selected indices on the same items
fix(forms): editing or sorting a list's items keeps its selected indices on the same items ## What this changes A list box or combo box stores its selection twice: as values in `/V` and as positions in `/Opt` in `/I` (ISO 32000-2 §12.7.5.4). `set_value` keeps both in step. But `set_props`, which rewrites `/Opt` when the items are edited (Properties → Options, or `form_set_props options`) or sorted ("Sort items"), left `/I` as it was. So `/I` pointed at whatever now sits at the old positions. Example: `/Opt [A B C]`, `/V [B C]`, `/I [1 2]`, then remove A. `/I` stays `[1 2]`, which now means C and the new item D. A reader that uses `/I` to tell which items are selected then shows th
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- KenM76 (imported from pull request #4987, open)
- Base
1dd2737b47a9c1c9562f0af563053d1bf510e4bf- Size
- +224 −20 in 5 files
- Review
- pending
- Shared
- 2026-10-28
- Id
bc73d1a464070ad6d79ca4b3
How it went
No other operator has reported using it yet.
Diff
raw · crates/automation/tests/it/automation.rs crates/forms/src/author/edit.rs crates/forms/src/tests.rs crates/forms/src/values.rs parity/acrobat-features.toml
@@ -9175,3 +9175,30 @@ fn page_render_draws_stamps_without_an_appearance() { let grey = custom.iter().filter(|p| p[0].abs_diff(p[1]) < 24 && p[1].abs_diff(p[2]) < 24).count(); assert!(grey > 100 && grey == custom.len(), "an unknown name is a grey placeholder: {grey} of {}", custom.len()); }++/// `/I` of the top-level field `name` in the saved file `path`.+fn saved_selection_indices(path: &Path, name: &str) -> Option<Vec<i64>> {+ let d = pdfcraft_cos::Document::open(std::sync::Arc::new(std::fs::read(path).unwrap())).unwrap();+ let root = d.get(d.root().unwrap()).as_dict().cloned().unwrap();+ let form = d.resolve(root.get(b"AcroForm").unwrap()).as_dict().cloned().unwrap();+ let all = d.resolve(form.get(b"Fields").unwrap()).as_array().cloned().unwrap();+ let f = all+ .iter()+ .filter_map(|o| d.resolve(o).as_dict().cloned())+ .find(|f| f.get(b"T").and_then(|t| d.resolve(t).as_string().map(|s| s.to_text())).as_deref() == Some(name))+ .unwrap();+ f.get(b"I").map(|i| d.resolve(i).as_array().unwrap().iter().map(|o| o.as_int().unwrap()).collect())+}++#[test]+fn changing_a_lists_options_through_tools_keeps_its_saved_selection_on_the_same_items() {+ let dir = workdir("choice-index");+ let mut a = auto(&dir);+ let doc = ok(&mut a, "doc_open", json!({ "path": "a.pdf" }))["doc"].as_u64().unwrap();+ let args = json!({ "doc": doc, "page": 1, "type": "list", "rect": [20, 60, 180, 140], "options": ["A", "B", "C"], "multi_select": true, "name": "letters" });+ ok(&mut a, "form_add_field", args);+ ok(&mut a, "form_fill", json!({ "doc": doc, "values": { "letters": ["B", "C"] } }));+ ok(&mut a, "form_set_props", json!({ "doc": doc, "field": "letters", "options": ["B", "C", "D"] }));+ ok(&mut a, "doc_save", json!({ "doc": doc, "path": "letters.pdf" }));+ assert_eq!(saved_selection_indices(&dir.join("letters.pdf"), "letters"), Some(vec![0, 1]), "B and C are now items 0 and 1");+}@@ -14,6 +14,21 @@ fn swapped_around_center(r: [f64; 4]) -> Option<[f64; 4]> { (out.iter().all(|v| v.is_finite()) && out[2] - out[0] >= MIN_FIELD_SIDE && out[3] - out[1] >= MIN_FIELD_SIDE).then_some(out) } +/// Re-point a choice field's `/I`, positions in `/Opt`, at the new list `exports` after `/Opt` was+/// replaced (ISO 32000-2 §12.7.5.4): the positions of the selected `values` still offered, by+/// `set_value`'s rule. Returns them.+fn repoint_selection(d: &mut Dict, values: &[String], exports: &[String]) -> Vec<usize> {+ let mut indices: Vec<usize> = values.iter().filter_map(|v| exports.iter().position(|e| e == v)).collect();+ indices.sort_unstable();+ indices.dedup();+ if indices.is_empty() {+ d.remove(b"I");+ } else {+ d.set(b"I".to_vec(), Object::Array(indices.iter().map(|&i| Object::Int(i64::try_from(i).unwrap_or(i64::MAX))).collect()));+ }+ indices+}+ /// Change a field's properties (General and Options tabs) and redraw it. pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result<String, FormError> { if props.font_size.is_some_and(|size| !size.is_finite() || size < 0.0) {@@ -173,6 +188,7 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result<S if props.check_style.is_some() && !matches!(f.kind, FieldKind::CheckBox | FieldKind::Radio) { return invalid("only check boxes and radio buttons have a check style"); }+ let mut list_picked = None; doc.update_dict(f.obj, |d| { d.set(b"Ff".to_vec(), Object::Int(ff as i64)); if let Some(t) = &props.tooltip {@@ -190,8 +206,11 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result<S } } }+ // The export values in `/Opt` order once it is rewritten, for re-pointing `/I`.+ let mut new_exports: Option<Vec<String>> = None; if let Some(opts) = &props.options { d.set(b"Opt".to_vec(), Object::Array(opts.iter().map(|o| Object::String(PdfString::text(o))).collect()));+ new_exports = Some(opts.clone()); } // "Sort items": the list itself is kept in order (export values stay with their items). if ff & flags::SORT != 0 && matches!(f.kind, FieldKind::Combo | FieldKind::List) {@@ -211,6 +230,12 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result<S }) .collect(); d.set(b"Opt".to_vec(), Object::Array(opt));+ new_exports = Some(items.into_iter().map(|(e, _)| e).collect());+ }+ if let Some(exports) = &new_exports+ && matches!(f.kind, FieldKind::Combo | FieldKind::List)+ {+ list_picked = Some((repoint_selection(d, &f.value, exports), exports.len())); } if let Some(q) = props.quadding { d.set(b"Q".to_vec(), Object::Int(q));@@ -385,6 +410,18 @@ 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);+ // A list box scrolls, as for a new value, so its first row is still an option and the+ // selection is in view. Its rows are measured with the edited font size and rectangle.+ if let Some((picked, count)) = list_picked+ && let Some(g) = fields(doc).into_iter().find(|g| g.obj == f.obj)+ {+ if let Some((old, new)) = crate::values::list_scroll(&g, &picked, count)+ && old != new+ {+ doc.update_dict(f.obj, |d| d.set(b"TI".to_vec(), Object::Int(i64::try_from(new).unwrap_or(0))))?;+ }+ crate::values::scroll_widgets(doc, &g, &picked, count)?;+ } redraw_widgets(doc, &new_name, only)?; if props.calculate.is_some() { crate::recalculate(doc)?;@@ -4577,3 +4577,129 @@ 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"); }++/// `/I` of field `name`: the selected positions in `/Opt`, or `None` when absent.+fn selection_indices(doc: &Document, name: &str) -> Option<Vec<i64>> {+ let f = field(&fields(doc), name).clone();+ let d = doc.get(f.obj).as_dict().cloned().unwrap();+ d.get(b"I").map(|i| doc.resolve(i).as_array().unwrap().iter().map(|o| o.as_int().unwrap()).collect())+}++#[test]+fn changing_a_lists_options_repoints_its_selected_indices() {+ let mut doc = one_page();+ let list =+ add_field(&mut doc, 0, [50.0, 600.0, 250.0, 680.0], &NewField::List { options: vec!["A".into(), "B".into(), "C".into()], multi: true }, None)+ .unwrap();+ set_value(&mut doc, &list, &FieldValue::Choice(vec!["C".into(), "B".into()])).unwrap();+ assert_eq!(selection_indices(&doc, &list), Some(vec![1, 2]));+ let f = field(&fields(&doc), &list).clone();+ doc.update_dict(f.obj, |d| d.set(b"TI".to_vec(), Object::Int(2))).unwrap();+ // Removing A moves B and C up one place.+ let opts = |o: &[&str]| FieldProps { options: Some(o.iter().map(|s| s.to_string()).collect()), ..FieldProps::default() };+ set_props(&mut doc, &list, &opts(&["B", "C", "D"])).unwrap();+ assert_eq!(selection_indices(&doc, &list), Some(vec![0, 1]));+ // Removing B leaves only C selected; the top row would point past the end.+ set_props(&mut doc, &list, &opts(&["C"])).unwrap();+ assert_eq!(selection_indices(&doc, &list), Some(vec![0]));+ assert_eq!(top_row(&doc, f.obj), Some(0), "the list scrolls back to its only row");+ // No selected value is offered any more.+ set_props(&mut doc, &list, &opts(&["X", "Y"])).unwrap();+ assert_eq!(selection_indices(&doc, &list), None);+ let doc = reopen(&doc);+ assert_eq!(selection_indices(&doc, &list), None);+}++fn top_row(doc: &Document, obj: ObjRef) -> Option<i64> {+ doc.get(obj).as_dict().and_then(|d| d.get(b"TI")).and_then(|o| o.as_int())+}++#[test]+fn shrinking_a_lists_options_scrolls_a_widget_with_its_own_top_row() {+ let objs: Vec<String> = vec![+ "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(),+ "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(),+ "<< /Type /Page /Parent 2 0 R /Annots [7 0 R] >>".into(),+ "<< /Fields [6 0 R] /DA (/Helv 10 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >>".into(),+ "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into(),+ "<< /FT /Ch /T (pick) /Opt [(A) (B) (C) (D) (E) (F)] /V (F) /I [5] /Kids [7 0 R] >>".into(),+ "<< /Type /Annot /Subtype /Widget /Parent 6 0 R /TI 5 /Rect [50 700 250 720] /P 3 0 R >>".into(),+ ];+ let mut doc = document(&objs);+ let widget = ObjRef::new(7, 0);+ set_props(&mut doc, "pick", &FieldProps { options: Some(vec!["F".into(), "A".into()]), ..FieldProps::default() }).unwrap();+ assert_eq!(selection_indices(&doc, "pick"), Some(vec![0]));+ // The widget's own /TI is read before the field's; row 5 no longer exists.+ assert_eq!(top_row(&doc, widget), Some(0), "the widget scrolls to F, now row 0");+ let doc = reopen(&doc);+ assert_eq!(top_row(&doc, widget), Some(0));+}++#[test]+fn each_widget_with_its_own_top_row_scrolls_by_itself() {+ // Widget 7 shows from the field's top row; widget 8 keeps its own /TI 4.+ let objs = |v: &str| -> Vec<String> {+ vec![+ "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(),+ "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(),+ "<< /Type /Page /Parent 2 0 R /Annots [7 0 R 8 0 R] >>".into(),+ "<< /Fields [6 0 R] /DA (/Helv 10 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >>".into(),+ "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into(),+ format!("<< /FT /Ch /T (pick) /Opt [(A) (B) (C) (D) (E) (F)] {v} /Kids [7 0 R 8 0 R] >>"),+ "<< /Type /Annot /Subtype /Widget /Parent 6 0 R /Rect [50 700 250 740] /P 3 0 R >>".into(),+ "<< /Type /Annot /Subtype /Widget /Parent 6 0 R /TI 4 /Rect [50 600 250 620] /P 3 0 R >>".into(),+ ]+ };+ let (first, other) = (ObjRef::new(7, 0), ObjRef::new(8, 0));+ // The options shrink: the first widget already shows A, the other's row 4 is gone.+ let mut doc = document(&objs("/V (A) /I [0]"));+ set_props(&mut doc, "pick", &FieldProps { options: Some(vec!["A".into(), "B".into()]), ..FieldProps::default() }).unwrap();+ assert_eq!(top_row(&doc, first), None);+ assert_eq!(top_row(&reopen(&doc), other), Some(0), "options: the other widget scrolls to A");+ // A new value the first widget already shows, below the other's view.+ let mut doc = document(&objs("/V (B) /I [1]"));+ set_value(&mut doc, "pick", &FieldValue::Choice(vec!["A".into()])).unwrap();+ assert_eq!(top_row(&doc, ObjRef::new(6, 0)), None, "the field's top row did not move");+ assert_eq!(top_row(&reopen(&doc), other), Some(0), "value: the other widget scrolls to A");+}++/// A list box's top row is chosen with the font size set in the same edit: at the larger size+/// fewer rows fit, so it scrolls further to keep the selection in view.+#[test]+fn a_list_boxs_top_row_follows_a_font_size_changed_with_its_items() {+ let mut doc = document(&[+ "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(),+ "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(),+ "<< /Type /Page /Parent 2 0 R /Annots [6 0 R] >>".into(),+ "<< /Fields [6 0 R] /DA (/Helv 10 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >>".into(),+ "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into(),+ "<< /FT /Ch /T (pick) /Opt [(A) (B) (C) (D) (E)] /V (C) /I [2] /Type /Annot /Subtype /Widget /Rect [50 700 250 740] /P 3 0 R >>".into(),+ ]);+ let items = ["Z", "A", "B", "C", "D"].map(String::from).to_vec();+ set_props(&mut doc, "pick", &FieldProps { options: Some(items), font_size: Some(20.0), ..FieldProps::default() }).unwrap();+ let doc = reopen(&doc);+ let g = field(&fields(&doc), "pick").clone();+ let w = &g.widgets[0];+ let rows = w.list.unwrap();+ let shown = rows.top..rows.top + rows.visible(w.rect[3] - w.rect[1]);+ assert!(shown.contains(&3), "C is row 3; rows {shown:?} show");+}++#[test]+fn sorting_a_combo_boxs_items_repoints_its_selected_index() {+ let mut doc = one_page();+ let combo = add_field(+ &mut doc,+ 0,+ [50.0, 600.0, 250.0, 620.0],+ &NewField::Combo { options: vec!["Pear".into(), "apple".into(), "Fig".into()], editable: false },+ None,+ )+ .unwrap();+ set_value(&mut doc, &combo, &FieldValue::Choice(vec!["Pear".into()])).unwrap();+ assert_eq!(selection_indices(&doc, &combo), Some(vec![0]));+ set_props(&mut doc, &combo, &FieldProps { flags: vec![(flags::SORT, true)], ..FieldProps::default() }).unwrap();+ // apple, Fig, Pear: the selection follows Pear to the end.+ assert_eq!(selection_indices(&doc, &combo), Some(vec![2]));+ assert_eq!(field(&fields(&doc), &combo).value, vec!["Pear".to_string()]);+}@@ -288,14 +288,8 @@ pub(crate) fn write_value(doc: &mut Document, f: &Field, value: &FieldValue, scr indices.sort_unstable_by_key(Object::as_int); // A list box scrolls (`/TI`) so the picked options are among the rows it draws, as // in other viewers: a pick below the last visible row would otherwise not show (#1524).- let top = match (f.kind, f.widgets.first()) {- (FieldKind::List, Some(w)) => w.list.map(|rows| {- let height = if matches!(w.rotation, 90 | 270) { w.rect[2] - w.rect[0] } else { w.rect[3] - w.rect[1] };- let picked: Vec<usize> = indices.iter().filter_map(|i| i.as_int()).filter_map(|i| usize::try_from(i).ok()).collect();- (rows.top, appearance::ListRows::top_showing(rows.top, &picked, rows.visible(height), f.options.len()))- }),- _ => None,- };+ let picked: Vec<usize> = indices.iter().filter_map(|i| i.as_int()).filter_map(|i| usize::try_from(i).ok()).collect();+ let top = list_scroll(f, &picked, f.options.len()); doc.update_dict(f.obj, |d| { match exports.as_slice() { [] => {@@ -315,16 +309,7 @@ pub(crate) fn write_value(doc: &mut Document, f: &Field, value: &FieldValue, scr d.set(b"TI".to_vec(), Object::Int(i64::try_from(new).unwrap_or(0))); } })?;- // A widget with its own `/TI` (read before the field's) scrolls too.- if let Some((old, new)) = top- && old != new- {- for w in f.widgets.iter().filter(|w| w.obj != f.obj) {- if doc.get(w.obj).as_dict().is_some_and(|d| d.contains(b"TI")) {- doc.update_dict(w.obj, |d| d.set(b"TI".to_vec(), Object::Int(i64::try_from(new).unwrap_or(0))))?;- }- }- }+ scroll_widgets(doc, f, &picked, f.options.len())?; redraw(doc, f, &exports, scripts) } (FieldKind::PushButton, _) => invalid(format!("{:?} is a button; it has no value", f.name)),@@ -333,6 +318,35 @@ pub(crate) fn write_value(doc: &mut Document, f: &Field, value: &FieldValue, scr } } +/// A list box's first visible row (`/TI`) before and after scrolling so the `picked` positions of+/// its `count` options are among the rows it draws, as in other viewers: a pick below the last+/// visible row would otherwise not show (#1524). `None` for any other field.+pub(crate) fn list_scroll(f: &Field, picked: &[usize], count: usize) -> Option<(usize, usize)> {+ f.widgets.first().filter(|_| f.kind == FieldKind::List).and_then(|w| widget_scroll(w, picked, count))+}++/// A list box widget's first row before and after scrolling so `picked` shows.+fn widget_scroll(w: &Widget, picked: &[usize], count: usize) -> Option<(usize, usize)> {+ w.list.map(|rows| {+ let height = if matches!(w.rotation, 90 | 270) { w.rect[2] - w.rect[0] } else { w.rect[3] - w.rect[1] };+ (rows.top, appearance::ListRows::top_showing(rows.top, picked, rows.visible(height), count))+ })+}++/// Scroll every widget of `f` with its own `/TI` (read before the field's) by its own rows and+/// size, so `picked` shows in each.+pub(crate) fn scroll_widgets(doc: &mut Document, f: &Field, picked: &[usize], count: usize) -> Result<(), FormError> {+ for w in f.widgets.iter().filter(|w| w.obj != f.obj) {+ if doc.get(w.obj).as_dict().is_some_and(|d| d.contains(b"TI"))+ && let Some((old, new)) = widget_scroll(w, picked, count)+ && old != new+ {+ doc.update_dict(w.obj, |d| d.set(b"TI".to_vec(), Object::Int(i64::try_from(new).unwrap_or(0))))?;+ }+ }+ Ok(())+}+ /// Check boxes and radio buttons: `/V` on the field, `/AS` on each widget. fn set_states(doc: &mut Document, f: &Field, on: Option<&str>) -> Result<(), FormError> { let v = on.unwrap_or("Off");@@ -4991,8 +4991,8 @@ milestone = "M6" status = "shipped" commands = ["form.field.properties"] tools = ["form_set_props"]-evidence = ["crates/forms/src/tests.rs::button_caption_preserves_other_widgets_and_shared_dictionaries", "crates/forms/src/tests.rs::a_button_caption_is_drawn_in_its_fields_font_size_and_color", "crates/ui-egui/tests/it/prepare.rs::button_label_is_editable_without_renaming_the_field", "crates/automation/tests/it/button_caption.rs::button_caption_can_be_changed_after_creation", "crates/forms/src/tests.rs::properties_rename_and_delete", "crates/automation/tests/it/automation.rs::preparing_a_form_through_tools", "crates/forms/src/tests.rs::options_tab_flags_alignment_and_defaults", "crates/ui-egui/tests/it/prepare.rs::options_tab_sets_flags_alignment_and_defaults", "crates/automation/tests/it/form_bulk.rs::bulk_options_apply_per_field_and_refuse_an_invalid_comb_atomically"]-notes = "push buttons: per-widget normal label independent of the field name, including clearing and save/reopen, drawn in the font, size and color of the button's /DA (the form's /DR font, else Helvetica; an automatic size fits the button) and centered by Helvetica's widths; icon-only layout retained, rollover/down labels and icon layout editing remain unavailable; text: alignment, default, multi-line, scroll, rich text flag, limit, password, file selection, spell check, comb; choices: items, default, sort, custom text, multiple selection, commit immediately; check boxes and radios: checked by default, radios in unison. Shared GUI offers applicable text/choice flags, alignment, text limits and check styles; choices/defaults remain individual GUI edits"+evidence = ["crates/forms/src/tests.rs::button_caption_preserves_other_widgets_and_shared_dictionaries", "crates/forms/src/tests.rs::a_button_caption_is_drawn_in_its_fields_font_size_and_color", "crates/ui-egui/tests/it/prepare.rs::button_label_is_editable_without_renaming_the_field", "crates/automation/tests/it/button_caption.rs::button_caption_can_be_changed_after_creation", "crates/forms/src/tests.rs::properties_rename_and_delete", "crates/automation/tests/it/automation.rs::preparing_a_form_through_tools", "crates/forms/src/tests.rs::options_tab_flags_alignment_and_defaults", "crates/ui-egui/tests/it/prepare.rs::options_tab_sets_flags_alignment_and_defaults", "crates/automation/tests/it/form_bulk.rs::bulk_options_apply_per_field_and_refuse_an_invalid_comb_atomically", "crates/forms/src/tests.rs::changing_a_lists_options_repoints_its_selected_indices", "crates/forms/src/tests.rs::shrinking_a_lists_options_scrolls_a_widget_with_its_own_top_row", "crates/forms/src/tests.rs::each_widget_with_its_own_top_row_scrolls_by_itself", "crates/forms/src/tests.rs::a_list_boxs_top_row_follows_a_font_size_changed_with_its_items", "crates/forms/src/tests.rs::sorting_a_combo_boxs_items_repoints_its_selected_index", "crates/automation/tests/it/automation.rs::changing_a_lists_options_through_tools_keeps_its_saved_selection_on_the_same_items"]+notes = "push buttons: per-widget normal label independent of the field name, including clearing and save/reopen, drawn in the font, size and color of the button's /DA (the form's /DR font, else Helvetica; an automatic size fits the button) and centered by Helvetica's widths; icon-only layout retained, rollover/down labels and icon layout editing remain unavailable; text: alignment, default, multi-line, scroll, rich text flag, limit, password, file selection, spell check, comb; choices: items (the selected indices /I follow their items when the list is edited or sorted), default, sort, custom text, multiple selection, commit immediately; check boxes and radios: checked by default, radios in unison. Shared GUI offers applicable text/choice flags, alignment, text limits and check styles; choices/defaults remain individual GUI edits" [[feature]] id = "form.props-actions"