diff --git a/crates/engine/src/cmd/controls.rs b/crates/engine/src/cmd/controls.rs index 773262480..0e03f8f9f 100644 --- a/crates/engine/src/cmd/controls.rs +++ b/crates/engine/src/cmd/controls.rs @@ -571,7 +571,7 @@ pub(crate) fn sync_linked_values(wb: &mut Workbook) { // ---------------------------------------------------------------- xlsx /// The hidden-data key of the button actions xlsx has no place for. -const ACTIONS_KEY: &str = "control_actions"; +pub(crate) const ACTIONS_KEY: &str = "control_actions"; /// A button's action that xlsx carries itself: a macro on the selection (`macro="[0]!Name"`). fn plain(a: &ControlAction) -> bool { @@ -604,11 +604,11 @@ pub(crate) fn with_action_names(wb: &Workbook) -> Result> { } /// After a file is opened: gives buttons back the actions kept in the hidden name, and removes -/// it. Entries that don't parse, or don't match a button, are dropped: the button at the stored -/// place must have the stored name and anchor cell (another application may have added, removed -/// or reordered controls), and must not have been given another macro since (a stored macro -/// action comes back only on a button that still runs that macro, a stored command only on one -/// the file gives no macro). +/// it. Entries that don't parse, or don't match a button, are dropped: an entry must carry the +/// button's name and anchor cell, the button at the stored place must have that name and anchor +/// cell (another application may have added, removed or reordered controls), and it must not +/// have been given another macro since (a stored macro action comes back only on a button that +/// still runs that macro, a stored command only on one the file gives no macro). pub(crate) fn load_action_names(wb: &mut Workbook) { let Some(items) = stash::get_json(wb, ACTIONS_KEY) else { return }; stash::remove(wb, ACTIONS_KEY); @@ -624,8 +624,8 @@ pub(crate) fn load_action_names(wb: &mut Workbook) { let text = |k: &str| item.get(k).and_then(Json::as_str); let same = |c: &FormControl| { c.kind == ControlKind::Button - && text("name").is_none_or(|n| n == c.display_name()) - && text("at").is_none_or(|a| a == c.anchor.cell.a1()) + && text("name").is_some_and(|n| n == c.display_name()) + && text("at").is_some_and(|a| a == c.anchor.cell.a1()) && match (&c.action, &action) { (None, ControlAction::Command { .. }) => true, (Some(ControlAction::Macro { name: now, .. }), ControlAction::Macro { name: kept, .. }) => now.eq_ignore_ascii_case(kept), diff --git a/crates/engine/src/tests/controls.rs b/crates/engine/src/tests/controls.rs index 8dce29fcd..e567fb913 100644 --- a/crates/engine/src/tests/controls.rs +++ b/crates/engine/src/tests/controls.rs @@ -429,6 +429,44 @@ fn a_kept_button_action_comes_back_only_on_the_same_button() { assert_eq!(cleared.sheets[0].controls[0].action, actions(&wb)[0], "the command comes back"); } +/// An entry of the hidden name must carry the button's name and anchor cell (the writer always +/// stores both): a hand-edited entry without either, or with something else there, is dropped +/// like an unparsable one, so it cannot pin its action onto whatever button sits at that index. +#[test] +fn a_kept_action_without_the_buttons_name_or_anchor_is_dropped() { + use crate::cmd::controls::{ACTIONS_KEY, load_action_names, with_action_names}; + use crate::cmd::stash; + let mut s = s(); + s.execute("insert.button", json!({"at": "D2", "text": "Run", "command": "cell.set", "params": {"cell": "A9", "input": "cmd"}})).unwrap(); + let stored = with_action_names(&s.doc().unwrap().wb).unwrap().unwrap(); + let items = stash::get_json(&stored, ACTIONS_KEY).unwrap(); + assert!(items[0]["name"].is_string() && items[0]["at"].is_string(), "the writer stores both keys: {items}"); + // As the file brings a command button back: with no action. + let kept = |items: &Json| { + let mut wb = stored.clone(); + wb.sheet_mut(0).unwrap().controls[0].action = None; + stash::remove(&mut wb, ACTIONS_KEY); + stash::set_json(&mut wb, ACTIONS_KEY, items).unwrap(); + load_action_names(&mut wb); + assert!(stash::get(&wb, ACTIONS_KEY).is_none(), "the hidden name is removed"); + wb.sheets[0].controls[0].action.clone() + }; + let edited = |key: &str, value: Option| { + let mut items = items.clone(); + let entry = items[0].as_object_mut().unwrap(); + match value { + Some(v) => entry.insert(key.to_string(), v), + None => entry.remove(key), + }; + items + }; + assert_eq!(kept(&edited("name", None)), None, "no `name`"); + assert_eq!(kept(&edited("at", None)), None, "no `at`"); + assert_eq!(kept(&edited("name", Some(json!(1)))), None, "`name` is not text"); + assert_eq!(kept(&edited("at", Some(Json::Null))), None, "`at` is null"); + assert!(matches!(kept(&items), Some(ControlAction::Command { .. })), "the writer's own entry still comes back"); +} + /// The saved xlsx bytes of `s`'s workbook. fn saved_xlsx(s: &mut Session) -> Vec { let r = s.execute("file.saveBytes", json!({"format": "xlsx"})).unwrap();