grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

ui: PreClick/PostClick, and a comma-separated template list

Browse Source 
Two of the `Addon_Ports` pair's load blockers, both measured with `addon_harness` either side.

**`PreClick`/`PostClick`** follow the convention `SCRIPT_KINDS` already set for names that are
not 1.12's — `OnHyperlinkEnter`/`Leave` sit there with the same justification: a later client
added them, addons register them at load, and refusing the name raises and takes the rest of
the file with it. `Cinematic/MenuWheel.lua`'s wheel slots are the callers here. They arrived
with the secure template system so a button could act either side of the protected part;
nothing protected sits between them here, so the order is all they are.

`button::click_button` fires each of the three **separately**, so one that raises is recorded
and the next still runs: the pair exists to bracket the click, and a broken bracket must not
eat it. That is the same reasoning the attribute-click dispatch below it already carries.

**A comma-separated template list**, which 1.12 does not take: its `inherits` is one name, and
`object.rs`'s lookup is a transcription of that (`0x6ee6f0` at `0x7061dd`, no splitter). Era
takes a list and applies them in order, and that is what an addon written for Era passes —
`Cinematic/MenuWheel.lua:312` asks for
`"SecureHandlerShowHideTemplate, SecureHandlerAttributeTemplate"`.

**The evidence is weaker than `compat/`'s other relaxations and the code says so.** The
one-argument `SetPoint` rests on the real `ClassicAPI.dll` writing that form itself; this rests
only on Era doing it and the addon targeting Era — nothing in the DLL's own Lua passes a list,
and **no addon in the installed corpus does either**, which is also why it can break nothing.

Safe by construction: the split is attempted only **after** the verbatim name misses, so every
existing call takes exactly the path it did before and a template whose own name contained a
comma would still win. One unknown name anywhere in the list and the whole call misses,
reporting the literal **unsplit** and leaving no frame behind — so a typo reads as a typo
rather than as a half-built frame.

`a_comma_list_is_one_name_and_a_single_name_still_chains` was a deliberate, traced pin on the
old behaviour, so it is rewritten rather than deleted. It now pins the shape of the relaxation:
the verbatim name tried first, both templates applied in the order written with `$parent`
resolving against the instance, a list with one unknown name still missing unsplit and leaving
nothing behind, and a plain unknown name unchanged. Its doc states the weaker footing, which is
why the miss path is pinned that closely.

