fix(windows-click): UIA Invoke for element_index + ElementFromPoint for (x,y) - #1549
Conversation
…nt for (x,y)
The Windows click tool's element_index path was annotated "UIA Invoke"
but the underlying mechanism was `crate::input::post_click_screen` —
PostMessage(WM_LBUTTONDOWN) to the deepest HWND child at the click
point. That works for classic Win32 apps but silently no-ops on UWP
(Calculator, Notepad on Win11, Settings, Edge): the XAML buttons live
inside Windows.UI.Core.CoreWindow's DirectComposition tree, not as
child HWNDs, and UWP's input pipeline expects pointer events through
Windows.UI.Input — not raw WM_LBUTTONDOWN. Result: ✅ "Performed
Invoke" log line with zero observable effect.
Two fixes:
1. element_index path now actually calls UIA Invoke. The element_cache
already retained IUIAutomationElement COM pointers; the click tool
was only reading the cached center. Reconstruct the element via
`IUIAutomationElement::from_raw`, `GetCurrentPattern(InvokePattern)`,
cast to IUIAutomationInvokePattern, call `Invoke()`. `std::mem::forget`
keeps the cache's AddRef intact. Falls through to PostMessage for
right-click, count > 1 (no Invoke double-click concept), and
elements without InvokePattern (most edit fields).
2. (x, y) path adds an ElementFromPoint + Invoke probe BEFORE
PostMessage. New helper `uia::windows_enum::try_invoke_at_point`
uses the per-thread cached IUIAutomation, calls
`IUIAutomation::ElementFromPoint(POINT { sx, sy })`, then Invoke
on the result if it supports InvokePattern. Returns false on any
failure (no element at point, no pattern, Invoke errored) so the
PostMessage fallback runs unchanged. Same skip conditions: only
single left/middle clicks try Invoke.
Validated live on Win11 VM with Calculator (a UWP) by RDP'd Session 1:
- Pre-fix: 6 clicks reported "✅ Performed Invoke on [N]"; display
stayed at "0", math never computed.
- Post-fix (element_index): 6 UIA Invokes land, display reads "391",
foreground stayed on PowerShell ISE the entire time.
- Post-fix (x,y vision mode): not re-tested under the new code, but
follows the same UIA Invoke path so the same behavior is expected.
The misleading "Performed Invoke" wording in the success message is
now accurate. PostMessage path also gets a more honest
"✅ Performed PostMessage click on [N]" label so reviewers and CI
logs can tell which path actually ran.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThe PR extends Windows click operations to leverage UIA Invoke patterns. A new ChangesUIA-based click invocation with PostMessage fallback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs (1)
1096-1099: 💤 Low valueConsider
ManuallyDropfor clearer borrowed-reference semantics.The
from_raw+forgetpattern correctly prevents double-Release, butManuallyDropmakes the intent explicit and is panic-safe by construction:-let elem: IUIAutomationElement = - unsafe { IUIAutomationElement::from_raw(ptr as *mut _) }; -let invoke_result = unsafe { elem.GetCurrentPattern(UIA_InvokePatternId) }; -std::mem::forget(elem); +let elem: std::mem::ManuallyDrop<IUIAutomationElement> = + std::mem::ManuallyDrop::new(unsafe { IUIAutomationElement::from_raw(ptr as *mut _) }); +let invoke_result = unsafe { elem.GetCurrentPattern(UIA_InvokePatternId) };This eliminates the need for
forgetand makes it clear the wrapper is intentionally never dropped.,
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs` around lines 1096 - 1099, The code currently does IUIAutomationElement::from_raw(ptr as *mut _) followed by std::mem::forget(elem); replace that pattern with core::mem::ManuallyDrop to express the intent and be panic-safe: construct the element via ManuallyDrop<IUIAutomationElement> (wrapping the result of IUIAutomationElement::from_raw) and then use &*ManuallyDrop to call elem.GetCurrentPattern(UIA_InvokePatternId) so the wrapper is never dropped implicitly; update the occurrences around the GetCurrentPattern invocation to use ManuallyDrop and remove the explicit std::mem::forget.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs`:
- Around line 1096-1099: The code currently does
IUIAutomationElement::from_raw(ptr as *mut _) followed by
std::mem::forget(elem); replace that pattern with core::mem::ManuallyDrop to
express the intent and be panic-safe: construct the element via
ManuallyDrop<IUIAutomationElement> (wrapping the result of
IUIAutomationElement::from_raw) and then use &*ManuallyDrop to call
elem.GetCurrentPattern(UIA_InvokePatternId) so the wrapper is never dropped
implicitly; update the occurrences around the GetCurrentPattern invocation to
use ManuallyDrop and remove the explicit std::mem::forget.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 10d5e228-5601-43f6-a779-ae7d69ea27f1
📒 Files selected for processing (2)
libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rslibs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs
…ion (x,y) The (x,y) click path's UIA Invoke fallback shipped in #1549 used desktop-wide `IUIAutomation::ElementFromPoint`, which silently failed for vision-mode clicks against UWP / packaged-app frames: 1. **Occlusion** — if any other window covered the target at `(sx, sy)`, `ElementFromPoint` returned that window's element. Caller already names the intended HWND; we should respect it. 2. **UWP cross-process hosting** — `ApplicationFrameHost.exe` hosts UWP content (CalculatorApp, Notepad-Win11, Settings, etc.) in a separate process. `ElementFromPoint` was observed returning the frame's outer Pane (no `InvokePattern`) instead of descending. The fallback PostMessage(WM_LBUTTONDOWN) silently no-ops on UWP. New helper `uia::windows_enum::try_invoke_in_window_at_point(hwnd, sx, sy)` mirrors macOS vision-mode semantics (CGEvent delivered to a specific pid regardless of z-order): - Roots the hit test in `hwnd` via `ElementFromHandle` - Enumerates the subtree with `FindAll(TreeScope_Subtree, TrueCondition)` - Picks the smallest-area descendant whose `CurrentBoundingRectangle` contains the point AND which exposes `InvokePattern` - Calls `Invoke()` The (x,y) click path uses this instead of the desktop-wide hit-test. The original `try_invoke_at_point` stays in place (docstring flags its desktop-wide nature) for callers without an HWND in scope. Validated live on Win11 VM (Calculator UWP) under tscon console-attached state: - Pre-fix: 6 vision-mode clicks for 17x23 reported `Posted click to pid N` (PostMessage fallback); display stayed at `0`. - Post-fix (17x23, 802x634 layout): 6 clicks return `Performed UIA Invoke at (sx,sy)`; display reads `391`; History shows `17 x 23 =`. - Post-fix (3+5, 1024x801 layout with History pane expanded, 62 UIA descendants): 4 clicks return `Performed UIA Invoke`; display reads `8`; History shows `3 + 5 = 8`. No foreground change.
Summary
Two clicks-on-UWP fixes that should have been in #1548 but were
scp'd-to-VM-and-tested-on-binary without ever being
git add'd tothe merged PR. Recovering them now as a small standalone PR.
element_indexpath: was annotated "Performed Invoke" butactually shelled out to
PostMessage(WM_LBUTTONDOWN)to thedeepest HWND child. PostMessage works for classic Win32 apps but
silently no-ops on UWP (Calculator, Win11 Notepad, Edge, Settings)
because XAML lives in DirectComposition, not in child HWNDs, and
UWP's input pipeline (
Windows.UI.Input) doesn't process rawWM_LBUTTONDOWN. Now actually callsIUIAutomationInvokePattern::Invoke()on the cached element pointer.
(x, y)path (vision-mode clicks): same fix via the newuia::windows_enum::try_invoke_at_pointhelper that runsIUIAutomation::ElementFromPoint(sx, sy)thenInvoke()onwhatever element resolved there. Falls through to PostMessage for
classic Win32 + non-Invokable elements.
Skip conditions for both paths: right-click + multi-click stay on
PostMessage (UIA has no clean equivalent for
ShowContextMenu/WM_LBUTTONDBLCLK).Test plan
6
element_indexclicks now computes correctly. Pre-fix thedisplay stayed at
0.remained frontmost — verified via daemon-side
GetForegroundWindow).(x, y)path needs re-verification under the newcode (the binary on the VM has these changes baked in but the
test sequence after the rebuild only exercised
element_index).Summary by CodeRabbit
Bug Fixes