fix: address 4th-pass review — visibility ordering, mouse_wheel tests, poll diagnostics

- actionability::visibility_check: run HIDDEN/OFFSCREEN state checks BEFORE the
  bounds read (P1 #2). Previously bounds=None short-circuited to unknown, so a
  hidden/offscreen element whose live bounds read failed slipped the visibility
  gate. Regression tests added (hidden/offscreen + bounds=None now fail, not unknown).
- mouse_wheel: add unit tests (P0 #1) mirroring the sibling mouse-command tests —
  args reach the adapter unchanged, the scrolled envelope shape, error propagation.
- ref_action_wait poll loop: on a retryable resolve failure, record the resolve
  error into last_report so an actionability_timeout on deadline expiry carries
  the last STALE_REF/AMBIGUOUS_TARGET context instead of an empty report (P2 #7).

Already-fixed in prior passes (review base predates 640e8e8): StepMechanism serde
tests (#3), clipboard trait removal doc note (#10). Deferred/accepted: test-adapter
boilerplate (#5 -> issue #95), hit-test ancestor + scroll thrashing (#15/#16/#17,
intentional design tradeoffs). Verified with cargo test --workspace (1186 passed).
This commit is contained in:
Lahfir 2026-07-05 16:59:28 -07:00
parent 19f2ef4a86
commit a8b5a59d1c
5 changed files with 173 additions and 3 deletions

View file

@ -117,15 +117,15 @@ fn check_with_stability(
}
fn visibility_check(entry: &RefEntry) -> ActionabilityCheck {
let Some(bounds) = entry.bounds else {
return unknown("visible", "bounds unavailable");
};
if state::has_state(&entry.states, state::HIDDEN) {
return fail("visible", "entry state contains hidden");
}
if state::has_state(&entry.states, state::OFFSCREEN) {
return fail("visible", "entry state contains offscreen");
}
let Some(bounds) = entry.bounds else {
return unknown("visible", "bounds unavailable");
};
if !bounds_are_visible(Some(bounds)) {
return fail("visible", "bounds are zero-sized");
}

View file

@ -99,6 +99,34 @@ fn offscreen_state_fails_visibility_before_action_dispatch() {
assert!(err.message.contains("visible"));
}
#[test]
fn hidden_entry_fails_visibility_even_when_bounds_are_none() {
let mut entry = entry();
entry.states.push(crate::state::HIDDEN.into());
entry.bounds = None;
entry.bounds_hash = None;
let err = check(&entry, &ActionRequest::headless(Action::Click)).unwrap_err();
assert_eq!(err.code, ErrorCode::ActionFailed);
assert!(err.message.contains("visible"));
assert!(err.message.contains("hidden"));
}
#[test]
fn offscreen_entry_fails_visibility_even_when_bounds_are_none() {
let mut entry = entry();
entry.states.push(crate::state::OFFSCREEN.into());
entry.bounds = None;
entry.bounds_hash = None;
let err = check(&entry, &ActionRequest::headless(Action::Click)).unwrap_err();
assert_eq!(err.code, ErrorCode::ActionFailed);
assert!(err.message.contains("visible"));
assert!(err.message.contains("offscreen"));
}
#[test]
fn text_input_requires_editable_target() {
let err = check(

View file

@ -13,3 +13,7 @@ pub fn execute(args: MouseWheelArgs, adapter: &dyn PlatformAdapter) -> Result<Va
adapter.mouse_wheel(args.x, args.y, args.dy, args.dx, &args.modifiers)?;
Ok(json!({ "scrolled": true, "dy": args.dy, "dx": args.dx }))
}
#[cfg(test)]
#[path = "mouse_wheel_tests.rs"]
mod tests;

View file

@ -0,0 +1,131 @@
use super::*;
use crate::action::Modifier;
use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
use crate::error::AdapterError;
use std::sync::Mutex;
#[derive(Debug, PartialEq)]
struct WheelCall {
x: f64,
y: f64,
dy: i32,
dx: i32,
modifiers: Vec<Modifier>,
}
struct WheelCaptureAdapter {
captured: Mutex<Option<WheelCall>>,
fail: bool,
}
impl WheelCaptureAdapter {
fn recording() -> Self {
Self {
captured: Mutex::new(None),
fail: false,
}
}
fn failing() -> Self {
Self {
captured: Mutex::new(None),
fail: true,
}
}
}
impl ObservationOps for WheelCaptureAdapter {}
impl ActionOps for WheelCaptureAdapter {}
impl SystemOps for WheelCaptureAdapter {}
impl InputOps for WheelCaptureAdapter {
fn mouse_wheel(
&self,
x: f64,
y: f64,
dy: i32,
dx: i32,
modifiers: &[Modifier],
) -> Result<(), AdapterError> {
*self.captured.lock().unwrap() = Some(WheelCall {
x,
y,
dy,
dx,
modifiers: modifiers.to_vec(),
});
if self.fail {
return Err(AdapterError::not_supported("mouse_wheel"));
}
Ok(())
}
}
#[test]
fn requested_wheel_args_reach_the_adapter_unchanged() {
let adapter = WheelCaptureAdapter::recording();
execute(
MouseWheelArgs {
x: 10.0,
y: 20.0,
dy: -3,
dx: 5,
modifiers: vec![Modifier::Shift, Modifier::Alt],
},
&adapter,
)
.unwrap();
let captured = adapter.captured.lock().unwrap();
let call = captured
.as_ref()
.expect("mouse_wheel must have been called");
assert_eq!(
*call,
WheelCall {
x: 10.0,
y: 20.0,
dy: -3,
dx: 5,
modifiers: vec![Modifier::Shift, Modifier::Alt],
}
);
}
#[test]
fn returns_scrolled_envelope_with_requested_deltas() {
let adapter = WheelCaptureAdapter::recording();
let value = execute(
MouseWheelArgs {
x: 0.0,
y: 0.0,
dy: 7,
dx: -2,
modifiers: Vec::new(),
},
&adapter,
)
.unwrap();
assert_eq!(value, json!({ "scrolled": true, "dy": 7, "dx": -2 }));
}
#[test]
fn adapter_error_propagates_as_err() {
let adapter = WheelCaptureAdapter::failing();
let result = execute(
MouseWheelArgs {
x: 1.0,
y: 2.0,
dy: 1,
dx: 0,
modifiers: Vec::new(),
},
&adapter,
);
assert!(result.is_err());
}

View file

@ -226,6 +226,13 @@ fn execute_poll_loop(
trace_resolve_error(ctx.context, ctx.ref_id, &err);
return Err(err);
}
last_report = Some(json!({
"resolve_error": {
"code": code.as_str(),
"message": err.message.clone(),
"details": err.details.clone(),
}
}));
sleep_poll_interval(deadline);
}
}