M6: list box picks survive same-name twins, repeats, Shift-click and an array /V

- A same-name twin gets the copied /I mapped to its own options by export
  value and text, so options listed in another order keep the option picked;
  one with no match there leaves the twin selecting from /V, never part of /I.
- A position picked twice is written once in /V and /I, so they still match
  item for item.
- Shift-click extends from the row clicked last, kept in the view: with rows
  sharing an export value the value can't say which came last.
- A single-select list loaded from an array /V writes one value when its
  shown row is clicked again.

(review)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv

crates/forms/src/lib.rs

@@ -164,6 +164,10 @@ pub struct Field {
     /// [`selected_options`]). Two options can share an export value, so `value` alone can't say
     /// which of them is selected. Empty for other kinds.
     pub selected: Vec<usize>,
+    /// A list box without MultiSelect whose `/V` is an array (a value of one option is a string,
+    /// ISO 32000-2 Table 233): `value` holds its first entry only, and picking that option again
+    /// writes the value as one string rather than leaving the array.
+    pub single_value_is_array: bool,
     /// Default appearance string (`/DA`, inherited from the form if absent).
     pub da: String,
     /// Quadding: 0 left, 1 centred, 2 right.

crates/forms/src/read.rs

@@ -614,6 +614,7 @@ fn walk(
     // value only, and shows and resets to it; the file keeps its entries until the field is filled.
     let mut default = values_of(doc, inh.dv.as_ref());
     let single = kind == FieldKind::List && ff & flags::MULTI_SELECT == 0;
+    let single_value_is_array = single && inh.v.as_ref().is_some_and(|v| doc.resolve(v).as_array().is_some());
     if single {
         value.truncate(1);
         default.truncate(1);
@@ -635,6 +636,7 @@ fn walk(
         max_len: inh.max_len,
         options,
         selected,
+        single_value_is_array,
         da: inh.da.clone().unwrap_or_else(|| "/Helv 0 Tf 0 g".into()),
         quadding: inh.q.unwrap_or(0),
         tooltip: d.get(b"TU").and_then(|o| text_of(&doc.resolve(o))),

crates/forms/src/tests.rs

@@ -5709,3 +5709,52 @@ fn picking_in_a_single_select_list_given_several_values_writes_one() {
     let pair = field(&fields(&doc), "pair").clone();
     assert_eq!((pair.value, pair.selected), (vec!["JP".to_string()], vec![1]));
 }
+
+/// A position picked twice is written once in both `/V` and `/I`, so they still match item for
+/// item and the option picked (the second of two sharing an export value) comes back on reopen.
+#[test]
+fn a_position_picked_twice_is_written_once_in_v_and_i() {
+    let mut doc = duplicate_exports(1 << 21, "");
+    set_value(&mut doc, "dup", &FieldValue::ChoiceIndices(vec![1, 1])).unwrap();
+    let dup = field(&fields(&doc), "dup").clone();
+    let d = doc.get(dup.obj).as_dict().cloned().unwrap();
+    let v: Vec<String> = match d.get(b"V") {
+        Some(Object::Array(a)) => a.iter().filter_map(|o| o.as_string().map(|s| s.to_text())).collect(),
+        Some(o) => o.as_string().map(|s| vec![s.to_text()]).unwrap_or_default(),
+        None => Vec::new(),
+    };
+    let i: Vec<i64> = d.get(b"I").and_then(|i| i.as_array()).map(|a| a.iter().filter_map(Object::as_int).collect()).unwrap_or_default();
+    assert_eq!((v, i), (vec!["1".to_string()], vec![1]), "one entry each");
+    assert_eq!(dup.selected, [1], "One-B, not One-A");
+}
+
+/// A same-name twin listing the options sharing an export value in another order is given the
+/// option picked, not the one at the same position: `/I` is mapped to the twin's own options.
+#[test]
+fn a_twin_with_options_in_another_order_gets_the_option_picked() {
+    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 [6 0 R 7 0 R] >>".into(),
+        "<< /Fields [6 0 R 7 0 R] /NeedAppearances true /DA (/Helv 10 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >>".into(),
+        "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".into(),
+        "<< /Type /Annot /Subtype /Widget /FT /Ch /T (dup) /Ff 0 /Opt [[(1) (One-A)] [(1) (One-B)] [(2) (Two)]] /Rect [50 700 200 746] /P 3 0 R >>"
+            .into(),
+        "<< /Type /Annot /Subtype /Widget /FT /Ch /T (dup) /Ff 0 /Opt [[(1) (One-B)] [(1) (One-A)] [(2) (Two)]] /Rect [50 600 200 646] /P 3 0 R >>"
+            .into(),
+    ];
+    let mut doc = document(&objs);
+    set_value(&mut doc, "dup", &FieldValue::ChoiceIndices(vec![1])).unwrap();
+    let all = fields(&doc);
+    let twin = all.iter().find(|f| f.obj == pdfcraft_cos::ObjRef::new(7, 0)).unwrap();
+    let i: Vec<i64> = doc
+        .get(twin.obj)
+        .as_dict()
+        .and_then(|d| d.get(b"I").and_then(|i| i.as_array().cloned()))
+        .unwrap_or_default()
+        .iter()
+        .filter_map(Object::as_int)
+        .collect();
+    assert_eq!(i, [0], "One-B is first in the twin's options");
+    assert_eq!(twin.selected, [0]);
+}

crates/forms/src/values.rs

@@ -102,15 +102,21 @@ fn fill(doc: &mut Document, all: &[Field], named: &[usize], name: &str, value: &
     if !twins.is_empty() {
         let taken = fields(doc).into_iter().find(|x| x.obj == f.obj).map(|x| x.value).unwrap_or_default();
         for t in twins {
-            share_value(doc, f.obj, t, &taken, scripts)?;
+            share_value(doc, (f.obj, &f.options), t, &taken, scripts)?;
         }
     }
     Ok(())
 }
 
-/// Give `twin` the value `source` holds: its `/V` (and a choice's `/I`), drawn in the twin's own
-/// widgets.
-fn share_value(doc: &mut Document, source: ObjRef, twin: &Field, value: &[String], scripts: &mut dyn Scripts) -> Result<(), FormError> {
+/// Give `twin` the value `source` (a field and its options) holds: its `/V`, and a choice's `/I`
+/// mapped to the twin's own options, drawn in the twin's own widgets.
+fn share_value(
+    doc: &mut Document,
+    (source, source_options): (ObjRef, &[(String, String)]),
+    twin: &Field,
+    value: &[String],
+    scripts: &mut dyn Scripts,
+) -> Result<(), FormError> {
     match twin.kind {
         FieldKind::CheckBox => {
             let state = twin.widgets.iter().find_map(|w| w.on_state.clone()).unwrap_or_else(|| "Yes".into());
@@ -122,22 +128,35 @@ fn share_value(doc: &mut Document, source: ObjRef, twin: &Field, value: &[String
         }
         FieldKind::Text | FieldKind::Combo | FieldKind::List => {
             let src = doc.get(source).as_dict().cloned().unwrap_or_default();
+            // `/I` holds positions in the source's options: each goes to the twin's first unused option
+            // with the same export value and text, its options perhaps in another order. One with no
+            // match leaves the twin no `/I` (its `/V` picks), never part of one.
+            let positions = match twin.kind {
+                FieldKind::Combo | FieldKind::List => {
+                    listed_indices(doc, &src, source_options.len()).and_then(|picked| twin_positions(&picked, source_options, &twin.options))
+                }
+                _ => None,
+            };
             doc.update_dict(twin.obj, |d| {
-                for key in [&b"V"[..], b"I"] {
-                    match src.get(key) {
-                        Some(v) => d.set(key.to_vec(), v.clone()),
-                        None => {
-                            d.remove(key);
-                        }
+                match src.get(b"V") {
+                    Some(v) => d.set(b"V".to_vec(), v.clone()),
+                    None => {
+                        d.remove(b"V");
+                    }
+                }
+                match &positions {
+                    Some(p) => d.set(b"I".to_vec(), Object::Array(p.iter().map(|&i| Object::Int(i64::try_from(i).unwrap_or(i64::MAX))).collect())),
+                    None => {
+                        d.remove(b"I");
                     }
                 }
                 d.remove(b"RV");
             })?;
-            // Drawn with the options the copied `/I` selects, not only the export values.
+            // Drawn with the options that `/I` selects, not only the export values.
             let mut shown = twin.clone();
             shown.value = value.to_vec();
             if matches!(twin.kind, FieldKind::Combo | FieldKind::List) {
-                shown.selected = selected_options(&twin.options, value, listed_indices(doc, &src, twin.options.len()).as_deref());
+                shown.selected = selected_options(&twin.options, value, positions.as_deref());
             }
             redraw(doc, &shown, value, scripts)
         }
@@ -282,14 +301,19 @@ pub(crate) fn write_value(doc: &mut Document, f: &Field, value: &FieldValue, scr
             // which one is picked (ISO 32000-2 Table 234).
             let mut exports = Vec::new();
             let mut picked: Vec<usize> = Vec::new();
+            // Each option once in both `/V` and `/I`: a reader that finds them differ (a repeat in one)
+            // goes by the export values alone and may pick another option sharing one.
+            let mut seen = vec![false; f.options.len()];
             match value {
                 FieldValue::ChoiceIndices(positions) => {
                     for &i in positions {
                         let Some((e, _)) = f.options.get(i) else {
                             return invalid(format!("{:?} has no option {} (it has {})", f.name, i.saturating_add(1), f.options.len()));
                         };
-                        exports.push(e.clone());
-                        picked.push(i);
+                        if first_pick(&mut seen, i) {
+                            exports.push(e.clone());
+                            picked.push(i);
+                        }
                     }
                 }
                 _ => {
@@ -303,8 +327,10 @@ pub(crate) fn write_value(doc: &mut Document, f: &Field, value: &FieldValue, scr
                     for v in &vals {
                         match f.options.iter().position(|(e, d)| e == v || d == v) {
                             Some(i) => {
-                                exports.push(f.options.get(i).map(|(e, _)| e.clone()).unwrap_or_else(|| v.clone()));
-                                picked.push(i);
+                                if first_pick(&mut seen, i) {
+                                    exports.push(f.options.get(i).map(|(e, _)| e.clone()).unwrap_or_else(|| v.clone()));
+                                    picked.push(i);
+                                }
                             }
                             None if f.kind == FieldKind::Combo && f.has(flags::EDIT) => exports.push(v.clone()),
                             None if f.options.is_empty() => exports.push(v.clone()),
@@ -689,3 +715,30 @@ pub fn reset(doc: &mut Document, names: Option<&[String]>) -> Result<usize, Form
     recalculate(doc)?;
     Ok(changed)
 }
+
+/// Marks option `i` picked; false when it already was, or there is no such option.
+fn first_pick(seen: &mut [bool], i: usize) -> bool {
+    match seen.get_mut(i) {
+        Some(taken) if !*taken => {
+            *taken = true;
+            true
+        }
+        _ => false,
+    }
+}
+
+/// The twin's positions for the source's picked positions `picked`: each picked option goes to
+/// the twin's first not yet taken option with the same export value and text. `None` when one
+/// has no such option there. The positions come back in ascending order, as `/I` lists them.
+fn twin_positions(picked: &[usize], source: &[(String, String)], twin: &[(String, String)]) -> Option<Vec<usize>> {
+    let mut free: std::collections::HashMap<(&str, &str), std::collections::VecDeque<usize>> = std::collections::HashMap::new();
+    for (i, (e, d)) in twin.iter().enumerate() {
+        free.entry((e.as_str(), d.as_str())).or_default().push_back(i);
+    }
+    let mut out = picked
+        .iter()
+        .map(|&p| source.get(p).and_then(|(e, d)| free.get_mut(&(e.as_str(), d.as_str()))).and_then(|q| q.pop_front()))
+        .collect::<Option<Vec<usize>>>()?;
+    out.sort_unstable();
+    Some(out)
+}

crates/ui-egui/src/tools/forms_ui.rs

@@ -73,6 +73,10 @@ pub struct FormView {
     /// The widget holding egui's keyboard focus for the focused check box or radio button, and
     /// whether it takes the arrows (radio buttons). The page keys still work around it.
     pub(crate) keys: Option<(egui::Id, bool)>,
+    /// The list box row clicked last, without Shift: (its field name, the row). A Shift-click picks
+    /// the rows from it to the one clicked; the field's value can't say which row came last when
+    /// rows share an export value, and `/I` lists them in ascending order.
+    pub(crate) list_anchor: Option<(String, usize)>,
 }
 
 /// A form message for the app to show, with document data kept separate so the visible
@@ -277,14 +281,20 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, form: &[FormField], allowed: b
                     picked
                 } else if multi
                     && shift
-                    && let Some(&anchor) = in_order.last()
+                    && let Some(anchor) =
+                        view.forms.list_anchor.as_ref().filter(|(name, _)| *name == f.name).map(|(_, row)| *row).or_else(|| in_order.last().copied())
                 {
                     let (min, max) = (anchor.min(opt_idx), anchor.max(opt_idx));
                     (min..=max).collect()
                 } else {
                     vec![opt_idx]
                 };
-                if picked != in_order {
+                if !(multi && shift) {
+                    view.forms.list_anchor = Some((f.name.clone(), opt_idx));
+                }
+                // A single-select list holding an array is written as one value even when its shown
+                // row is clicked again.
+                if picked != in_order || f.single_value_is_array {
                     view.pending_edit = Some(Edit::SetFieldValue { name: f.name.clone(), value: FieldValue::ChoiceIndices(picked) });
                 }
             }

crates/ui-egui/tests/it/forms.rs

@@ -1668,3 +1668,86 @@ fn a_form_button_shows_and_hides_the_attachments_panel() {
     h.run_steps(2);
     assert_eq!(h.state().right, None);
 }
+
+/// Click at `p` with `modifiers` held.
+fn click_with(h: &mut Harness<'static, PdfCraftApp>, p: egui::Pos2, modifiers: egui::Modifiers) {
+    h.event(egui::Event::ModifiersChanged(modifiers));
+    h.hover_at(p);
+    h.run_steps(1);
+    for pressed in [true, false] {
+        h.event(egui::Event::PointerButton { pos: p, button: egui::PointerButton::Primary, pressed, modifiers });
+        h.run_steps(1);
+    }
+    h.event(egui::Event::ModifiersChanged(egui::Modifiers::NONE));
+    h.run_steps(3);
+}
+
+/// A list box opened at 100% zoom on its own page, and its rectangle on screen.
+fn list_box(objects: &[&str], name: &str) -> (Harness<'static, PdfCraftApp>, egui::Rect) {
+    let bytes = pdf_from_objects(objects);
+    let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(|_cc| {
+        let mut app = PdfCraftApp::new();
+        app.set_option("language", "en").unwrap();
+        app.open_bytes("list.pdf", None, bytes).unwrap();
+        app.set_option("left", "closed").unwrap();
+        app.set_option("zoom", "100").unwrap();
+        app
+    });
+    h.run_steps(4);
+    let r = {
+        let s = h.state();
+        let doc = s.session.get(s.views[0].id).unwrap();
+        let f = doc.form.iter().find(|f| f.name == name).unwrap();
+        pdfcraft_ui_egui::forms_ui::field_screen_rect(&s.views[0], &doc.info, f, 0).expect("on screen")
+    };
+    (h, r)
+}
+
+/// A Shift-click picks the rows from the one clicked last: after Ctrl-clicking row 3 and then
+/// row 1, Shift on row 4 picks rows 1 to 4, not 3 to 4. The rows share one export value, so the
+/// value can't say which came last, and `/I` lists them in ascending order.
+#[test]
+fn a_shift_click_in_a_list_box_extends_from_the_row_clicked_last() {
+    let (mut h, r) = list_box(
+        &[
+            "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] /NeedAppearances true /DA (/Helv 10 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >> >>",
+            "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 400 400] >>",
+            "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>",
+            "<< /Type /Annot /Subtype /Widget /FT /Ch /T (days) /Ff 2097152 /Opt [[(x) (Mon)] [(x) (Tue)] [(x) (Wed)] [(x) (Thu)] [(x) (Fri)] [(x) (Sat)]] /Rect [50 220 200 320] /P 3 0 R /F 4 >>",
+            "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>",
+        ],
+        "days",
+    );
+    let zoom = r.height() / 100.0;
+    let row = |k: f32| egui::pos2(r.center().x, r.min.y + (8.0 + 11.5 * k) * zoom);
+    click_with(&mut h, row(3.0), egui::Modifiers::COMMAND);
+    click_with(&mut h, row(1.0), egui::Modifiers::COMMAND);
+    assert_eq!(selected(&h, "days"), [1, 3]);
+    click_with(&mut h, row(4.0), egui::Modifiers::SHIFT);
+    assert_eq!(selected(&h, "days"), [1, 2, 3, 4], "from row 1, the one clicked last");
+}
+
+/// A single-select list box holding an array `/V` is written as one value when its shown row is
+/// clicked again, not left as an array because the row was already selected.
+#[test]
+fn clicking_the_shown_row_of_a_single_select_list_holding_an_array_writes_one_value() {
+    let (mut h, r) = list_box(
+        &[
+            "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] /NeedAppearances true /DA (/Helv 10 Tf 0 g) /DR << /Font << /Helv 5 0 R >> >> >> >>",
+            "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 400 400] >>",
+            "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>",
+            "<< /Type /Annot /Subtype /Widget /FT /Ch /T (pair) /Opt [[(TW) (Taiwan)] [(JP) (Japan)] [(US) (United States)]] /V [(TW) (JP)] /Rect [50 250 200 320] /P 3 0 R /F 4 >>",
+            "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>",
+        ],
+        "pair",
+    );
+    let field = |h: &Harness<'static, PdfCraftApp>| {
+        let s = h.state();
+        s.session.get(s.views[0].id).unwrap().form.iter().find(|f| f.name == "pair").unwrap().clone()
+    };
+    assert!(field(&h).single_value_is_array, "loaded from an array");
+    let zoom = r.height() / 70.0;
+    click_at(&mut h, egui::pos2(r.center().x, r.min.y + 8.0 * zoom));
+    let f = field(&h);
+    assert_eq!((f.value, f.selected, f.single_value_is_array), (vec!["TW".to_string()], vec![0], false), "Taiwan, now one value");
+}