From 61fe4b56a71617f75eaa5652a8109b41884e39a7 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 23 Oct 2026 20:28:13 +0200 Subject: [PATCH] =?UTF-8?q?M6:=20Field=20Properties=20=E2=96=B8=20Actions:?= =?UTF-8?q?=20Edit=20opens=20an=20existing=20script=20(#4006)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Edit on a Run a JavaScript action loads its script into the editor; Update replaces it on the same trigger, Discard leaves it. Saving the tab now keeps actions that did not change as they are, and an edited script keeps the actions chained after it (/Next). --- crates/forms/src/actions.rs | 21 +++++++++- crates/forms/src/tests.rs | 41 ++++++++++++++++++++ crates/ui-egui/src/prepare.rs | 62 +++++++++++++++++++++++------- crates/ui-egui/tests/it/prepare.rs | 62 ++++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 5 files changed, 173 insertions(+), 17 deletions(-) diff --git a/crates/forms/src/actions.rs b/crates/forms/src/actions.rs index cdc89e5632..25609b3cbb 100644 --- a/crates/forms/src/actions.rs +++ b/crates/forms/src/actions.rs @@ -207,11 +207,13 @@ fn write(doc: &mut Document, a: &FieldAction, pages: &[pdfcraft_cos::ObjRef]) -> } /// Replace field `name`'s actions (on every widget) with `actions`; triggers not listed lose -/// theirs. `Other` actions are left as they are. +/// theirs. `Other` actions, and actions that read the same as before, are left as they are (with +/// their chain and the keys this module doesn't read); a script edited in place keeps its chain. pub fn set_field_actions(doc: &mut Document, name: &str, actions: &[(Trigger, FieldAction)]) -> Result<(), FormError> { let all = fields(doc); let f = all.iter().find(|f| f.name == name).ok_or_else(|| FormError::NoSuchField(name.into()))?.clone(); let pages = crate::page_refs(doc); + let mut scripts = crate::script_budget(); for w in &f.widgets { let wd = doc.get(w.obj).as_dict().cloned().unwrap_or_default(); let mut aa = wd.get(b"AA").map(|a| doc.resolve(a)).and_then(|a| a.as_dict().cloned()).unwrap_or_default(); @@ -221,10 +223,27 @@ pub fn set_field_actions(doc: &mut Document, name: &str, actions: &[(Trigger, Fi if matches!(new, Some(FieldAction::Other(_))) { continue; } + let old = match t.key() { + None => wd.get(b"A").cloned().or_else(|| aa.get(b"U").cloned()), + Some(k) => aa.get(k).cloned(), + }; + let old_action = old.as_ref().and_then(|o| read(doc, o, &pages, &mut scripts)); + if new.is_some() && new == old_action.as_ref() { + continue; + } + let next = match (new, &old_action) { + (Some(FieldAction::JavaScript(_)), Some(FieldAction::JavaScript(_))) => { + old.and_then(|o| doc.resolve(&o).as_dict().and_then(|d| d.get(b"Next").cloned())) + } + _ => None, + }; let obj = match new { Some(a) => write(doc, a, &pages)?, None => None, }; + if let (Some(Object::Ref(r)), Some(next)) = (&obj, next) { + doc.update_dict(*r, |d| d.set(b"Next".to_vec(), next))?; + } match t.key() { None => { a_entry = obj; diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index ea683bb67f..8b83c69b7d 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -1907,6 +1907,47 @@ fn field_actions_round_trip_on_every_trigger() { assert_eq!(fields(&doc).iter().find(|f| f.name == "go").unwrap().button, Some(af::ButtonAction::Named("Print".into()))); } +#[test] +fn editing_one_script_keeps_the_other_actions_and_the_chain() { + let mut doc = fixture(); + let go = field(&fields(&doc), "go").obj; + let action = |doc: &mut Document, entries: Vec<(&[u8], Object)>| { + let mut a = Dict::new(); + for (k, v) in entries { + a.set(k.to_vec(), v); + } + doc.add(Object::Dict(a)) + }; + let next = action(&mut doc, vec![(b"S", Object::name("URI")), (b"URI", Object::String(PdfString::literal(b"https://example.org".to_vec())))]); + let up = action( + &mut doc, + vec![(b"S", Object::name("JavaScript")), (b"JS", Object::String(PdfString::text("app.alert(1);"))), (b"Next", Object::Ref(next))], + ); + let blur = action( + &mut doc, + vec![(b"S", Object::name("JavaScript")), (b"JS", Object::String(PdfString::text("this.dirty = true;"))), (b"Foo", Object::Int(7))], + ); + let mut aa = Dict::new(); + aa.set(b"Bl".to_vec(), Object::Ref(blur)); + doc.update_dict(go, |d| { + d.set(b"A".to_vec(), Object::Ref(up)); + d.set(b"AA".to_vec(), Object::Dict(aa)); + }) + .unwrap(); + let mut acts = field_actions(&doc, "go").unwrap(); + assert_eq!(acts[0], (Trigger::MouseUp, FieldAction::JavaScript("app.alert(1);".into()))); + acts[0].1 = FieldAction::JavaScript("app.alert(2);".into()); + set_field_actions(&mut doc, "go", &acts).unwrap(); + let w = doc.get(go).as_dict().cloned().unwrap(); + // The action that did not change is the same object, with its keys this crate doesn't read. + let aa = w.get(b"AA").unwrap().as_dict().unwrap().clone(); + assert_eq!(aa.get(b"Bl"), Some(&Object::Ref(blur))); + // The edited script keeps the actions chained after it. + let a = doc.resolve(w.get(b"A").unwrap()).as_dict().cloned().unwrap(); + assert_eq!(a.get(b"Next"), Some(&Object::Ref(next))); + assert_eq!(field_actions(&doc, "go").unwrap()[0], (Trigger::MouseUp, FieldAction::JavaScript("app.alert(2);".into()))); +} + #[test] fn push_buttons_read_hide_actions() { let mut doc = fixture(); diff --git a/crates/ui-egui/src/prepare.rs b/crates/ui-egui/src/prepare.rs index f79ebf17f9..1cba5d89f2 100644 --- a/crates/ui-egui/src/prepare.rs +++ b/crates/ui-egui/src/prepare.rs @@ -852,6 +852,8 @@ pub struct FieldDraft { /// Actions tab: each trigger's action, and the one being added. pub actions: Vec<(pdfcraft_engine::FieldTrigger, pdfcraft_engine::FieldAction)>, pub new_action: ActionDraft, + /// Actions tab: the trigger whose script `new_action` is editing (Edit), if any. + pub editing: Option, original: Box>, } @@ -913,6 +915,7 @@ impl FieldDraft { others: Vec::new(), actions: Vec::new(), new_action: ActionDraft::default(), + editing: None, original: Box::new(None), }; d.original = Box::new(Some(d.clone())); @@ -1533,21 +1536,27 @@ fn validate_tab(ui: &mut egui::Ui, d: &mut FieldDraft) { fn actions_tab(ui: &mut egui::Ui, d: &mut FieldDraft, t: &crate::theme::Tokens) { use pdfcraft_engine::FieldTrigger as T; - ui.label(egui::RichText::new(tl!("Add an Action")).font(theme::semibold(13.0))); + let editing = d.editing.is_some(); + ui.label(egui::RichText::new(if editing { tl!("Edit Action") } else { tl!("Add an Action") }).font(theme::semibold(13.0))); let a = &mut d.new_action; egui::Grid::new("field-actions-add").num_columns(2).spacing([12.0, 8.0]).show(ui, |ui| { ui.label(tl!("Select Trigger:")); - egui::ComboBox::from_id_salt("action-trigger").selected_text(tl!(a.trigger.label())).width(220.0).show_ui(ui, |ui| { - for tr in T::ALL { - ui.selectable_value(&mut a.trigger, tr, tl!(tr.label())); - } + // Editing changes the script of one trigger's action: its trigger and kind stay. + ui.add_enabled_ui(!editing, |ui| { + egui::ComboBox::from_id_salt("action-trigger").selected_text(tl!(a.trigger.label())).width(220.0).show_ui(ui, |ui| { + for tr in T::ALL { + ui.selectable_value(&mut a.trigger, tr, tl!(tr.label())); + } + }); }); ui.end_row(); ui.label(tl!("Select Action:")); - egui::ComboBox::from_id_salt("action-kind").selected_text(tl!(ACTION_KINDS[a.kind])).width(220.0).show_ui(ui, |ui| { - for (i, k) in ACTION_KINDS.iter().enumerate() { - ui.selectable_value(&mut a.kind, i, tl!(k)); - } + ui.add_enabled_ui(!editing, |ui| { + egui::ComboBox::from_id_salt("action-kind").selected_text(tl!(ACTION_KINDS[a.kind])).width(220.0).show_ui(ui, |ui| { + for (i, k) in ACTION_KINDS.iter().enumerate() { + ui.selectable_value(&mut a.kind, i, tl!(k)); + } + }); }); ui.end_row(); let hint = match a.kind { @@ -1568,7 +1577,12 @@ fn actions_tab(ui: &mut egui::Ui, d: &mut FieldDraft, t: &crate::theme::Tokens) ui.end_row(); }); let mut error = None; - if ui.button(tl!("Add")).clicked() { + let (mut commit, mut discard) = (false, false); + ui.horizontal(|ui| { + commit = ui.button(if editing { tl!("Update") } else { tl!("Add") }).clicked(); + discard = editing && ui.button(tl!("Discard")).clicked(); + }); + if commit { match d.new_action.action() { Ok(act) => { let tr = d.new_action.trigger; @@ -1576,10 +1590,15 @@ fn actions_tab(ui: &mut egui::Ui, d: &mut FieldDraft, t: &crate::theme::Tokens) d.actions.push((tr, act)); d.actions.sort_by_key(|(x, _)| T::ALL.iter().position(|y| y == x)); d.new_action.text.clear(); + d.editing = None; } Err(e) => error = Some(e), } } + if discard { + d.new_action = ActionDraft::default(); + d.editing = None; + } if let Some(e) = error { ui.label(egui::RichText::new(tl!(&e)).small().color(t.text_muted)); } @@ -1590,19 +1609,34 @@ fn actions_tab(ui: &mut egui::Ui, d: &mut FieldDraft, t: &crate::theme::Tokens) if d.actions.is_empty() { ui.label(egui::RichText::new(tl!("No actions")).color(t.text_muted)); } - let mut remove = None; + let (mut remove, mut edit) = (None, None); for (i, (tr, act)) in d.actions.iter().enumerate() { ui.horizontal(|ui| { ui.label(egui::RichText::new(tl!(tr.label())).strong()); ui.label(act.describe()); ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { - if ui.push_id(i, |ui| ui.button(tl!("Delete"))).inner.clicked() { - remove = Some(i); - } + ui.push_id(i, |ui| { + if ui.button(tl!("Delete")).clicked() { + remove = Some(i); + } + if let pdfcraft_engine::FieldAction::JavaScript(js) = act + && ui.add_enabled(d.editing != Some(*tr), egui::Button::new(tl!("Edit"))).clicked() + { + edit = Some((*tr, js.clone())); + } + }); }); }); } + if let Some((trigger, text)) = edit { + d.new_action = ActionDraft { trigger, kind: 0, text }; + d.editing = Some(trigger); + } if let Some(i) = remove { + if d.actions.get(i).map(|(x, _)| Some(*x)) == Some(d.editing) { + d.new_action = ActionDraft::default(); + d.editing = None; + } d.actions.remove(i); } }); diff --git a/crates/ui-egui/tests/it/prepare.rs b/crates/ui-egui/tests/it/prepare.rs index 106d2d5391..84305fa7ea 100644 --- a/crates/ui-egui/tests/it/prepare.rs +++ b/crates/ui-egui/tests/it/prepare.rs @@ -712,6 +712,68 @@ fn actions_tab_adds_and_removes_actions() { ); } +#[test] +fn actions_tab_edits_the_script_of_an_existing_action() { + use pdfcraft_engine::{FieldAction, FieldTrigger}; + let mut h = harness(); + h.state_mut().execute("form.prepare"); + h.run_steps(2); + let script = "if (event.value > 10) {\n app.alert('Too many');\n}"; + let actions = |h: &Harness<'static, PdfCraftApp>| { + let s = h.state(); + s.session.get(s.views[0].id).unwrap().field_actions("city") + }; + let open = |h: &mut Harness<'static, PdfCraftApp>| { + h.state_mut().open_field_props("city", 0); + h.run_steps(2); + h.get_by_label("Actions").click(); + h.run_steps(2); + }; + // A script on Mouse Up and a link on Blur, saved with the field. + open(&mut h); + for (trigger, kind, text) in [(FieldTrigger::MouseUp, 0, script), (FieldTrigger::OnBlur, 1, "https://example.org")] { + let d = h.state_mut().field_props.as_mut().unwrap(); + d.new_action = pdfcraft_ui_egui::prepare::ActionDraft { trigger, kind, text: text.into() }; + h.run_steps(3); + h.get_by_label("Add").click(); + h.run_steps(2); + } + h.get_by_label("OK").click(); + h.run_steps(3); + let saved = actions(&h); + // Edit shows the script exactly as stored; Discard leaves everything as it was. + open(&mut h); + h.get_all_by_label("Edit").last().unwrap().click(); + h.run_steps(2); + h.get_by_label("Edit Action"); + assert_eq!(h.state().field_props.as_ref().unwrap().new_action.text, script); + h.state_mut().field_props.as_mut().unwrap().new_action.text = "app.alert('changed');".into(); + h.run_steps(2); + h.get_by_label("Discard").click(); + h.run_steps(2); + h.get_by_label("Add an Action"); + h.get_by_label("OK").click(); + h.run_steps(3); + assert_eq!(actions(&h), saved); + // Update replaces that script only: same trigger, the link untouched. + open(&mut h); + h.get_all_by_label("Edit").last().unwrap().click(); + h.run_steps(2); + h.state_mut().field_props.as_mut().unwrap().new_action.text = "app.alert('changed');".into(); + h.run_steps(2); + h.get_by_label("Update").click(); + h.run_steps(2); + h.get_by_label("OK").click(); + h.run_steps(3); + assert_eq!( + actions(&h), + [ + (FieldTrigger::MouseUp, FieldAction::JavaScript("app.alert('changed');".into())), + (FieldTrigger::OnBlur, FieldAction::Uri("https://example.org".into())) + ] + ); +} + /// Text field table: pick the tool and drag out an area in the empty lower part of page 1, 200 by /// 120 points from (40, 240) as displayed. The table's window opens for it. fn draw_table(h: &mut Harness<'static, PdfCraftApp>) { diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 6d950a71ca..06c38dfd21 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4956,8 +4956,8 @@ milestone = "M6" status = "shipped" commands = ["form.prepare"] tools = ["form_actions", "form_set_actions"] -evidence = ["crates/forms/src/tests.rs::field_actions_round_trip_on_every_trigger", "crates/automation/tests/it/automation.rs::field_actions_through_tools", "crates/ui-egui/tests/it/prepare.rs::actions_tab_adds_and_removes_actions"] -notes = "Six triggers; Run a JavaScript, Open a web link, Reset a form, Execute a menu item, Go to a page view, Show/hide a field, Submit a form. Other action types are kept." +evidence = ["crates/forms/src/tests.rs::field_actions_round_trip_on_every_trigger", "crates/automation/tests/it/automation.rs::field_actions_through_tools", "crates/ui-egui/tests/it/prepare.rs::actions_tab_adds_and_removes_actions", "crates/ui-egui/tests/it/prepare.rs::actions_tab_edits_the_script_of_an_existing_action", "crates/forms/src/tests.rs::editing_one_script_keeps_the_other_actions_and_the_chain"] +notes = "Six triggers; Run a JavaScript, Open a web link, Reset a form, Execute a menu item, Go to a page view, Show/hide a field, Submit a form. Other action types are kept. Edit opens the script of an existing Run a JavaScript action; unchanged actions and the chain after an edited script are kept as they are. Edit for the other action types, and several actions per trigger, are missing." [[feature]] id = "form.props-format"