From 3f708f8eb8e4b525b43061403d80fe04b36994c3 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 30 Oct 2026 22:03:45 +0800 Subject: [PATCH] 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 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/forms/src/lib.rs | 4 ++ crates/forms/src/read.rs | 2 + crates/forms/src/tests.rs | 49 ++++++++++++++++ crates/forms/src/values.rs | 85 ++++++++++++++++++++++------ crates/ui-egui/src/tools/forms_ui.rs | 14 ++++- crates/ui-egui/tests/it/forms.rs | 83 +++++++++++++++++++++++++++ 6 files changed, 219 insertions(+), 18 deletions(-) diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index d6cf3a2012..0be2912518 100644 --- a/crates/forms/src/lib.rs +++ b/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, + /// 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. diff --git a/crates/forms/src/read.rs b/crates/forms/src/read.rs index 177e32fd1d..64394b18ab 100644 --- a/crates/forms/src/read.rs +++ b/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))), diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index c76f934553..dbd2016a90 100644 --- a/crates/forms/src/tests.rs +++ b/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 = 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 = 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 = 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 = 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]); +} diff --git a/crates/forms/src/values.rs b/crates/forms/src/values.rs index fc4263b345..6996549520 100644 --- a/crates/forms/src/values.rs +++ b/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 = 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 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> { + let mut free: std::collections::HashMap<(&str, &str), std::collections::VecDeque> = 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::>>()?; + out.sort_unstable(); + Some(out) +} diff --git a/crates/ui-egui/src/tools/forms_ui.rs b/crates/ui-egui/src/tools/forms_ui.rs index 1cf6a1b610..7da73bf032 100644 --- a/crates/ui-egui/src/tools/forms_ui.rs +++ b/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) }); } } diff --git a/crates/ui-egui/tests/it/forms.rs b/crates/ui-egui/tests/it/forms.rs index f089b33729..db7fd1bd14 100644 --- a/crates/ui-egui/tests/it/forms.rs +++ b/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"); +}