benilla-ui 2081 passed. Full chain green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
grom
2026-11-01 17:14:32 -07:00
co-authored by Claude Opus 5
parent af823dd708
commit 139965f415
4 changed files with 103 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-ui/src/script/button.rs
+15 -6
View File
@@ -1004,12 +1004,21 @@ pub(super) fn click_button(lua: &Lua, id: u32, button: &str, down: bool, scripte
Ok(s) => Value::String(s),
Err(_) => return,
};
if let Err(e) = event::fire_widget_handler(lua, id, "OnClick", vec![btn, Value::Boolean(down)])
{
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
}
// `PreClick`, the handler, `PostClick` (`script/mod.rs`'s `SCRIPT_KINDS` has why the pair is
// accepted at all). Each is fired on its own, so one that raises is recorded and the next
// still runs: the pair exists to bracket the click, and a broken bracket must not eat it.
let fire = |kind: &str| {
if let Err(e) =
event::fire_widget_handler(lua, id, kind, vec![btn.clone(), Value::Boolean(down)])
{
lua.app_data_mut::<Model>()
.expect("model app_data")
.record_script_error(e.to_string());
}
};
fire("PreClick");
fire("OnClick");
fire("PostClick");
// ClassicAPI's secure-attribute click, **after** the `OnClick` handler for the same reason the
// nameplate selection below is: a frame may have both, and a hook that raises must not eat the
// action. A frame with no `type<N>` attribute is untouched.
crates/benilla-ui/src/script/mod.rs
+10 -1
View File
@@ -434,7 +434,7 @@ pub const SCREEN: crate::layout::Handle = 0;
/// ScrollFrame's three scroll kinds are `+0x32c`/`+0x334`/`+0x33c` (script-name map `0x786c40`).
/// The EditBox's vtable (`0x81c910`) replaces the key and char slots: an EditBox never fires
/// `OnKeyDown`, and fires `OnChar` only from `Insert`, with the inserted text (`0x77c13c`).
const SCRIPT_KINDS: [&str; 43] = [
const SCRIPT_KINDS: [&str; 45] = [
"OnLoad",
// Accepted and never fired, deliberately: the realm's own `FloatingChatFrame.xml` sets it on
// `ChatFrameEditBox`, so refusing it put a red `SetScript: unsupported script` in front of
@@ -451,6 +451,15 @@ const SCRIPT_KINDS: [&str; 43] = [
"OnShow",
"OnHide",
"OnClick",
// The pair either side of `OnClick`, fired by `button::click_button`. **Not 1.12's** — they
// arrived with the secure template system, so a button could act before and after the
// protected part — but an addon written for a later client registers them at load, and
// refusing the name raises there and takes the rest of the file with it
// (`Cinematic/MenuWheel.lua`'s wheel slots). Nothing protected sits between them here, so the
// order is all they are: `PreClick`, the handler, `PostClick`, each with the same
// `(button, down)` the click carries.
"PreClick",
"PostClick",
"OnEnter",
"OnLeave",
"OnMouseDown",
crates/benilla-ui/src/script/object.rs
+34 -8
View File
@@ -618,17 +618,41 @@ pub(super) fn create_frame(
// at `0x7061ed`, before the node build (`0x706208`) and the name store (`0x70622d`), so it
// leaves no partial widget and no global. The raise never returns and `pcall` catches it, as
// the widget dispatcher (`0x704f10`) does for every handler.
let mut templates_to_apply: Vec<String> = Vec::new();
if let Some(t) = template.as_deref().filter(|t| !t.is_empty()) {
let known = {
let known = |name: &str| {
let model = lua.app_data_ref::<Model>().expect("model app_data");
let templates = model.framexml_templates.borrow();
// One name, verbatim, case-folded, as `framexml::expand` resolves it.
templates.contains_key(t) || templates.keys().any(|k| k.eq_ignore_ascii_case(t))
templates.contains_key(name) || templates.keys().any(|k| k.eq_ignore_ascii_case(name))
};
if !known {
return Err(mlua::Error::runtime(format!(
"CreateFrame(): Couldn't find inherited node \"{t}\""
)));
if known(t) {
templates_to_apply.push(t.to_string());
} else {
// **A comma-separated list, which 1.12 does not take.** Its `inherits` is one name
// (the lookup above, `0x7061dd`); Era's takes a list and applies them in order, and
// that is what an addon written for Era passes — `Cinematic/MenuWheel.lua:312` asks
// for `"SecureHandlerShowHideTemplate, SecureHandlerAttributeTemplate"`.
//
// **The evidence here is weaker than `compat/`'s other relaxations and that is worth
// saying**: `anchor_args.rs`'s one-argument `SetPoint` rests on the real
// `ClassicAPI.dll` writing that form itself, where this rests only on Era doing it and
// the addon targeting Era — nothing in the DLL's own Lua passes a list, and **no addon
// in the installed corpus does either**, which is also why it can break nothing.
//
// Tried only after the verbatim name misses, so a template whose own name contained a
// comma would still win, and every existing call takes exactly the path it did before.
let parts: Vec<String> = t
.split(',')
.map(|p| p.trim().to_string())
.filter(|p| !p.is_empty())
.collect();
if parts.len() < 2 || !parts.iter().all(|p| known(p)) {
return Err(mlua::Error::runtime(format!(
"CreateFrame(): Couldn't find inherited node \"{t}\""
)));
}
templates_to_apply = parts;
}
}
@@ -663,8 +687,10 @@ pub(super) fn create_frame(
// The wrapper exists, published under its name, before the template's `OnLoad` runs inside
// this call, so the handler can reach the frame by name.
let wrapper = frame_wrapper(lua, id)?;
if let Some(template) = template {
let messages = crate::loader::apply_template(lua, &wrapper, &kind, &template);
// In the order written: a later template's `OnLoad` and regions land over an earlier one's,
// which is the order `inherits=` resolves a list in.
for template in &templates_to_apply {
let messages = crate::loader::apply_template(lua, &wrapper, &kind, template);
if !messages.is_empty() {
let mut model = lua.app_data_mut::<Model>().expect("model app_data");
for m in messages {
crates/benilla-ui/src/script/tests/create_frame_template.rs
+44 -8
View File
@@ -129,10 +129,19 @@ fn a_nested_frames_child_is_named_against_the_caller() {
assert!(s.take_warnings().is_empty());
}
/// The template argument is one name: 1.12's `CreateFrame` hands the string to the XML loader's
/// lookup (`0x6ee6f0`, called at `0x7061dd`), which has no splitter; comma lists came later.
/// The template argument is one name **to 1.12**: its `CreateFrame` hands the string to the XML
/// loader's lookup (`0x6ee6f0`, called at `0x7061dd`), which has no splitter; comma lists came
/// later, and Era takes one.
///
/// `script/object.rs` now accepts a list, as `compat/` does elsewhere, and this test pins the
/// shape of that: the **verbatim name is tried first**, so every single-name call takes exactly
/// the path it did before; a list applies its templates in the order written; and a list naming
/// anything unknown still misses, still reporting the literal it looked up **unsplit** and still
/// leaving no frame behind. That relaxation rests on weaker evidence than the rest of `compat/`
/// (Era does it and the addon targets Era, where the one-argument `SetPoint` rests on the real
/// `ClassicAPI.dll` writing that form itself), which is why the miss path is pinned this closely.
#[test]
fn a_comma_list_is_one_name_and_a_single_name_still_chains() {
fn a_comma_list_applies_both_and_an_unknown_one_still_misses_unsplit() {
let mut s = script();
register(
&s,
@@ -162,19 +171,46 @@ fn a_comma_list_is_one_name_and_a_single_name_still_chains() {
assert!(s.take_errors().is_empty());
assert!(s.take_warnings().is_empty());
// A list of known names applies both, in the order written: the size and alpha off TemplA's
// own root, and TemplB's region.
s.run(r#"Both = CreateFrame("Frame", "Both", nil, "TemplA, TemplB")"#)
.unwrap();
assert_eq!(
s.eval::<(f32, f32)>("return Both:GetWidth(), Both:GetHeight()")
.unwrap(),
(50.0, 60.0),
"the first template applied"
);
assert_eq!(s.eval::<f32>("return Both:GetAlpha()").unwrap(), 0.25);
assert!(
s.eval::<bool>(r#"return getglobal("BothBTex") ~= nil"#)
.unwrap(),
"the second template's own region applied too, with $parent resolved against the instance"
);
assert!(s.take_errors().is_empty());
assert!(s.take_warnings().is_empty());
// One unknown name in the list and the whole call misses, reporting the literal unsplit —
// so a typo reads as the typo it is rather than as a half-built frame.
let err = s
.run(r#"Both = CreateFrame("Frame", "Both", nil, "TemplA, TemplB")"#)
.expect_err("a comma list is one name, and it misses");
.run(r#"Half = CreateFrame("Frame", "Half", nil, "TemplA, NoSuchTemplate")"#)
.expect_err("an unknown name in the list misses");
let text = err.to_string();
assert!(
text.contains("TemplA, TemplB"),
text.contains("TemplA, NoSuchTemplate"),
"the miss must name the literal it looked up, unsplit: {text}"
);
assert!(
s.eval::<bool>(r#"return getglobal("Both") == nil"#)
s.eval::<bool>(r#"return getglobal("Half") == nil"#)
.unwrap(),
"a missed template must leave no frame behind"
"a missed list must leave no frame behind"
);
// And a name that is simply unknown, with no comma in it at all, is unchanged.
let err = s
.run(r#"Nope = CreateFrame("Frame", "Nope", nil, "NoSuchTemplate")"#)
.expect_err("an unknown single name misses");
assert!(err.to_string().contains("NoSuchTemplate"));
}
/// The lookup folds ASCII case: `SStrHash` (`0x64b3f0`) uppercases before hashing, so a mis-cased