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

OnClick fires on release while the button is still PUSHED, as in 1.12.1 (#923)

Browse Source 
A Button release un-pushed the button before `OnMouseUp` and `OnClick` ran, so both read NORMAL. A plain click on a floating chat tab then took the pick-up branch and left the window following the cursor.

In the reference, the mouse-up handler fires `OnMouseUp` first, then reads the state: a PUSHED button with the cursor inside it and the Up registration fires `OnDoubleClick` or `OnClick`, and only then is it set NORMAL, unless locked. A failed gate still un-pushes; an `OnMouseUp` that un-pushes cancels the click.

The release now runs that order after `OnMouseUp`, with the started-drag skip kept.

Tests read the state inside each handler and drive the stock chat tab.

Fixes #699

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
Sam Sung
2026-10-12 01:52:46 +07:00
committed by GitHub
co-authored by Claude Sonnet 5.5
parent ff3d15b132
commit d65fd19095
3 changed files with 268 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/ui_script/chat_resize_tests.rs
+72 -1
View File
@@ -79,7 +79,11 @@ fn left_click(s: &mut UiScript, frame: &str) {
/// The tabs ship hidden until a stationary hover reveals them (`FCF_OnUpdate`), and a window
/// moves by its tab (`OnDragStart` calls `StartMoving`), so every drag starts here.
fn reveal(s: &mut UiScript) {
let (cx, cy) = centre(s, "ChatFrame1");
reveal_window(s, "ChatFrame1");
}
fn reveal_window(s: &mut UiScript, window: &str) {
let (cx, cy) = centre(s, window);
s.mouse_move(cx, cy);
for _ in 0..45 {
s.tick(0.016);
@@ -517,3 +521,70 @@ fn the_docked_combat_log_cannot_be_moved_or_resized_on_its_own() {
assert_eq!(one, two, "the dock moved as one");
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
}
/// Drag `window`'s tab by `(dx, dy)`: press, past the drag threshold, move, release.
fn drag_named_tab(s: &mut UiScript, window: &str, dx: f32, dy: f32) {
reveal_window(s, window);
let tab = format!("{window}Tab");
let tab = tab.as_str();
let (tx, ty) = centre(s, tab);
assert_eq!(
s.hit_test_name(tx, ty).as_deref(),
Some(tab),
"the tab is hit"
);
s.mouse_button(tx, ty, "LeftButton", true);
s.mouse_move(tx + 5.0, ty);
s.mouse_move(tx + dx, ty + dy);
s.mouse_button(tx + dx, ty + dy, "LeftButton", false);
s.resolve();
}
/// `FCF_Tab_OnClick` reads `GetButtonState()` inside `OnClick`: NORMAL on the press (`0x779285`
/// before the push `0x7792b1`) so an undocked tab picks its window up, PUSHED on the release
/// (`0x7793a9` before `0x7793de`) so it drops it. A tab pulled out of the dock drags again, and a
/// plain click picks the window up and puts it down, never leaving it on the cursor.
#[test]
fn an_undocked_tab_drags_again_and_a_plain_click_drops_the_window() {
let _data = benilla_formats::wow_data_or_skip!();
let mut s = chat_ui();
s.run("FCF_SelectDockFrame(ChatFrame2)").unwrap();
s.resolve();
drag_named_tab(&mut s, "ChatFrame2", 0.0, -200.0);
assert!(
s.eval::<bool>("return not ChatFrame2.isDocked").unwrap(),
"the first drag pulled the window out of the dock"
);
assert!(s.eval::<bool>("return MOVING_CHATFRAME == nil").unwrap());
let first = centre(&mut s, "ChatFrame2");
drag_named_tab(&mut s, "ChatFrame2", 0.0, 120.0);
let second = centre(&mut s, "ChatFrame2");
assert_eq!(
second.1,
first.1 + 120.0,
"the second drag of the undocked tab moved the window"
);
reveal_window(&mut s, "ChatFrame2");
let (tx, ty) = centre(&mut s, "ChatFrame2Tab");
s.mouse_button(tx, ty, "LeftButton", true);
assert!(
s.eval::<bool>("return MOVING_CHATFRAME == ChatFrame2")
.unwrap(),
"the press picked the window up"
);
s.mouse_button(tx, ty, "LeftButton", false);
assert!(
s.eval::<bool>("return MOVING_CHATFRAME == nil").unwrap(),
"the release put it down"
);
let before = centre(&mut s, "ChatFrame2");
s.mouse_move(tx + 90.0, ty + 40.0);
assert_eq!(
centre(&mut s, "ChatFrame2"),
before,
"the window does not follow a cursor with no button held"
);
assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
}
crates/benilla-ui/src/script/pointer.rs
+75 -58
View File
@@ -356,7 +356,7 @@ impl UiScript {
drag_release,
world_dropped,
slider_jump,
double_id,
released,
abandoned,
color_jump,
): (
@@ -364,7 +364,7 @@ impl UiScript {
Option<cursor::DragRelease>,
bool,
Option<(u32, f32)>,
Option<u32>,
Option<crate::widget::FrameHandle>,
Option<crate::widget::FrameHandle>,
Option<(u32, f64, f64, f64)>,
) = {
@@ -428,19 +428,6 @@ impl UiScript {
}
let release = cursor::take_drag(&mut model, button);
let started = release.as_ref().is_some_and(|r| r.started);
// The Button release slot `0x7792d0` clicks only a PUSHED button (`0x7792fc`) with
// the cursor inside its own rect (`0x779335`), not "the topmost frame under the
// cursor", read before the release edge below un-pushes it.
let sources = scroll_clip_sources(&model);
let may_click = pressed.filter(|&h| {
button::release_may_click(&model, h) && frame_contains(&model, &sources, h, x, y)
});
// The release edge, `SetButtonState(NORMAL)` (`0x7793de`), on the pressed frame
// wherever the cursor is, skipped after a started drag (`0x7792df` tests
// `0x76c040`), which already un-pressed it.
if let (Some(h), false) = (pressed, started) {
button::edge(&mut model, h, ButtonState::on_mouse_up);
}
// The world drop: a completed left click on the world, never a drag release, which
// keeps carrying (`0x495300` runs on the WorldFrame click release). The world is
// `over_world`, not "no frame": the stock `WorldFrame` covers the screen.
@@ -450,42 +437,9 @@ impl UiScript {
&& over_world(&model, pressed)
&& !capture_lost
&& cursor::world_drop_click(&mut model);
let wants = format!("{button}Up");
let click = if started {
None
} else {
may_click
.filter(|&h| button::wants_click(&model, h, &wants))
.and_then(|h| model.frame_to_id.get(&h).copied())
};
// The double click, on the release that qualified as a click: `0x77938d` in the
// mouse-up dispatcher `0x7792d0` fires `OnDoubleClick` instead of the second
// `OnClick` (`0x77939d jmp 0x7793b5` skips the single leg), only for a frame with
// an `OnDoubleClick` script (`[+0x4d4] != 0`). What suppresses the click
// suppresses it too.
let target = click.and(may_click);
let double = target.filter(|&h| {
model
.scripts
.get(&h)
.is_some_and(|s| s.contains("OnDoubleClick"))
&& model
.last_click
.get(&h)
.is_some_and(|&t| now - t <= DOUBLE_CLICK_SECS)
});
if let Some(h) = double {
// A double clears the stamp (`0x779393`), so clicks pair: four fast clicks are
// click, double, click, double.
model.last_click.remove(&h);
} else if let Some(h) = target {
// A single stamps the detector (`0x7793af`) whether or not an `OnClick` script
// is bound.
model.last_click.insert(h, now);
}
let double_id = double.and_then(|h| model.frame_to_id.get(&h).copied());
let click = if double_id.is_some() { None } else { click };
(click, release, dropped, None, double_id, None, None)
// The Button release slot `0x7792d0` (below, after `OnMouseUp`) does nothing on a
// started drag (`0x7792df` tests `0x76c040`), which already un-pressed the button.
(None, release, dropped, None, pressed.filter(|_| !started), None, None)
}
};
// A track press's value jump fires `OnValueChanged` first, outside the borrow.
@@ -590,13 +544,8 @@ impl UiScript {
if let Some(h) = press_art {
button::edge(&mut self.model_mut(), h, ButtonState::on_mouse_down);
}
// `OnDoubleClick(self, button)` in place of `OnClick` (`0x77938d call [edx+0x98]`), with
// the same click sound: its firer `0x779650` is the twin of `OnClick`'s `0x779540` and
// passes only the button name of the completing release.
if let Some(id) = double_id {
if let Err(e) = event::fire_widget_handler(&self.lua, id, "OnDoubleClick", vec![btn]) {
self.push_error(e);
}
if let Some(h) = released {
self.release_slot(h, button, (x, y), now, &btn);
}
// A world-frame hit is not the UI consuming the event: the reference runs its
// `OnMouseDown` without consuming (`0x483c40`), then the `BUTTON1`/`BUTTON2` binding, the
@@ -604,6 +553,74 @@ impl UiScript {
hit_id.is_some_and(|id| !self.is_world_frame(id)) || world_dropped
}
/// The Button release slot `0x7792d0` past its base (`0x76c040`: `OnMouseUp`, which the caller
/// has fired), on the frame that took the press: a PUSHED button (`0x7792fc`) with the cursor
/// inside its own rect (`0x779335`), not "the topmost frame under the cursor", and registered
/// for this button's Up (`0x77934e`) fires `OnDoubleClick` (`0x77938d`) or `OnClick`
/// (`0x7793a9`); only then does the tail set NORMAL (`0x7793de`) if the button is unlocked and
/// not disabled (`0x7793c2`..`0x7793d4`), on every path that got past the pushed gate and
/// wherever the cursor is. So a handler reads PUSHED, and what it sets (a lock, a disable)
/// decides the tail. The state is read here, after `OnMouseUp`, so a script that un-pushes the
/// button there cancels the click.
fn release_slot(
&mut self,
h: crate::widget::FrameHandle,
button: &str,
(x, y): (f32, f32),
now: f64,
btn: &Value,
) {
let (click, double) = {
let model = self.model_ref();
if model.arena.frame(h).is_none() {
return;
}
let sources = scroll_clip_sources(&model);
let clicks = button::release_may_click(&model, h)
&& frame_contains(&model, &sources, h, x, y)
&& button::wants_click(&model, h, &format!("{button}Up"));
// The double click: `0x77938d` fires `OnDoubleClick` instead of the second `OnClick`
// (`0x77939d jmp 0x7793b5` skips the single leg), only for a frame with an
// `OnDoubleClick` script (`[+0x4d4] != 0`) whose last click was within the interval.
// What suppresses the click suppresses it too.
let double = clicks
&& model
.scripts
.get(&h)
.is_some_and(|s| s.contains("OnDoubleClick"))
&& model
.last_click
.get(&h)
.is_some_and(|&t| now - t <= DOUBLE_CLICK_SECS);
(clicks, double)
};
let id = self.model_ref().frame_to_id.get(&h).copied();
if let (true, Some(id)) = (click, id) {
if double {
if let Err(e) =
event::fire_widget_handler(&self.lua, id, "OnDoubleClick", vec![btn.clone()])
{
self.push_error(e);
}
} else {
button::click_button(&self.lua, id, button, false, false);
}
// A double clears the stamp (`0x779393`), so clicks pair: four fast clicks are click,
// double, click, double. A single stamps the detector (`0x7793af`) whether or not an
// `OnClick` script is bound.
let mut model = self.model_mut();
if model.arena.frame(h).is_some() {
if double {
model.last_click.remove(&h);
} else {
model.last_click.insert(h, now);
}
}
}
// The release edge, `SetButtonState(NORMAL)` (`0x7793de`).
button::edge(&mut self.model_mut(), h, ButtonState::on_mouse_up);
}
/// Fire `OnDragStop` on a gesture's source, the one place a started drag ends; a source that
/// died since the press fires nothing.
pub(super) fn fire_drag_stop(&mut self, source: crate::widget::FrameHandle) {
crates/benilla-ui/src/script/tests/capture.rs
+121
View File
@@ -363,3 +363,124 @@ fn the_press_art_comes_after_on_mouse_down_and_the_down_click() {
"PUSHED"
);
}
// ── The release slot's order: OnMouseUp, then the click, then NORMAL (`0x7792d0`) ───────────────
/// A Button `A` that logs the state its `OnClick` and `OnDoubleClick` read.
fn click_state_probe(s: &mut UiScript, body: &str) {
s.run(&format!(
r#"
A:SetScript("OnClick", function() table.insert(log, "click:"..A:GetButtonState()) {body} end)
A:SetScript("OnDoubleClick", function() table.insert(log, "double:"..A:GetButtonState()) end)
"#
))
.unwrap();
s.mouse_move(200.0, 300.0);
}
fn press_release(s: &mut UiScript) {
s.mouse_button(200.0, 300.0, "LeftButton", true);
s.mouse_button(200.0, 300.0, "LeftButton", false);
}
fn state_of_a(s: &mut UiScript) -> String {
s.eval::<String>("return A:GetButtonState()").unwrap()
}
/// The mouse-up handler fires `OnClick` (`0x7793a9`) and only then sets NORMAL (`0x7793de`), so a
/// click registered for the release reads PUSHED; one registered for the press reads NORMAL
/// (`0x779285` before `0x7792b1`).
#[test]
fn on_click_reads_pushed_on_a_release_click_and_normal_on_a_press_click() {
let mut s = two_frames("Button");
click_state_probe(&mut s, "");
press_release(&mut s);
assert_eq!(
log_of(&mut s),
"A:enter A:down:LeftButton A:up:LeftButton click:PUSHED"
);
assert_eq!(
state_of_a(&mut s),
"NORMAL",
"…and the release then un-pushes it"
);
let mut s = two_frames("Button");
s.run(r#"A:RegisterForClicks("LeftButtonDown")"#).unwrap();
click_state_probe(&mut s, "");
press_release(&mut s);
assert!(
log_of(&mut s).contains("click:NORMAL"),
"{}",
log_of(&mut s)
);
assert!(
!log_of(&mut s).contains("click:PUSHED"),
"{}",
log_of(&mut s)
);
let mut s = two_frames("Button");
s.run(r#"A:RegisterForClicks("LeftButtonDown", "LeftButtonUp")"#)
.unwrap();
click_state_probe(&mut s, "");
press_release(&mut s);
assert!(
log_of(&mut s).ends_with("click:NORMAL A:up:LeftButton click:PUSHED"),
"{}",
log_of(&mut s)
);
}
/// `OnDoubleClick` replaces the second `OnClick` (`0x77938d`) and runs before the same NORMAL
/// write, so it reads PUSHED too.
#[test]
fn on_double_click_reads_pushed() {
let mut s = two_frames("Button");
click_state_probe(&mut s, "");
press_release(&mut s);
press_release(&mut s);
assert!(
log_of(&mut s).ends_with("A:up:LeftButton double:PUSHED"),
"{}",
log_of(&mut s)
);
assert_eq!(state_of_a(&mut s), "NORMAL");
}
/// After the click the handler sets NORMAL only if `[+0x32c]` (the lock) is 0 and `[+0x328]` (the
/// state) is not DISABLED (`0x7793c2`..`0x7793de`): an `OnClick` that pushes the button unlocked is
/// released by the tail, one that locks it keeps what it set.
#[test]
fn the_tail_after_the_click_honours_what_the_click_set() {
let mut s = two_frames("Button");
click_state_probe(&mut s, r#"A:SetButtonState("PUSHED")"#);
press_release(&mut s);
assert_eq!(state_of_a(&mut s), "NORMAL", "unlocked: the tail un-pushes");
let mut s = two_frames("Button");
click_state_probe(&mut s, r#"A:SetButtonState("PUSHED", 1)"#);
press_release(&mut s);
assert_eq!(state_of_a(&mut s), "PUSHED", "locked: the tail is skipped");
let mut s = two_frames("Button");
click_state_probe(&mut s, "A:Disable()");
press_release(&mut s);
assert_eq!(
state_of_a(&mut s),
"DISABLED",
"disabled: the tail is skipped"
);
}
/// The state is tested after the base slot's `OnMouseUp` script has run (`0x76c099`, then
/// `0x7792fc`): an `OnMouseUp` that un-pushes the button cancels the click.
#[test]
fn an_on_mouse_up_that_unpushes_the_button_cancels_the_click() {
let mut s = two_frames("Button");
click_state_probe(&mut s, "");
s.run(r#"A:SetScript("OnMouseUp", function() table.insert(log, "up:"..A:GetButtonState()) A:SetButtonState("NORMAL") end)"#)
.unwrap();
press_release(&mut s);
assert_eq!(log_of(&mut s), "A:enter A:down:LeftButton up:PUSHED");
}