From 9d7728e68ae6849bc341ca5b22c4dcc7e8058227 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sat, 31 Oct 2026 17:46:54 +0800 Subject: [PATCH] M6: a click in a same-name twin list box picks the row clicked The click queued the clicked list box's own option positions under the shared field name, but set_value reads ChoiceIndices in the first field of that name. A twin listing options that share an export value in another order therefore gave the first field the option at the same position, not the one clicked. The UI now maps the twin's picks to the first field's positions by export value and text (Field::positions_in, the same matching the engine uses to copy /I to twins), falling back to the export values when an option has no match there. (review) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/forms/src/lib.rs | 19 ++++++++++++++ crates/forms/src/values.rs | 14 +++------- crates/ui-egui/src/tools/forms_ui.rs | 13 +++++++++- crates/ui-egui/tests/it/forms.rs | 39 ++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 12 deletions(-) diff --git a/crates/forms/src/lib.rs b/crates/forms/src/lib.rs index 0be2912518..608e5231d7 100644 --- a/crates/forms/src/lib.rs +++ b/crates/forms/src/lib.rs @@ -238,6 +238,14 @@ impl Field { } } + /// Combo and list boxes: this field's option positions `picked` as positions in `other`'s + /// options, in the same order. Same-name twins are one field (ISO 32000-2 §12.7.3.1) but can + /// list their options in different orders, and a value given by position is read in the first + /// twin's options. `None` when an option picked has no match in `other`. + pub fn positions_in(&self, other: &Field, picked: &[usize]) -> Option> { + option_positions_in(picked, &self.options, &other.options) + } + /// Combo and list boxes: the selected options in the order the value names them (`/V` keeps /// the order they were picked in), so the last is the latest pick. pub fn picked_in_order(&self) -> Vec { @@ -357,3 +365,14 @@ fn name_obj(text: &str) -> Object { fn invalid(m: impl Into) -> Result { Err(FormError::Invalid(m.into())) } + +/// The positions in `target` of the options at positions `picked` in `source`, in the same order: +/// each goes to `target`'s first not yet taken option with the same export value and text, so two +/// options sharing both stay apart. `None` when one has no such option in `target`. +pub(crate) fn option_positions_in(picked: &[usize], source: &[(String, String)], target: &[(String, String)]) -> Option> { + let mut free: std::collections::HashMap<(&str, &str), std::collections::VecDeque> = std::collections::HashMap::new(); + for (i, (e, d)) in target.iter().enumerate() { + free.entry((e.as_str(), d.as_str())).or_default().push_back(i); + } + 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() +} diff --git a/crates/forms/src/values.rs b/crates/forms/src/values.rs index 6996549520..348b18b347 100644 --- a/crates/forms/src/values.rs +++ b/crates/forms/src/values.rs @@ -727,18 +727,10 @@ fn first_pick(seen: &mut [bool], i: usize) -> bool { } } -/// 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. +/// The twin's positions for the source's picked positions `picked`, as +/// [`crate::option_positions_in`] maps them, 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::>>()?; + let mut out = crate::option_positions_in(picked, source, twin)?; 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 7da73bf032..8219ee3e5b 100644 --- a/crates/ui-egui/src/tools/forms_ui.rs +++ b/crates/ui-egui/src/tools/forms_ui.rs @@ -295,7 +295,18 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, form: &[FormField], allowed: b // 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) }); + // The edit names the field, and positions are read in the first field of that + // name: a same-name twin listing its options in another order gives its own + // picks as positions there. One option missing there goes by export value. + let first = form.iter().find(|x| x.name == f.name).filter(|x| x.obj != f.obj); + let value = match first { + None => FieldValue::ChoiceIndices(picked), + Some(first) => match f.positions_in(first, &picked) { + Some(there) => FieldValue::ChoiceIndices(there), + None => FieldValue::Choice(picked.iter().filter_map(|&i| f.options.get(i)).map(|(e, _)| e.clone()).collect()), + }, + }; + view.pending_edit = Some(Edit::SetFieldValue { name: f.name.clone(), value }); } } } diff --git a/crates/ui-egui/tests/it/forms.rs b/crates/ui-egui/tests/it/forms.rs index db7fd1bd14..781f23150b 100644 --- a/crates/ui-egui/tests/it/forms.rs +++ b/crates/ui-egui/tests/it/forms.rs @@ -1352,6 +1352,45 @@ fn a_list_box_click_picks_one_of_two_options_with_the_same_export_value() { let _ = std::fs::remove_dir_all(&dir); } +/// Same-name list boxes are one field (ISO 32000-2 §12.7.3.1), and a twin can list the options +/// sharing an export value in another order: clicking One-B in the second gives both One-B, not +/// the option at the same position in the first. +#[test] +fn a_click_in_a_twin_list_box_with_options_in_another_order_picks_the_row_clicked() { + let bytes = pdf_from_objects(&[ + "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R 6 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 6 0 R] >>", + "<< /Type /Annot /Subtype /Widget /FT /Ch /T (dup) /Opt [[(1) (One-A)] [(1) (One-B)] [(2) (Two)]] /Rect [50 250 200 320] /P 3 0 R /F 4 >>", + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>", + "<< /Type /Annot /Subtype /Widget /FT /Ch /T (dup) /Opt [[(1) (One-B)] [(1) (One-A)] [(2) (Two)]] /Rect [50 100 200 170] /P 3 0 R /F 4 >>", + ]); + 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("twins.pdf", None, bytes).unwrap(); + app.set_option("left", "closed").unwrap(); + app.set_option("zoom", "100").unwrap(); + app + }); + h.run_steps(4); + let twins = |h: &Harness<'static, PdfCraftApp>| -> Vec> { + let s = h.state(); + s.session.get(s.views[0].id).unwrap().form.iter().filter(|f| f.name == "dup").map(|f| f.selected.clone()).collect() + }; + let r = { + let s = h.state(); + let doc = s.session.get(s.views[0].id).unwrap(); + let f = doc.form.iter().filter(|f| f.name == "dup").nth(1).unwrap(); + pdfcraft_ui_egui::forms_ui::field_screen_rect(&s.views[0], &doc.info, f, 0).expect("on screen") + }; + let zoom = r.height() / 70.0; + // Row 0 of the second list box: One-B. + click_at(&mut h, egui::pos2(r.center().x, r.min.y + 8.0 * zoom)); + assert_eq!(value(&h, "dup"), ["1"]); + assert_eq!(twins(&h), [vec![1], vec![0]], "One-B in both: second in the first, first in the twin"); +} + /// ISO 32000-2 Table 233: a list box without MultiSelect selects "at most one item". Given /// `/V [(TW) (JP)]` it shows Taiwan alone, and a click writes the one option clicked. #[test]