mirror of
https://github.com/lahfir/agent-desktop.git
synced 2026-08-09 00:17:27 +00:00
fix: three-way hit_test with probe-error Unknown and occluder detail
HitTestResult becomes ReachesTarget | InterceptedBy { role, name, bounds }
| Unknown instead of a boolean, so a probe failure or a hit on the
target's own ancestor (composited/custom-drawn containers) is never
reported as a false occlusion Fail. receives_events_check maps the
three states to Pass/Fail/Unknown and carries the occluder's role and
redactable name in ActionabilityCheck.occluder instead of a hardcoded
string. requires_hit_test() now covers Drag. visibility_check also
fails on the HIDDEN/OFFSCREEN state vocabulary, not just zero bounds.
macOS hit_test_impl reuses the bounded ax_helpers::try_each_ancestor
walk instead of a hand-rolled unbounded AXParent loop, and treats
every probe failure (null/zero bounds, missing pid, AX error) as
Unknown rather than a false Fail.
hover --ref and drag --from/--to previously resolved a ref straight to
a center point via point_resolve without ever consulting hit_test, so
an occluded target dispatched blind. resolve_point_from_ref_or_xy_with_context
now runs the new receives_events-only check on the ref-targeted path
before returning the point; raw --xy input is unaffected by design.
This commit is contained in:
parent
a62805a89c
commit
d18b70ef93
14 changed files with 689 additions and 101 deletions
|
|
@ -59,7 +59,12 @@ impl Action {
|
|||
pub fn requires_hit_test(&self) -> bool {
|
||||
matches!(
|
||||
self,
|
||||
Self::Click | Self::DoubleClick | Self::RightClick | Self::TripleClick | Self::Hover
|
||||
Self::Click
|
||||
| Self::DoubleClick
|
||||
| Self::RightClick
|
||||
| Self::TripleClick
|
||||
| Self::Hover
|
||||
| Self::Drag(_)
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -123,3 +123,47 @@ fn hover_and_drag_base_policy_is_headless_independent_of_cursor_requirement() {
|
|||
"Drag.requires_cursor_policy() must still be true"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn requires_hit_test_covers_ref_targeted_pointer_actions() {
|
||||
let hit_tested: &[Action] = &[
|
||||
Action::Click,
|
||||
Action::DoubleClick,
|
||||
Action::RightClick,
|
||||
Action::TripleClick,
|
||||
Action::Hover,
|
||||
Action::Drag(dummy_drag()),
|
||||
];
|
||||
for action in hit_tested {
|
||||
assert!(
|
||||
action.requires_hit_test(),
|
||||
"{} must require a hit test before dispatch",
|
||||
action.name()
|
||||
);
|
||||
}
|
||||
|
||||
let not_hit_tested: &[Action] = &[
|
||||
Action::SetValue("v".into()),
|
||||
Action::SetFocus,
|
||||
Action::Expand,
|
||||
Action::Collapse,
|
||||
Action::Select("s".into()),
|
||||
Action::Toggle,
|
||||
Action::Check,
|
||||
Action::Uncheck,
|
||||
Action::Scroll(Direction::Down, 1),
|
||||
Action::ScrollTo,
|
||||
Action::PressKey(dummy_key()),
|
||||
Action::KeyDown(dummy_key()),
|
||||
Action::KeyUp(dummy_key()),
|
||||
Action::TypeText("t".into()),
|
||||
Action::Clear,
|
||||
];
|
||||
for action in not_hit_tested {
|
||||
assert!(
|
||||
!action.requires_hit_test(),
|
||||
"{} must not require a hit test",
|
||||
action.name()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,10 +1,27 @@
|
|||
use super::ActionabilityStatus;
|
||||
use crate::node::Rect;
|
||||
use serde::Serialize;
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq, Eq)]
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub struct ActionabilityCheck {
|
||||
pub name: &'static str,
|
||||
pub status: ActionabilityStatus,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub reason: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub occluder: Option<Occluder>,
|
||||
}
|
||||
|
||||
/// The element a hit test actually landed on when it failed to reach the
|
||||
/// intended target. `name` carries the occluder's accessible name under a
|
||||
/// `name`-keyed field so `sanitize_trace_value` redacts it automatically;
|
||||
/// `role` is a bounded AX vocabulary token, safe to surface in free text.
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub struct Occluder {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub role: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub name: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub bounds: Option<Rect>,
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,11 +1,13 @@
|
|||
use crate::capability;
|
||||
use crate::{
|
||||
action::Action,
|
||||
action::{Action, Point},
|
||||
action_request::ActionRequest,
|
||||
adapter::{NativeHandle, PlatformAdapter},
|
||||
error::{AdapterError, ErrorCode},
|
||||
hit_test::HitTestResult,
|
||||
node::Rect,
|
||||
refs::RefEntry,
|
||||
state,
|
||||
};
|
||||
use serde_json::json;
|
||||
|
||||
|
|
@ -13,7 +15,7 @@ mod check;
|
|||
mod report;
|
||||
mod status;
|
||||
|
||||
pub use check::ActionabilityCheck;
|
||||
pub use check::{ActionabilityCheck, Occluder};
|
||||
pub use report::ActionabilityReport;
|
||||
pub use status::ActionabilityStatus;
|
||||
|
||||
|
|
@ -120,6 +122,12 @@ 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");
|
||||
}
|
||||
if !bounds_are_visible(Some(bounds)) {
|
||||
return fail("visible", "bounds are zero-sized");
|
||||
}
|
||||
|
|
@ -209,7 +217,6 @@ fn receives_events_check(
|
|||
adapter: &dyn PlatformAdapter,
|
||||
request: &ActionRequest,
|
||||
) -> ActionabilityCheck {
|
||||
use crate::action::Point;
|
||||
if !request.action.requires_hit_test() {
|
||||
return pass("receives_events");
|
||||
}
|
||||
|
|
@ -221,20 +228,47 @@ fn receives_events_check(
|
|||
y: bounds.y + bounds.height / 2.0,
|
||||
};
|
||||
match adapter.hit_test(handle, point) {
|
||||
Ok(result) if result.receives_events => pass("receives_events"),
|
||||
Ok(_) => fail("receives_events", "element is occluded at its center"),
|
||||
Err(err)
|
||||
if matches!(
|
||||
err.code,
|
||||
ErrorCode::PlatformNotSupported | ErrorCode::ActionNotSupported
|
||||
) =>
|
||||
{
|
||||
unknown("receives_events", "hit test unavailable")
|
||||
}
|
||||
Err(_) => fail("receives_events", "hit test failed"),
|
||||
Ok(HitTestResult::ReachesTarget) => pass("receives_events"),
|
||||
Ok(HitTestResult::Unknown) => unknown("receives_events", "hit test result inconclusive"),
|
||||
Ok(HitTestResult::InterceptedBy {
|
||||
role,
|
||||
name,
|
||||
bounds: occluder_bounds,
|
||||
}) => occluded(role, name, occluder_bounds),
|
||||
Err(_) => unknown("receives_events", "hit test unavailable"),
|
||||
}
|
||||
}
|
||||
|
||||
/// The occlusion-only counterpart to [`check_live`] for ref-targeted pointer
|
||||
/// commands (`hover`, `drag`) that resolve a point via `point_resolve`
|
||||
/// instead of dispatching an [`ActionRequest`] through `check_live` — they
|
||||
/// have no `supported_action`/`editable` semantics to check, only whether the
|
||||
/// resolved point actually reaches the target. Mirrors the three-way
|
||||
/// [`HitTestResult`] contract: `ReachesTarget` and `Unknown` (including probe
|
||||
/// errors and `not_supported`) both proceed, only `InterceptedBy` fails.
|
||||
pub(crate) fn require_receives_events(
|
||||
handle: &NativeHandle,
|
||||
point: Point,
|
||||
adapter: &dyn PlatformAdapter,
|
||||
) -> Result<(), AdapterError> {
|
||||
let check = match adapter.hit_test(handle, point) {
|
||||
Ok(HitTestResult::ReachesTarget | HitTestResult::Unknown) | Err(_) => return Ok(()),
|
||||
Ok(HitTestResult::InterceptedBy { role, name, bounds }) => occluded(role, name, bounds),
|
||||
};
|
||||
let report = ActionabilityReport {
|
||||
actionable: false,
|
||||
checks: vec![check],
|
||||
};
|
||||
Err(AdapterError::new(
|
||||
ErrorCode::ActionFailed,
|
||||
format!("Target is not actionable: {}", failure_reasons(&report)),
|
||||
)
|
||||
.with_details(json!(report))
|
||||
.with_suggestion(
|
||||
"Wait for the target to become actionable, refresh the snapshot, or use an explicit physical/focus command if intended.",
|
||||
))
|
||||
}
|
||||
|
||||
fn failure_reasons(report: &ActionabilityReport) -> String {
|
||||
report
|
||||
.checks
|
||||
|
|
@ -257,6 +291,7 @@ fn pass(name: &'static str) -> ActionabilityCheck {
|
|||
name,
|
||||
status: ActionabilityStatus::Pass,
|
||||
reason: None,
|
||||
occluder: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -265,6 +300,7 @@ fn fail(name: &'static str, reason: impl Into<String>) -> ActionabilityCheck {
|
|||
name,
|
||||
status: ActionabilityStatus::Fail,
|
||||
reason: Some(reason.into()),
|
||||
occluder: None,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -273,6 +309,24 @@ fn unknown(name: &'static str, reason: impl Into<String>) -> ActionabilityCheck
|
|||
name,
|
||||
status: ActionabilityStatus::Unknown,
|
||||
reason: Some(reason.into()),
|
||||
occluder: None,
|
||||
}
|
||||
}
|
||||
|
||||
fn occluded(
|
||||
role: Option<String>,
|
||||
name: Option<String>,
|
||||
bounds: Option<Rect>,
|
||||
) -> ActionabilityCheck {
|
||||
let reason = match role.as_deref() {
|
||||
Some(role) => format!("occluded by {role}"),
|
||||
None => "occluded by another element".to_string(),
|
||||
};
|
||||
ActionabilityCheck {
|
||||
name: "receives_events",
|
||||
status: ActionabilityStatus::Fail,
|
||||
reason: Some(reason),
|
||||
occluder: Some(Occluder { role, name, bounds }),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
use super::ActionabilityCheck;
|
||||
use serde::Serialize;
|
||||
|
||||
#[derive(Debug, Clone, Serialize, PartialEq, Eq)]
|
||||
#[derive(Debug, Clone, Serialize, PartialEq)]
|
||||
pub struct ActionabilityReport {
|
||||
pub actionable: bool,
|
||||
pub checks: Vec<ActionabilityCheck>,
|
||||
|
|
|
|||
|
|
@ -72,6 +72,26 @@ fn zero_sized_bounds_fail_visibility() {
|
|||
assert!(err.message.contains("visible"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hidden_state_fails_visibility_before_action_dispatch() {
|
||||
let mut entry = entry();
|
||||
entry.states.push(crate::state::HIDDEN.into());
|
||||
|
||||
let err = check(&entry, &ActionRequest::headless(Action::Click)).unwrap_err();
|
||||
|
||||
assert!(err.message.contains("visible"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn offscreen_state_fails_visibility_before_action_dispatch() {
|
||||
let mut entry = entry();
|
||||
entry.states.push(crate::state::OFFSCREEN.into());
|
||||
|
||||
let err = check(&entry, &ActionRequest::headless(Action::Click)).unwrap_err();
|
||||
|
||||
assert!(err.message.contains("visible"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn text_input_requires_editable_target() {
|
||||
let err = check(
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ use crate::{
|
|||
adapter::NativeHandle,
|
||||
capability,
|
||||
error::{AdapterError, ErrorCode},
|
||||
hit_test::HitTestResult,
|
||||
node::Rect,
|
||||
refs::{RefEntry, RefMap},
|
||||
refs_store::RefStore,
|
||||
|
|
@ -297,6 +298,72 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
|
|||
assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
|
||||
}
|
||||
|
||||
struct OccludedFromAdapter {
|
||||
captured: Mutex<Option<DragParams>>,
|
||||
}
|
||||
|
||||
impl ObservationOps for OccludedFromAdapter {
|
||||
fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
|
||||
Ok(NativeHandle::null())
|
||||
}
|
||||
|
||||
fn get_element_bounds(&self, _handle: &NativeHandle) -> Result<Option<Rect>, AdapterError> {
|
||||
Ok(Some(Rect {
|
||||
x: 10.0,
|
||||
y: 20.0,
|
||||
width: 40.0,
|
||||
height: 60.0,
|
||||
}))
|
||||
}
|
||||
|
||||
fn hit_test(
|
||||
&self,
|
||||
_handle: &NativeHandle,
|
||||
_point: crate::action::Point,
|
||||
) -> Result<HitTestResult, AdapterError> {
|
||||
Ok(HitTestResult::InterceptedBy {
|
||||
role: Some("AXSheet".into()),
|
||||
name: Some("Save changes?".into()),
|
||||
bounds: None,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl ActionOps for OccludedFromAdapter {}
|
||||
|
||||
impl InputOps for OccludedFromAdapter {
|
||||
fn drag(&self, params: DragParams) -> Result<(), AdapterError> {
|
||||
*self.captured.lock().unwrap() = Some(params);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl SystemOps for OccludedFromAdapter {}
|
||||
|
||||
/// F27 regression: `drag --from <ref>` previously resolved bounds to a point
|
||||
/// and dispatched without ever consulting `hit_test`, so a `from` ref
|
||||
/// occluded by a modal sheet was dragged blind. This proves the preflight
|
||||
/// now fails before `adapter.drag` is ever called.
|
||||
#[test]
|
||||
fn drag_from_occluded_ref_fails_preflight_before_dispatch() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = cross_app_snapshot();
|
||||
let adapter = OccludedFromAdapter {
|
||||
captured: Mutex::new(None),
|
||||
};
|
||||
|
||||
let err = execute(
|
||||
cross_app_args(snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default().with_headed(true),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.code(), "ACTION_FAILED");
|
||||
assert!(err.to_string().contains("AXSheet"));
|
||||
assert!(adapter.captured.lock().unwrap().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn timeout_none_makes_exactly_one_resolve_attempt() {
|
||||
let _guard = HomeGuard::new();
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ use crate::{
|
|||
adapter::NativeHandle,
|
||||
capability,
|
||||
error::{AdapterError, ErrorCode},
|
||||
hit_test::HitTestResult,
|
||||
node::Rect,
|
||||
refs::{RefEntry, RefMap},
|
||||
refs_store::RefStore,
|
||||
|
|
@ -215,6 +216,72 @@ fn transient_stale_ref_retries_then_succeeds_when_timeout_wired() {
|
|||
assert!(adapter.resolve_calls.load(Ordering::SeqCst) >= 3);
|
||||
}
|
||||
|
||||
struct OccludedTargetAdapter {
|
||||
moved_to: Mutex<Option<MouseEvent>>,
|
||||
}
|
||||
|
||||
impl ObservationOps for OccludedTargetAdapter {
|
||||
fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
|
||||
Ok(NativeHandle::null())
|
||||
}
|
||||
|
||||
fn get_element_bounds(&self, _handle: &NativeHandle) -> Result<Option<Rect>, AdapterError> {
|
||||
Ok(Some(Rect {
|
||||
x: 100.0,
|
||||
y: 200.0,
|
||||
width: 20.0,
|
||||
height: 10.0,
|
||||
}))
|
||||
}
|
||||
|
||||
fn hit_test(
|
||||
&self,
|
||||
_handle: &NativeHandle,
|
||||
_point: crate::action::Point,
|
||||
) -> Result<HitTestResult, AdapterError> {
|
||||
Ok(HitTestResult::InterceptedBy {
|
||||
role: Some("AXSheet".into()),
|
||||
name: Some("Save changes?".into()),
|
||||
bounds: None,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
impl ActionOps for OccludedTargetAdapter {}
|
||||
|
||||
impl InputOps for OccludedTargetAdapter {
|
||||
fn mouse_event(&self, event: MouseEvent) -> Result<(), AdapterError> {
|
||||
*self.moved_to.lock().unwrap() = Some(event);
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
impl SystemOps for OccludedTargetAdapter {}
|
||||
|
||||
/// F27 regression: `hover --ref` previously resolved the ref's bounds to a
|
||||
/// point and dispatched the mouse move without ever consulting `hit_test`,
|
||||
/// so an occluded target (e.g. a modal sheet over it) was hovered blind.
|
||||
/// This proves the preflight now fails before any mouse event is sent.
|
||||
#[test]
|
||||
fn hover_on_occluded_ref_fails_preflight_before_dispatch() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = ref_snapshot(42);
|
||||
let adapter = OccludedTargetAdapter {
|
||||
moved_to: Mutex::new(None),
|
||||
};
|
||||
|
||||
let err = execute(
|
||||
ref_args(snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default().with_headed(true),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert_eq!(err.code(), "ACTION_FAILED");
|
||||
assert!(err.to_string().contains("AXSheet"));
|
||||
assert!(adapter.moved_to.lock().unwrap().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn timeout_none_makes_exactly_one_resolve_attempt() {
|
||||
let _guard = HomeGuard::new();
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
use crate::{
|
||||
action::Point,
|
||||
actionability,
|
||||
adapter::PlatformAdapter,
|
||||
commands::helpers::resolve_ref_with_context,
|
||||
context::CommandContext,
|
||||
|
|
@ -44,11 +45,13 @@ pub(crate) fn resolve_point_from_ref_or_xy_with_context(
|
|||
let bounds = adapter
|
||||
.get_element_bounds(handle.handle())?
|
||||
.ok_or_else(|| AppError::invalid_input(format!("Element {ref_id} has no bounds")))?;
|
||||
let point = Point {
|
||||
x: bounds.x + bounds.width / 2.0,
|
||||
y: bounds.y + bounds.height / 2.0,
|
||||
};
|
||||
actionability::require_receives_events(handle.handle(), point.clone(), adapter)?;
|
||||
return Ok(ResolvedPoint {
|
||||
point: Point {
|
||||
x: bounds.x + bounds.width / 2.0,
|
||||
y: bounds.y + bounds.height / 2.0,
|
||||
},
|
||||
point,
|
||||
pid: Some(entry.pid),
|
||||
});
|
||||
}
|
||||
|
|
@ -87,18 +90,5 @@ pub(crate) fn focus_for_physical_input(
|
|||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn physical_input_requires_headed_context() {
|
||||
let err = require_cursor_policy(&CommandContext::default(), "mouse-move").unwrap_err();
|
||||
|
||||
assert_eq!(err.code(), "POLICY_DENIED");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headed_context_allows_physical_input() {
|
||||
require_cursor_policy(&CommandContext::default().with_headed(true), "mouse-move").unwrap();
|
||||
}
|
||||
}
|
||||
#[path = "point_resolve_tests.rs"]
|
||||
mod tests;
|
||||
|
|
|
|||
206
crates/core/src/commands/point_resolve_tests.rs
Normal file
206
crates/core/src/commands/point_resolve_tests.rs
Normal file
|
|
@ -0,0 +1,206 @@
|
|||
use super::*;
|
||||
use crate::adapter::{ActionOps, InputOps, ObservationOps, SystemOps};
|
||||
use crate::{
|
||||
adapter::NativeHandle,
|
||||
capability,
|
||||
error::ErrorCode,
|
||||
hit_test::HitTestResult,
|
||||
node::Rect,
|
||||
refs::{RefEntry, RefMap},
|
||||
refs_store::RefStore,
|
||||
refs_test_support::HomeGuard,
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn physical_input_requires_headed_context() {
|
||||
let err = require_cursor_policy(&CommandContext::default(), "mouse-move").unwrap_err();
|
||||
|
||||
assert_eq!(err.code(), "POLICY_DENIED");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn headed_context_allows_physical_input() {
|
||||
require_cursor_policy(&CommandContext::default().with_headed(true), "mouse-move").unwrap();
|
||||
}
|
||||
|
||||
/// An adapter whose `hit_test` outcome is fixed per test, so each occlusion
|
||||
/// scenario (F27) exercises the real `resolve_point_from_ref_or_xy_with_context`
|
||||
/// path rather than a mock that always echoes success.
|
||||
struct HitTestOutcomeAdapter {
|
||||
outcome: Result<HitTestResult, AdapterError>,
|
||||
}
|
||||
|
||||
impl ObservationOps for HitTestOutcomeAdapter {
|
||||
fn resolve_element_strict(&self, _entry: &RefEntry) -> Result<NativeHandle, AdapterError> {
|
||||
Ok(NativeHandle::null())
|
||||
}
|
||||
|
||||
fn get_element_bounds(&self, _handle: &NativeHandle) -> Result<Option<Rect>, AdapterError> {
|
||||
Ok(Some(Rect {
|
||||
x: 100.0,
|
||||
y: 200.0,
|
||||
width: 20.0,
|
||||
height: 10.0,
|
||||
}))
|
||||
}
|
||||
|
||||
fn hit_test(
|
||||
&self,
|
||||
_handle: &NativeHandle,
|
||||
_point: Point,
|
||||
) -> Result<HitTestResult, AdapterError> {
|
||||
self.outcome.clone()
|
||||
}
|
||||
}
|
||||
|
||||
impl ActionOps for HitTestOutcomeAdapter {}
|
||||
impl InputOps for HitTestOutcomeAdapter {}
|
||||
impl SystemOps for HitTestOutcomeAdapter {}
|
||||
|
||||
fn ref_snapshot(pid: i32) -> String {
|
||||
let store = RefStore::new().unwrap();
|
||||
let mut refmap = RefMap::new();
|
||||
refmap.allocate(RefEntry {
|
||||
pid,
|
||||
role: "button".into(),
|
||||
name: Some("Target".into()),
|
||||
value: None,
|
||||
description: None,
|
||||
native_id: None,
|
||||
states: vec![],
|
||||
bounds: None,
|
||||
bounds_hash: None,
|
||||
available_actions: vec![capability::CLICK.into()],
|
||||
source_app: None,
|
||||
source_window_id: None,
|
||||
source_window_title: None,
|
||||
source_surface: crate::adapter::SnapshotSurface::Window,
|
||||
root_ref: None,
|
||||
path_is_absolute: false,
|
||||
path: smallvec::SmallVec::new(),
|
||||
});
|
||||
store.save_new_snapshot(&refmap).unwrap()
|
||||
}
|
||||
|
||||
fn ref_args(snapshot_id: &str) -> PointResolveArgs<'_> {
|
||||
PointResolveArgs {
|
||||
ref_id: Some("@e1"),
|
||||
xy: None,
|
||||
snapshot_id: Some(snapshot_id),
|
||||
missing_input_message: "Provide a ref (@e1) or --xy x,y",
|
||||
}
|
||||
}
|
||||
|
||||
/// F27 regression: previously the ref-targeted path never called
|
||||
/// `adapter.hit_test` at all, so an occluded target resolved to a point and
|
||||
/// dispatch proceeded blind. This proves `InterceptedBy` now blocks
|
||||
/// resolution and names the occluder.
|
||||
#[test]
|
||||
fn intercepted_by_blocks_ref_targeted_point_resolution() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = ref_snapshot(42);
|
||||
let adapter = HitTestOutcomeAdapter {
|
||||
outcome: Ok(HitTestResult::InterceptedBy {
|
||||
role: Some("AXSheet".into()),
|
||||
name: Some("Save changes?".into()),
|
||||
bounds: None,
|
||||
}),
|
||||
};
|
||||
|
||||
let result = resolve_point_from_ref_or_xy_with_context(
|
||||
ref_args(&snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default(),
|
||||
);
|
||||
let err = match result {
|
||||
Ok(_) => panic!("occluded ref target must not resolve to a point"),
|
||||
Err(err) => err,
|
||||
};
|
||||
|
||||
assert_eq!(err.code(), ErrorCode::ActionFailed.as_str());
|
||||
let message = err.to_string();
|
||||
assert!(message.contains("AXSheet"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reaches_target_allows_ref_targeted_point_resolution() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = ref_snapshot(42);
|
||||
let adapter = HitTestOutcomeAdapter {
|
||||
outcome: Ok(HitTestResult::ReachesTarget),
|
||||
};
|
||||
|
||||
let resolved = resolve_point_from_ref_or_xy_with_context(
|
||||
ref_args(&snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(resolved.point.x, 110.0);
|
||||
assert_eq!(resolved.point.y, 205.0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_hit_test_result_does_not_block_ref_targeted_resolution() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = ref_snapshot(42);
|
||||
let adapter = HitTestOutcomeAdapter {
|
||||
outcome: Ok(HitTestResult::Unknown),
|
||||
};
|
||||
|
||||
resolve_point_from_ref_or_xy_with_context(
|
||||
ref_args(&snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default(),
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
/// A hit-test probe error must never be treated as occlusion (the reliability
|
||||
/// learning's evidence rule) — resolution proceeds exactly as if hit-testing
|
||||
/// were unavailable.
|
||||
#[test]
|
||||
fn hit_test_probe_error_does_not_block_ref_targeted_resolution() {
|
||||
let _guard = HomeGuard::new();
|
||||
let snapshot_id = ref_snapshot(42);
|
||||
let adapter = HitTestOutcomeAdapter {
|
||||
outcome: Err(AdapterError::internal(
|
||||
"AXUIElementCopyElementAtPosition failed",
|
||||
)),
|
||||
};
|
||||
|
||||
resolve_point_from_ref_or_xy_with_context(
|
||||
ref_args(&snapshot_id),
|
||||
&adapter,
|
||||
&CommandContext::default(),
|
||||
)
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
/// Raw `--xy` input stays raw by design (KTD4): no ref means no occlusion
|
||||
/// check, even against an adapter that would otherwise report occlusion.
|
||||
#[test]
|
||||
fn raw_xy_input_never_calls_hit_test() {
|
||||
let adapter = HitTestOutcomeAdapter {
|
||||
outcome: Ok(HitTestResult::InterceptedBy {
|
||||
role: Some("AXSheet".into()),
|
||||
name: None,
|
||||
bounds: None,
|
||||
}),
|
||||
};
|
||||
|
||||
let resolved = resolve_point_from_ref_or_xy_with_context(
|
||||
PointResolveArgs {
|
||||
ref_id: None,
|
||||
xy: Some((5.0, 6.0)),
|
||||
snapshot_id: None,
|
||||
missing_input_message: "Provide a ref (@e1) or --xy x,y",
|
||||
},
|
||||
&adapter,
|
||||
&CommandContext::default(),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!((resolved.point.x, resolved.point.y), (5.0, 6.0));
|
||||
}
|
||||
|
|
@ -1,26 +1,25 @@
|
|||
use crate::node::Rect;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
|
||||
pub struct HitTestResult {
|
||||
pub receives_events: bool,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub topmost_role: Option<String>,
|
||||
}
|
||||
|
||||
impl HitTestResult {
|
||||
pub fn receives_events(topmost_role: Option<String>) -> Self {
|
||||
Self {
|
||||
receives_events: true,
|
||||
topmost_role,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn blocked(topmost_role: Option<String>) -> Self {
|
||||
Self {
|
||||
receives_events: false,
|
||||
topmost_role,
|
||||
}
|
||||
}
|
||||
/// Classifies whether a hit-tested point reaches the intended target. A hit
|
||||
/// on the target itself or one of its descendants reaches it; a hit outside
|
||||
/// the target's ancestor chain names a real occluder (modal, overlay,
|
||||
/// sibling); a hit on the target's own ancestor is `Unknown` rather than a
|
||||
/// false occlusion, since composited or custom-drawn views often expose no
|
||||
/// distinct child node to hit-test. Probe failures are `Unknown` for the
|
||||
/// same reason: unavailable evidence is never a false failure.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
|
||||
pub enum HitTestResult {
|
||||
ReachesTarget,
|
||||
InterceptedBy {
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
role: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
name: Option<String>,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
bounds: Option<Rect>,
|
||||
},
|
||||
Unknown,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ use super::*;
|
|||
use crate::{
|
||||
action::Action,
|
||||
action_request::ActionRequest,
|
||||
actionability::check_live,
|
||||
actionability::{ActionabilityCheck, ActionabilityStatus, check_live},
|
||||
adapter::{ActionOps, InputOps, LiveElement, NativeHandle, ObservationOps, SystemOps},
|
||||
element_state::ElementState,
|
||||
error::{AdapterError, ErrorCode},
|
||||
|
|
@ -11,7 +11,7 @@ use crate::{
|
|||
use smallvec::SmallVec;
|
||||
|
||||
struct HitTestAdapter {
|
||||
receives: bool,
|
||||
outcome: Result<HitTestResult, AdapterError>,
|
||||
}
|
||||
|
||||
impl ObservationOps for HitTestAdapter {
|
||||
|
|
@ -41,11 +41,7 @@ impl ObservationOps for HitTestAdapter {
|
|||
_handle: &NativeHandle,
|
||||
_point: crate::action::Point,
|
||||
) -> Result<HitTestResult, AdapterError> {
|
||||
Ok(if self.receives {
|
||||
HitTestResult::receives_events(Some("AXButton".into()))
|
||||
} else {
|
||||
HitTestResult::blocked(Some("AXGroup".into()))
|
||||
})
|
||||
self.outcome.clone()
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -80,21 +76,84 @@ fn clickable_entry() -> RefEntry {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn occluded_target_fails_receives_events_check() {
|
||||
let adapter = HitTestAdapter { receives: false };
|
||||
/// Runs `check_live` for a `Click` (which requires a hit test) against the
|
||||
/// given hit-test outcome and returns the `receives_events` check. Only the
|
||||
/// `InterceptedBy` outcome fails actionability, so every other outcome is
|
||||
/// safe to unwrap here; the `InterceptedBy` case is asserted separately
|
||||
/// against the `Err` path so its occluder details can be inspected.
|
||||
fn run_receives_events_check(outcome: Result<HitTestResult, AdapterError>) -> ActionabilityCheck {
|
||||
let adapter = HitTestAdapter { outcome };
|
||||
let entry = clickable_entry();
|
||||
let request = ActionRequest::headless(Action::Click);
|
||||
let err = check_live(&entry, &NativeHandle::null(), &adapter, &request)
|
||||
.expect_err("occluded targets must fail actionability");
|
||||
assert_eq!(err.code, ErrorCode::ActionFailed);
|
||||
assert!(err.message.contains("receives_events"));
|
||||
let report = check_live(&entry, &NativeHandle::null(), &adapter, &request)
|
||||
.expect("only an InterceptedBy hit-test outcome fails actionability");
|
||||
report
|
||||
.checks
|
||||
.into_iter()
|
||||
.find(|check| check.name == "receives_events")
|
||||
.expect("Click requires a receives_events check")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unoccluded_target_passes_receives_events_check() {
|
||||
let adapter = HitTestAdapter { receives: true };
|
||||
fn reaches_target_result_passes_receives_events_check() {
|
||||
let check = run_receives_events_check(Ok(HitTestResult::ReachesTarget));
|
||||
assert_eq!(check.status, ActionabilityStatus::Pass);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unknown_hit_test_result_does_not_block_action() {
|
||||
let check = run_receives_events_check(Ok(HitTestResult::Unknown));
|
||||
assert_eq!(check.status, ActionabilityStatus::Unknown);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn not_supported_hit_test_does_not_block_action() {
|
||||
let check = run_receives_events_check(Err(AdapterError::not_supported("hit_test")));
|
||||
assert_eq!(check.status, ActionabilityStatus::Unknown);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn hit_test_probe_error_does_not_block_action() {
|
||||
let check = run_receives_events_check(Err(AdapterError::internal(
|
||||
"AXUIElementCopyElementAtPosition failed",
|
||||
)));
|
||||
assert_eq!(
|
||||
check.status,
|
||||
ActionabilityStatus::Unknown,
|
||||
"a probe failure must never be reported as a Fail"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn intercepted_by_result_fails_and_carries_redactable_occluder() {
|
||||
let adapter = HitTestAdapter {
|
||||
outcome: Ok(HitTestResult::InterceptedBy {
|
||||
role: Some("AXSheet".into()),
|
||||
name: Some("Save changes?".into()),
|
||||
bounds: None,
|
||||
}),
|
||||
};
|
||||
let entry = clickable_entry();
|
||||
let request = ActionRequest::headless(Action::Click);
|
||||
check_live(&entry, &NativeHandle::null(), &adapter, &request).expect("clickable target");
|
||||
|
||||
let err = check_live(&entry, &NativeHandle::null(), &adapter, &request)
|
||||
.expect_err("a hit outside the target's ancestor chain must fail actionability");
|
||||
|
||||
assert_eq!(err.code, ErrorCode::ActionFailed);
|
||||
assert!(err.message.contains("AXSheet"));
|
||||
assert!(!err.message.contains("Save changes?"));
|
||||
|
||||
let details = err
|
||||
.details
|
||||
.expect("a Fail report attaches actionability details");
|
||||
let checks = details["checks"]
|
||||
.as_array()
|
||||
.expect("details.checks is an array");
|
||||
let receives_events = checks
|
||||
.iter()
|
||||
.find(|check| check["name"] == "receives_events")
|
||||
.expect("receives_events check must be present when Click requires a hit test");
|
||||
assert_eq!(receives_events["status"], "fail");
|
||||
assert_eq!(receives_events["occluder"]["name"], "Save changes?");
|
||||
assert_eq!(receives_events["occluder"]["role"], "AXSheet");
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,14 +1,22 @@
|
|||
#[cfg(target_os = "macos")]
|
||||
mod imp {
|
||||
use crate::actions::ax_helpers;
|
||||
use crate::tree::{
|
||||
AXElement, capabilities::same_element, copy_element_attr, copy_string_attr, read_bounds,
|
||||
AXElement, capabilities::same_element, element::ABSOLUTE_MAX_DEPTH, read_bounds,
|
||||
resolve_element_name,
|
||||
};
|
||||
use accessibility_sys::{AXUIElementCopyElementAtPosition, kAXErrorSuccess, kAXRoleAttribute};
|
||||
use accessibility_sys::{AXUIElementCopyElementAtPosition, kAXErrorSuccess};
|
||||
use agent_desktop_core::{
|
||||
action::Point, error::AdapterError, hit_test::HitTestResult, native_handle::NativeHandle,
|
||||
};
|
||||
use std::mem::ManuallyDrop;
|
||||
|
||||
/// Hit-tests `point` against `target`'s owning application, then
|
||||
/// classifies the result against `target`'s ancestor chain: a hit on
|
||||
/// `target` or a descendant reaches it, a hit outside that chain names a
|
||||
/// real occluder, and a hit on `target`'s own ancestor — like any probe
|
||||
/// failure — is `Unknown` rather than a false occlusion, since composited
|
||||
/// or custom-drawn views often expose no distinct child node to hit-test.
|
||||
pub fn hit_test_impl(
|
||||
handle: &NativeHandle,
|
||||
point: Point,
|
||||
|
|
@ -28,46 +36,61 @@ mod imp {
|
|||
|
||||
fn hit_test_element(target: &AXElement, point: Point) -> Result<HitTestResult, AdapterError> {
|
||||
let Some(bounds) = read_bounds(target) else {
|
||||
return Err(AdapterError::new(
|
||||
agent_desktop_core::error::ErrorCode::ActionFailed,
|
||||
"Element bounds unavailable for hit test",
|
||||
));
|
||||
return Ok(HitTestResult::Unknown);
|
||||
};
|
||||
if bounds.width <= 0.0 || bounds.height <= 0.0 {
|
||||
return Ok(HitTestResult::blocked(None));
|
||||
return Ok(HitTestResult::Unknown);
|
||||
}
|
||||
let pid = crate::system::app_ops::pid_from_element(target)
|
||||
.ok_or_else(|| AdapterError::internal("Could not read pid for hit test"))?;
|
||||
let Some(pid) = crate::system::app_ops::pid_from_element(target) else {
|
||||
return Ok(HitTestResult::Unknown);
|
||||
};
|
||||
let app = crate::tree::element_for_pid(pid);
|
||||
let mut hit_ref: accessibility_sys::AXUIElementRef = std::ptr::null_mut();
|
||||
let err = unsafe {
|
||||
AXUIElementCopyElementAtPosition(app.0, point.x as f32, point.y as f32, &mut hit_ref)
|
||||
};
|
||||
if err != kAXErrorSuccess || hit_ref.is_null() {
|
||||
return Ok(HitTestResult::blocked(None));
|
||||
return Ok(HitTestResult::Unknown);
|
||||
}
|
||||
let hit = AXElement(hit_ref);
|
||||
let topmost_role = copy_string_attr(&hit, kAXRoleAttribute);
|
||||
let receives = element_contains(target, &hit) || element_contains(&hit, target);
|
||||
Ok(if receives {
|
||||
HitTestResult::receives_events(topmost_role)
|
||||
} else {
|
||||
HitTestResult::blocked(topmost_role)
|
||||
})
|
||||
Ok(classify_hit(target, &hit))
|
||||
}
|
||||
|
||||
fn element_contains(ancestor: &AXElement, candidate: &AXElement) -> bool {
|
||||
if same_element(ancestor, candidate) {
|
||||
return true;
|
||||
fn classify_hit(target: &AXElement, hit: &AXElement) -> HitTestResult {
|
||||
let limit = ABSOLUTE_MAX_DEPTH as usize;
|
||||
let reaches_target = same_element(target, hit)
|
||||
|| ax_helpers::try_each_ancestor(hit, |ancestor| same_element(ancestor, target), limit);
|
||||
let is_ancestor_of_target = !reaches_target
|
||||
&& ax_helpers::try_each_ancestor(target, |ancestor| same_element(ancestor, hit), limit);
|
||||
match classify_relation(reaches_target, is_ancestor_of_target) {
|
||||
HitClassification::ReachesTarget => HitTestResult::ReachesTarget,
|
||||
HitClassification::AncestorOfTarget => HitTestResult::Unknown,
|
||||
HitClassification::Unrelated => HitTestResult::InterceptedBy {
|
||||
role: ax_helpers::element_role(hit),
|
||||
name: resolve_element_name(hit),
|
||||
bounds: read_bounds(hit),
|
||||
},
|
||||
}
|
||||
let mut current = copy_element_attr(candidate, "AXParent");
|
||||
while let Some(parent) = current {
|
||||
if same_element(ancestor, &parent) {
|
||||
return true;
|
||||
}
|
||||
current = copy_element_attr(&parent, "AXParent");
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(super) enum HitClassification {
|
||||
ReachesTarget,
|
||||
AncestorOfTarget,
|
||||
Unrelated,
|
||||
}
|
||||
|
||||
pub(super) fn classify_relation(
|
||||
reaches_target: bool,
|
||||
is_ancestor_of_target: bool,
|
||||
) -> HitClassification {
|
||||
if reaches_target {
|
||||
HitClassification::ReachesTarget
|
||||
} else if is_ancestor_of_target {
|
||||
HitClassification::AncestorOfTarget
|
||||
} else {
|
||||
HitClassification::Unrelated
|
||||
}
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -86,3 +109,7 @@ mod imp {
|
|||
}
|
||||
|
||||
pub use imp::hit_test_impl;
|
||||
|
||||
#[cfg(all(test, target_os = "macos"))]
|
||||
#[path = "hit_test_tests.rs"]
|
||||
mod tests;
|
||||
|
|
|
|||
33
crates/macos/src/tree/hit_test_tests.rs
Normal file
33
crates/macos/src/tree/hit_test_tests.rs
Normal file
|
|
@ -0,0 +1,33 @@
|
|||
use super::imp::{HitClassification, classify_relation};
|
||||
|
||||
#[test]
|
||||
fn self_hit_reaches_target() {
|
||||
assert_eq!(
|
||||
classify_relation(true, false),
|
||||
HitClassification::ReachesTarget
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reaches_target_takes_priority_over_ancestor() {
|
||||
assert_eq!(
|
||||
classify_relation(true, true),
|
||||
HitClassification::ReachesTarget
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ancestor_hit_is_reported_separately_from_unrelated() {
|
||||
assert_eq!(
|
||||
classify_relation(false, true),
|
||||
HitClassification::AncestorOfTarget
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unrelated_hit_is_neither_target_nor_ancestor() {
|
||||
assert_eq!(
|
||||
classify_relation(false, false),
|
||||
HitClassification::Unrelated
|
||||
);
|
||||
}
|
||||
Loading…
Reference in a new issue