From 6e0eec60e92601b73400f92f5869886a6eb8ea58 Mon Sep 17 00:00:00 2001 From: KenM76 Date: Tue, 27 Oct 2026 22:53:37 -0400 Subject: [PATCH] C011: sorting keeps which of two items with one export value is selected Sorting a field's own items carries its /I positions through the sort (ISO 32000-2 12.7.5.4) instead of re-deriving them from /V, which names only the export value. Values no carried position covers still take their first position. items_to_sort and write_sorted_opt keep set_props under the complexity limit. Test: sorting_keeps_which_of_two_items_with_one_export_value_is_selected (fails before: /I [0], Alpha, instead of [2], Zeta). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01SqmHYx8FiWM5PcsoiErgp9 --- crates/forms/src/author/edit.rs | 79 ++++++++++++++++++++++++--------- crates/forms/src/tests.rs | 18 ++++++++ 2 files changed, 75 insertions(+), 22 deletions(-) diff --git a/crates/forms/src/author/edit.rs b/crates/forms/src/author/edit.rs index d163923e78..7f63aa6118 100644 --- a/crates/forms/src/author/edit.rs +++ b/crates/forms/src/author/edit.rs @@ -15,10 +15,17 @@ fn swapped_around_center(r: [f64; 4]) -> Option<[f64; 4]> { } /// 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 { - let mut indices: Vec = values.iter().filter_map(|v| exports.iter().position(|e| e == v)).collect(); +/// replaced (ISO 32000-2 §12.7.5.4): the positions of the selected `values` still offered. `kept` +/// are the old `/I` positions carried into the new list, which tell apart options sharing an +/// export value; a selected value none of them covers takes its first position, by `set_value`'s +/// rule. Returns them. +fn repoint_selection(d: &mut Dict, values: &[String], exports: &[String], kept: &[usize]) -> Vec { + let mut indices: Vec = kept.iter().copied().filter(|&i| exports.get(i).is_some_and(|e| values.contains(e))).collect(); + for v in values { + if !indices.iter().any(|&i| exports.get(i) == Some(v)) { + indices.extend(exports.iter().position(|e| e == v)); + } + } indices.sort_unstable(); indices.dedup(); if indices.is_empty() { @@ -29,6 +36,46 @@ fn repoint_selection(d: &mut Dict, values: &[String], exports: &[String]) -> Vec indices } +/// The positions in `/Opt` that a choice field's `/I` holds, those below `count`. +fn selected_indices(doc: &Document, field: ObjRef, count: usize) -> Vec { + let Some(i) = doc.get(field).as_dict().and_then(|d| d.get(b"I").cloned()) else { + return Vec::new(); + }; + let positions = doc.resolve(&i); + positions.as_array().map(|a| a.iter().filter_map(|o| usize::try_from(o.as_int()?).ok()).filter(|&n| n < count).collect()).unwrap_or_default() +} + +/// The items (export value, display text) "Sort items" orders: the new `props.options`, or else +/// the field's own, whose selection, `/I` included, moves with them. +fn items_to_sort(doc: &Document, f: &Field, props: &FieldProps) -> (Vec<(String, String)>, Vec) { + match &props.options { + Some(o) => (o.iter().map(|x| (x.clone(), x.clone())).collect(), Vec::new()), + None => (f.options.clone(), selected_indices(doc, f.obj, f.options.len())), + } +} + +/// Sort a choice field's items (export value, display text) by display text, ignoring case, and +/// write them as `/Opt`; export values stay with their items. Returns the export values in their +/// new order and the `old` positions carried through the sort. +fn write_sorted_opt(d: &mut Dict, items: &[(String, String)], old: &[usize]) -> (Vec, Vec) { + let mut order: Vec = (0..items.len()).collect(); + order.sort_by_key(|&i| items.get(i).map(|(_, display)| display.to_lowercase())); + let sorted: Vec<&(String, String)> = order.iter().filter_map(|&i| items.get(i)).collect(); + let opt = sorted + .iter() + .map(|(e, disp)| { + if e == disp { + Object::String(PdfString::text(disp)) + } else { + Object::Array(vec![Object::String(PdfString::text(e)), Object::String(PdfString::text(disp))]) + } + }) + .collect(); + d.set(b"Opt".to_vec(), Object::Array(opt)); + let carried = old.iter().filter_map(|o| order.iter().position(|i| i == o)).collect(); + (sorted.into_iter().map(|(e, _)| e.clone()).collect(), carried) +} + /// Change a field's properties (General and Options tabs) and redraw it. pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result { if props.font_size.is_some_and(|size| !size.is_finite() || size < 0.0) { @@ -137,6 +184,7 @@ pub fn set_props(doc: &mut Document, name: &str, props: &FieldProps) -> Result Result> = None; + let mut kept = Vec::new(); 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) { - let mut items: Vec<(String, String)> = match &props.options { - Some(o) => o.iter().map(|x| (x.clone(), x.clone())).collect(), - None => f.options.clone(), - }; - items.sort_by_key(|(_, display)| display.to_lowercase()); - let opt = items - .iter() - .map(|(e, disp)| { - if e == disp { - Object::String(PdfString::text(disp)) - } else { - Object::Array(vec![Object::String(PdfString::text(e)), Object::String(PdfString::text(disp))]) - } - }) - .collect(); - d.set(b"Opt".to_vec(), Object::Array(opt)); - new_exports = Some(items.into_iter().map(|(e, _)| e).collect()); + let (exports, carried) = write_sorted_opt(d, &sort_items, &old_selection); + new_exports = Some(exports); + kept = carried; } if let Some(exports) = &new_exports && matches!(f.kind, FieldKind::Combo | FieldKind::List) { - list_picked = Some((repoint_selection(d, &f.value, exports), exports.len())); + list_picked = Some((repoint_selection(d, &f.value, exports, &kept), exports.len())); } if let Some(q) = props.quadding { d.set(b"Q".to_vec(), Object::Int(q)); diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index a1b791c4fb..8326f62cb7 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -4746,3 +4746,21 @@ fn sorting_a_combo_boxs_items_repoints_its_selected_index() { assert_eq!(selection_indices(&doc, &combo), Some(vec![2])); assert_eq!(field(&fields(&doc), &combo).value, vec!["Pear".to_string()]); } + +#[test] +fn sorting_keeps_which_of_two_items_with_one_export_value_is_selected() { + 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()], multi: false }, None).unwrap(); + let f = field(&fields(&doc), &list).clone(); + let item = |e: &str, shown: &str| Object::Array(vec![Object::String(PdfString::text(e)), Object::String(PdfString::text(shown))]); + // Zeta and Alpha both export "x"; `/I` says Zeta is the one selected. + doc.update_dict(f.obj, |d| { + d.set(b"Opt".to_vec(), Object::Array(vec![item("x", "Zeta"), item("x", "Alpha"), item("y", "Mid")])); + d.set(b"V".to_vec(), PdfString::text("x")); + d.set(b"I".to_vec(), Object::Array(vec![Object::Int(0)])); + }) + .unwrap(); + set_props(&mut doc, &list, &FieldProps { flags: vec![(flags::SORT, true)], ..FieldProps::default() }).unwrap(); + // Alpha, Mid, Zeta: the selection follows Zeta, not the first item exporting "x". + assert_eq!(selection_indices(&doc, &list), Some(vec![2])); +}