feat(cua-driver-rs)(windows)(#1623): route Chromium coord clicks through SendInput - #1625
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR consolidates cross-platform HTML test fixtures into a canonical shared repository with documented symlink distribution, adds Windows SendInput-based click synthesis for Chromium browsers (PostMessage doesn't work), injects anti-throttling flags at launch, and refines UIA element selection to prefer coordinate-independent actions for pixel-addressed clicks. ChangesUnified Test Fixture Repository
Chromium-Aware Windows Input Synthesis
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The PR combines cross-domain changes: fixture consolidation (low risk, documentation-heavy), and substantial Windows input architecture (SendInput synthesis, Chromium detection, UIA pattern refinement). The input-dispatch logic introduces new Chromium-specific branching and requires careful validation of UIPI constraints, cursor restoration, coordinate normalization, and partial-insertion error handling. UIA changes expand pattern acceptance and add coordinate-independence filtering, increasing logic density. No single file is particularly large, but the heterogeneous scope (fixtures, mouse input, tool orchestration, UIA) and multiple interdependent changes across the Windows stack demand careful sequential review. Possibly related issues
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 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.
Actionable comments posted: 5
🤖 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.
Inline comments:
In `@libs/cua-driver-fixtures/gesture_panels.html`:
- Around line 98-119: The dragEvents array is not reset between drag runs
causing stale events to accumulate; update the dragstart handler (attached to
src via addEventListener('dragstart', ...)) to clear or reassign dragEvents
(e.g., dragEvents = []) at the start of that function before pushing
'dragstart', so each new drag run starts with a fresh sequence and
window._lastDrag/events reflect only the current run.
In `@libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs`:
- Around line 323-345: In send_click_synthesized, don't ignore
SetForegroundWindow(target): check its boolean return and fail fast if it
returns false so we don't proceed sending events to the wrong window. If
SetForegroundWindow(target) fails, restore the previous cursor (prev_cursor) and
previous foreground window (prev_fg) like the existing SendInput error path, and
bail! with a clear diagnostic mentioning focus denial (same style as the
existing bail! for partial SendInput). Keep the subsequent SetCursorPos(target)
and SendInput-only path for the successful-focus case.
- Around line 248-253: The precheck using post_message_blocked_by_uipi should
not run for the SendInput click path (e.g., inside send_click_synthesized)
because it lacks UIAccess awareness and can incorrectly bail for UIAccess
processes; remove or guard the current if-let block that calls
post_message_blocked_by_uipi so it only executes for the PostMessage path (not
for SendInput/send_click_synthesized), and rely on the existing
SetForegroundWindow/SendInput UIAccess diagnostics for the SendInput branch.
In `@libs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rs`:
- Around line 837-843: The current guard only checks launch_path_opt, path_opt,
and name_opt before calling is_chromium_browser_target/
inject_chromium_anti_throttling_flags, so plain non-AUMID bundle_id aliases are
skipped; update the condition to include the bundle_id variant (e.g. add
bundle_id_opt.is_some() or otherwise detect when target came from bundle_id) so
that when target.as_deref() refers to a Chromium alias you still call
is_chromium_browser_target(t) and then
inject_chromium_anti_throttling_flags(&mut extra_args); keep references to
launch_path_opt, path_opt, name_opt, bundle_id_opt (or the bundle_id source),
target, is_chromium_browser_target, and inject_chromium_anti_throttling_flags.
In `@libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs`:
- Around line 26-30: Remove UIA_SplitButtonControlTypeId from the
coord-independent allowlist so SplitButton is no longer treated as
coord-independent (ensuring InvokePattern remains the primary action instead of
preferring ExpandCollapse); update the declaration that lists control type
constants to drop UIA_SplitButtonControlTypeId from the allowlist and then
remove the UIA_SplitButtonControlTypeId import if it becomes unused elsewhere.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b4d9128-bff6-412c-8623-6fb1394d1f48
📒 Files selected for processing (19)
libs/cua-driver-fixtures/README.mdlibs/cua-driver-fixtures/form_all_inputs.htmllibs/cua-driver-fixtures/gesture_panels.htmllibs/cua-driver-fixtures/interactive.htmllibs/cua-driver-fixtures/test_page.htmllibs/cua-driver-rs/crates/platform-windows/src/input/mod.rslibs/cua-driver-rs/crates/platform-windows/src/input/mouse.rslibs/cua-driver-rs/crates/platform-windows/src/tools/impl_.rslibs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rslibs/cua-driver-rs/tests/integration/fixtures/interactive.htmllibs/cua-driver-rs/tests/integration/fixtures/interactive.htmllibs/cua-driver-rs/tests/integration/v2/assets/test_page.htmllibs/cua-driver-rs/tests/integration/v2/assets/test_page.htmllibs/cua-driver/Tests/integration/assets/test_page.htmllibs/cua-driver/Tests/integration/assets/test_page.htmllibs/cua-driver/Tests/integration/fixtures/form_all_inputs.htmllibs/cua-driver/Tests/integration/fixtures/form_all_inputs.htmllibs/cua-driver/Tests/integration/fixtures/interactive.htmllibs/cua-driver/Tests/integration/fixtures/interactive.html
| var dragEvents = []; | ||
| var src = document.getElementById('drag-source'); | ||
| var tgt = document.getElementById('drag-target'); | ||
| src.addEventListener('dragstart', function(e) { | ||
| dragEvents.push('dragstart'); | ||
| e.dataTransfer.setData('text/plain', 'DRAG ME'); | ||
| document.getElementById('drag-status').textContent = 'drag: ' + dragEvents.join(' → '); | ||
| }); | ||
| tgt.addEventListener('dragover', function(e) { | ||
| e.preventDefault(); | ||
| if (dragEvents[dragEvents.length-1] !== 'dragover') dragEvents.push('dragover'); | ||
| tgt.classList.add('over'); | ||
| document.getElementById('drag-status').textContent = 'drag: ' + dragEvents.join(' → '); | ||
| }); | ||
| tgt.addEventListener('dragleave', function() { tgt.classList.remove('over'); }); | ||
| tgt.addEventListener('drop', function(e) { | ||
| e.preventDefault(); | ||
| dragEvents.push('drop'); | ||
| tgt.classList.remove('over'); | ||
| var payload = e.dataTransfer.getData('text/plain'); | ||
| document.getElementById('drag-status').textContent = 'drag: ' + dragEvents.join(' → ') + ' payload=' + payload; | ||
| window._lastDrag = { events: dragEvents.slice(), payload: payload }; |
There was a problem hiding this comment.
Reset drag sequence state at the start of each drag run.
On Line 98 and Line 102, dragEvents persists across runs, so a second drag can include stale events and break deterministic assertions. Reset it on dragstart.
Proposed fix
var dragEvents = [];
var src = document.getElementById('drag-source');
var tgt = document.getElementById('drag-target');
src.addEventListener('dragstart', function(e) {
+ dragEvents = [];
dragEvents.push('dragstart');
e.dataTransfer.setData('text/plain', 'DRAG ME');
document.getElementById('drag-status').textContent = 'drag: ' + dragEvents.join(' → ');
});🤖 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-fixtures/gesture_panels.html` around lines 98 - 119, The
dragEvents array is not reset between drag runs causing stale events to
accumulate; update the dragstart handler (attached to src via
addEventListener('dragstart', ...)) to clear or reassign dragEvents (e.g.,
dragEvents = []) at the start of that function before pushing 'dragstart', so
each new drag run starts with a fresh sequence and window._lastDrag/events
reflect only the current run.
| if let Some(msg) = crate::input::post_message_blocked_by_uipi(target.0 as u64) { | ||
| // Same UIPI defense as PostMessage path — SendInput from non-UIAccess | ||
| // would fail just as silently as PostMessage when target is at higher | ||
| // integrity. Surface the diagnostic early. | ||
| bail!(msg); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and key symbols
ls -la libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs
rg -n "post_message_blocked_by_uipi|send_click_synthesized|SetForegroundWindow|SendInput|uipi|UIAccess|uiAccess|bail!|is_chromium_target_window" libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs
# Show the exact snippet area around the referenced lines
sed -n '220,280p' libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs
# Find where post_message_blocked_by_uipi is defined
rg -n "fn post_message_blocked_by_uipi|post_message_blocked_by_uipi\(" libs/cua-driver-rs/crates -S
# Show the implementation of post_message_blocked_by_uipi (and any helpers it calls)
ppath="$(rg -n "fn post_message_blocked_by_uipi" -S libs/cua-driver-rs/crates | head -n1 | cut -d: -f1)"
echo "Implementation file: $ppath"
sed -n '1,220p' "$ppath"
# Also inspect any send-click helpers or foreground-related logic near send_click_synthesized
rg -n "fn send_click_synthesized|send_click_synthesized\(" libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs
sed -n '1,220p' libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rsRepository: trycua/cua
Length of output: 21357
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '230,420p' libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rsRepository: trycua/cua
Length of output: 5265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '230,420p' libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rsRepository: trycua/cua
Length of output: 5265
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "cua-driver-uia|uiAccess|UIAccess|TokenUIAccess|token uiaccess|mandatory integrity|integrity.*uia" .Repository: trycua/cua
Length of output: 10969
🌐 Web query:
UIAccess bypasses UIPI SendInput allowed across integrity levels
💡 Result:
UIAccess can bypass UIPI restrictions for cross-privilege UI interaction (including using SendInput), but only within the specific trust/launch model Microsoft defines for UIAccess assistive-technology apps. Microsoft’s position (grounded in docs) 1) UIAccess is specifically meant to bypass UIPI across privilege levels for assistive technology. - An assistive-technology app that needs access to higher integrity level (IL) processes must set the UIAccess flag in its manifest and be launched by a user with administrator privileges [1]. - If an app presents a UIAccess attribute when requesting privileges, it is stating a requirement to bypass UIPI restrictions for sending messages across privilege levels [2]. - Microsoft also describes that a process started with UIAccess rights can “drive any application window by using the SendInput function” [2]. 2) UIAccess is not “unrestricted bypass” for all IL boundaries. - Microsoft explicitly says UIAccess is not enough “for a process to move up through the IL boundary,” and it also notes that none of the described scenarios provides access to UI running under system IL [1]. Related behavior when UIAccess is not used - A Microsoft-authored forum post summarizes that SendInput can be blocked by UIPI so an app can “only send commands to applications of the same or lower elevation” [3]. So, how to interpret the query statement - Accurate in the supported sense: With UIAccess configured and launched under the required conditions, Windows allows cross-privilege UI interaction that UIPI would otherwise restrict, including driving windows via SendInput [1][2]. - Inaccurate if read as: UIAccess lets an arbitrary process bypass UIPI/IL boundaries in all cases (especially system-IL targets), which Microsoft explicitly denies [1].
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/winauto/uiauto-securityoverview
- 2: https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/jj852244(v=ws.11)
- 3: https://learn.microsoft.com/en-us/archive/msdn-technet-forums/b68a77e7-cd00-48d0-90a6-d6a4a46a95aa
🌐 Web query:
Microsoft UIAccess lifts User Interface Privilege Isolation UIPI rules
💡 Result:
UIAccess is a mechanism in Windows that allows an application to bypass User Interface Privilege Isolation (UIPI) restrictions to interact with applications running at higher privilege levels [1][2]. UIPI is a Windows feature designed to prevent lower-privilege processes from sending messages, installing hooks, or otherwise interacting with higher-privilege processes [1][2][3]. While UIPI effectively isolates processes of different integrity levels, it can inadvertently block legitimate accessibility software—such as screen readers—that require access to these higher-privilege interfaces [1][2][4]. The UIAccess attribute is used within an application's manifest file (specifically in the requestedExecutionLevel tag) to indicate that the application requires these elevated UI capabilities [5][6]. To successfully use UIAccess and bypass UIPI, an application must meet several strict security requirements: 1. Manifest Requirement: The application's manifest must include the uiAccess="true" attribute [5][6]. 2. Code Signing: The application must be digitally signed with an Authenticode certificate [5][6]. 3. Trusted Location: By default, the application must be installed in a secure, write-protected location on the file system, such as %ProgramFiles% or %WinDir% [5][7]. This prevents unauthorized or malicious code from being injected into the elevated process [8][4]. When these conditions are met, a process started with UIAccess gains specific privileges, including the ability to set the foreground window, drive other application windows via the SendInput function, and set low-level journal hooks [4]. It is important to note that UIAccess does not provide full system-level access; for example, it cannot interact with UI running under the SYSTEM integrity level, which is often reserved for critical OS components and protected UAC prompts [5]. Security policies, such as "User Account Control: Only elevate UIAccess applications that are installed in secure locations," can be used to enforce these constraints [2][8]. Disabling such security policies may allow UIAccess to function from insecure locations, but doing so significantly reduces system security by removing the protection against unauthorized application elevation [7].
Citations:
- 1: https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/jj852245(v=ws.11)
- 2: https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-10/security/threat-protection/security-policy-settings/user-account-control-only-elevate-uiaccess-applications-that-are-installed-in-secure-locations
- 3: https://stackoverflow.com/questions/40122964/cross-process-postmessage-uipi-restrictions-and-uiaccess-true
- 4: https://calcomsoftware.com/elevate-uiaccess-apps-installed-in-secure-locations/
- 5: https://learn.microsoft.com/en-us/windows/win32/winauto/uiauto-securityoverview
- 6: https://github.com/MicrosoftDocs/win32/blob/docs/desktop-src/WinAuto/uiauto-securityoverview.md
- 7: https://learn.microsoft.com/en-us/answers/questions/568475/is-the-secure-location-to-bypass-user-interface-pr
- 8: https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-server-2012-r2-and-2012/jj852244(v=ws.11)
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "UIAccess|uiAccess|ui_access|ui-access|TOKEN_UIACCESS|TokenUIAccess|RequestedExecutionLevel|GetTokenInformation\\(|TOKEN_QUERY|Token.*UIAccess" libs/cua-driver-rs/crates/platform-windows/src/input libs/cua-driver-rs/crates/platform-windows/src/tools libs/cua-driver-rs/crates -SRepository: trycua/cua
Length of output: 7936
Stop using post_message_blocked_by_uipi for the SendInput click path.
post_message_blocked_by_uipi only compares TokenIntegrityLevel of the current process vs the target and has no UIAccess awareness. When send_click_synthesized runs in the cua-driver-uia UIAccess worker, UIPI should be bypassed for SendInput, but this precheck can still bail for higher-integrity targets before the existing SetForegroundWindow/partial SendInput UIAccess diagnostic triggers.
🤖 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/input/mouse.rs` around lines
248 - 253, The precheck using post_message_blocked_by_uipi should not run for
the SendInput click path (e.g., inside send_click_synthesized) because it lacks
UIAccess awareness and can incorrectly bail for UIAccess processes; remove or
guard the current if-let block that calls post_message_blocked_by_uipi so it
only executes for the PostMessage path (not for
SendInput/send_click_synthesized), and rely on the existing
SetForegroundWindow/SendInput UIAccess diagnostics for the SendInput branch.
| let _ = SetForegroundWindow(target); | ||
| sleep(Duration::from_millis(8)); | ||
|
|
||
| // Move the cursor first so the OS hover state matches before the click. | ||
| // `SetCursorPos` is the visible cursor move; the MOUSEEVENTF_MOVE input | ||
| // ensures Chromium's input filter sees a coordinated move event. | ||
| let _ = SetCursorPos(sx, sy); | ||
|
|
||
| let count = count.max(1); | ||
| for i in 0..count { | ||
| let events = [move_input, down_input, up_input]; | ||
| let sent = SendInput(&events, std::mem::size_of::<INPUT>() as i32); | ||
| if sent as usize != events.len() { | ||
| // Partial insertion — restore foreground+cursor and bail with | ||
| // the standard "needs UIAccess worker" diagnostic. | ||
| let _ = SetCursorPos(prev_cursor.x, prev_cursor.y); | ||
| let _ = SetForegroundWindow(prev_fg); | ||
| bail!( | ||
| "SendInput inserted only {sent} of {} mouse events. Likely cause: \ | ||
| the daemon is not at UIAccess integrity, so SetForegroundWindow was \ | ||
| rejected and the events landed on the wrong window. Route Chromium \ | ||
| coord clicks through the cua-driver-uia worker.", | ||
| events.len() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "SetForegroundWindow\(|GetForegroundWindow\(|SendInput\(|UIAccess worker|SetCursorPos\(" libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs
# Show the relevant section with line numbers
sed -n '250,430p' libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs | cat -nRepository: trycua/cua
Length of output: 5682
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all SetForegroundWindow/GetForegroundWindow occurrences in the file
FILE="libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs"
rg -n "SetForegroundWindow\(|GetForegroundWindow\(|needs UIAccess worker|UIAccess worker" "$FILE"
# Show a wider section around the SetForegroundWindow call site
sed -n '260,380p' "$FILE" | cat -n
# Also search for SetForegroundWindow usage in the whole crate to see if there is an existing guard elsewhere
rg -n "SetForegroundWindow\(" libs/cua-driver-rs/crates/platform-windows/src/inputRepository: trycua/cua
Length of output: 5643
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver-rs/crates/platform-windows/src/input/keyboard.rs"
sed -n '260,380p' "$FILE" | cat -nRepository: trycua/cua
Length of output: 6455
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs"
# Print function header and relevant checks preceding the SetForegroundWindow block
sed -n '240,360p' "$FILE" | cat -nRepository: trycua/cua
Length of output: 5556
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs"
# Print more context around the function start and the checks preceding the SetForegroundWindow block
sed -n '200,360p' "$FILE" | cat -nRepository: trycua/cua
Length of output: 7909
Fail fast when focusing target fails in send_click_synthesized (mouse.rs)
SetForegroundWindow(target) is currently ignored, and the only failure guard is SendInput returning a partial insertion. SendInput fully succeeding only means the events were queued—it doesn’t guarantee they land on target, so focus denial can cause the click to hit the current foreground window.
🛠️ Suggested guard
- let _ = SetForegroundWindow(target);
+ if !SetForegroundWindow(target).as_bool() {
+ bail!(
+ "failed to foreground hwnd 0x{:x}; refusing to inject SendInput into the current foreground window",
+ target.0 as usize
+ );
+ }
sleep(Duration::from_millis(8));
+ if GetForegroundWindow() != target {
+ bail!(
+ "hwnd 0x{:x} never became foreground; route Chromium coord clicks through the cua-driver-uia worker",
+ target.0 as usize
+ );
+ }File: libs/cua-driver-rs/crates/platform-windows/src/input/mouse.rs (around lines 323-345)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let _ = SetForegroundWindow(target); | |
| sleep(Duration::from_millis(8)); | |
| // Move the cursor first so the OS hover state matches before the click. | |
| // `SetCursorPos` is the visible cursor move; the MOUSEEVENTF_MOVE input | |
| // ensures Chromium's input filter sees a coordinated move event. | |
| let _ = SetCursorPos(sx, sy); | |
| let count = count.max(1); | |
| for i in 0..count { | |
| let events = [move_input, down_input, up_input]; | |
| let sent = SendInput(&events, std::mem::size_of::<INPUT>() as i32); | |
| if sent as usize != events.len() { | |
| // Partial insertion — restore foreground+cursor and bail with | |
| // the standard "needs UIAccess worker" diagnostic. | |
| let _ = SetCursorPos(prev_cursor.x, prev_cursor.y); | |
| let _ = SetForegroundWindow(prev_fg); | |
| bail!( | |
| "SendInput inserted only {sent} of {} mouse events. Likely cause: \ | |
| the daemon is not at UIAccess integrity, so SetForegroundWindow was \ | |
| rejected and the events landed on the wrong window. Route Chromium \ | |
| coord clicks through the cua-driver-uia worker.", | |
| events.len() | |
| if !SetForegroundWindow(target).as_bool() { | |
| bail!( | |
| "failed to foreground hwnd 0x{:x}; refusing to inject SendInput into the current foreground window", | |
| target.0 as usize | |
| ); | |
| } | |
| sleep(Duration::from_millis(8)); | |
| if GetForegroundWindow() != target { | |
| bail!( | |
| "hwnd 0x{:x} never became foreground; route Chromium coord clicks through the cua-driver-uia worker", | |
| target.0 as usize | |
| ); | |
| } | |
| // Move the cursor first so the OS hover state matches before the click. | |
| // `SetCursorPos` is the visible cursor move; the MOUSEEVENTF_MOVE input | |
| // ensures Chromium's input filter sees a coordinated move event. | |
| let _ = SetCursorPos(sx, sy); | |
| let count = count.max(1); | |
| for i in 0..count { | |
| let events = [move_input, down_input, up_input]; | |
| let sent = SendInput(&events, std::mem::size_of::<INPUT>() as i32); | |
| if sent as usize != events.len() { | |
| // Partial insertion — restore foreground+cursor and bail with | |
| // the standard "needs UIAccess worker" diagnostic. | |
| let _ = SetCursorPos(prev_cursor.x, prev_cursor.y); | |
| let _ = SetForegroundWindow(prev_fg); | |
| bail!( | |
| "SendInput inserted only {sent} of {} mouse events. Likely cause: \ | |
| the daemon is not at UIAccess integrity, so SetForegroundWindow was \ | |
| rejected and the events landed on the wrong window. Route Chromium \ | |
| coord clicks through the cua-driver-uia worker.", | |
| events.len() |
🤖 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/input/mouse.rs` around lines
323 - 345, In send_click_synthesized, don't ignore SetForegroundWindow(target):
check its boolean return and fail fast if it returns false so we don't proceed
sending events to the wrong window. If SetForegroundWindow(target) fails,
restore the previous cursor (prev_cursor) and previous foreground window
(prev_fg) like the existing SendInput error path, and bail! with a clear
diagnostic mentioning focus denial (same style as the existing bail! for partial
SendInput). Keep the subsequent SetCursorPos(target) and SendInput-only path for
the successful-focus case.
| if launch_path_opt.is_some() || path_opt.is_some() || name_opt.is_some() { | ||
| if let Some(t) = target.as_deref() { | ||
| if is_chromium_browser_target(t) { | ||
| inject_chromium_anti_throttling_flags(&mut extra_args); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Plain bundle_id Chromium aliases skip flag injection.
On Windows, a non-AUMID bundle_id is treated as a name alias, but this guard excludes that input shape. launch_app({bundle_id:"chrome"}) will therefore miss the anti-throttling flags and regress to occluded/blank Chromium launches.
🛠️ Suggested fix
+ let plain_bundle_id_alias = bundle_id_opt
+ .as_deref()
+ .map(|s| !crate::launch_uwp::is_aumid(s))
+ .unwrap_or(false);
- if launch_path_opt.is_some() || path_opt.is_some() || name_opt.is_some() {
+ if launch_path_opt.is_some() || path_opt.is_some() || name_opt.is_some() || plain_bundle_id_alias {
if let Some(t) = target.as_deref() {
if is_chromium_browser_target(t) {
inject_chromium_anti_throttling_flags(&mut extra_args);
}
}
}🤖 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
837 - 843, The current guard only checks launch_path_opt, path_opt, and name_opt
before calling is_chromium_browser_target/
inject_chromium_anti_throttling_flags, so plain non-AUMID bundle_id aliases are
skipped; update the condition to include the bundle_id variant (e.g. add
bundle_id_opt.is_some() or otherwise detect when target came from bundle_id) so
that when target.as_deref() refers to a Chromium alias you still call
is_chromium_browser_target(t) and then
inject_chromium_anti_throttling_flags(&mut extra_args); keep references to
launch_path_opt, path_opt, name_opt, bundle_id_opt (or the bundle_id source),
target, is_chromium_browser_target, and inject_chromium_anti_throttling_flags.
| UIA_AcceleratorKeyPropertyId, UIA_ButtonControlTypeId, UIA_CheckBoxControlTypeId, | ||
| UIA_CONTROLTYPE_ID, UIA_HyperlinkControlTypeId, UIA_InvokePatternId, | ||
| UIA_ListItemControlTypeId, UIA_MenuItemControlTypeId, UIA_PROPERTY_ID, | ||
| UIA_RadioButtonControlTypeId, UIA_SplitButtonControlTypeId, UIA_TabItemControlTypeId, | ||
| UIA_TogglePatternId, UIA_TreeItemControlTypeId, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate the target file
ls -la libs/cua-driver-rs/crates/platform-windows/src/uia/ || true
sed -n '1,200p' libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs
# 2) Find the coord-independent helper and where UIA_SplitButtonControlTypeId is referenced
rg -n "is_coord_independent_action|coord[-_ ]independent|UIA_SplitButtonControlTypeId|ExpandCollapse|InvokePatternId" \
libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs
# 3) Search for click routing / preference logic across the same crate for ExpandCollapse vs Invoke
rg -n "ExpandCollapse|Invoke" libs/cua-driver-rs/crates/platform-windows/src/uia \
|| true
# 4) If the above doesn't find the preference logic, widen search to the whole windows platform crate
rg -n "ExpandCollapse|Invoke" libs/cua-driver-rs/crates/platform-windows/src \
|| trueRepository: trycua/cua
Length of output: 30721
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver-rs/crates/platform-windows/src/uia/windows_enum.rs"
# Show the coord-independent allowlist function body and its usage
sed -n '150,240p' "$FILE"
sed -n '240,380p' "$FILE"
# Also show the imports region around the line numbers the review comment references
sed -n '1,60p' "$FILE"
# Search for any other mention of split button / coord-independent / expand-collapse preference
rg -n "SplitButton|split button|coord-independ" "$FILE"
rg -n "ExpandCollapse.*Invoke|Prefer ExpandCollapse|winner_has_invoke|noop|submenu" "$FILE"
rg -n "SplitButtonControlTypeId" libs/cua-driver-rs/crates/platform-windows/src/uia -S
# Search repo-wide for split button semantics notes (keep output small)
rg -n "SplitButton|SplitButtonControlTypeId|split button" libs/cua-driver-rs/crates -S | head -n 50Repository: trycua/cua
Length of output: 14944
🌐 Web query:
UIA SplitButtonControlTypeId InvokePattern ExpandCollapsePattern behavior primary button vs dropdown
💡 Result:
In Microsoft UI Automation (UIA), a control with the SplitButton control type (UIA_SplitButtonControlTypeId) is required to support both the InvokePattern and the ExpandCollapsePattern to manage its dual-function behavior [1][2]. The behavior is distinct for each pattern: InvokePattern (Primary Action): The InvokePattern represents the default action associated with the main button part of the control [1][2]. Calling the Invoke method triggers this primary action (e.g., executing a command like 'Save' or 'Paste') without opening the associated menu [1][3]. ExpandCollapsePattern (Dropdown Action): The ExpandCollapsePattern is used to interact with the secondary part of the control, which manages the visibility of the dropdown menu [1][2]. Calling Expand or Collapse on this pattern toggles the state of the menu (e.g., showing or hiding available options) [2][4]. In practice, a SplitButton functions as a composite control [2]. When automated via UIA, the primary button area typically maps to the Invoke functionality, while a secondary "more options" button or indicator area maps to the ExpandCollapse functionality [1][2]. Developers of custom controls must ensure that these patterns are implemented separately so that UIA clients can invoke the primary action without affecting the dropdown state, and vice versa [1][2]. If a control does not clearly separate these actions—for instance, if the primary button itself acts as the trigger for the menu—it should typically be classified as a standard button with different pattern support rather than a SplitButton [5].
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/winauto/uiauto-supportsplitbuttoncontroltype
- 2: https://learn.microsoft.com/en-us/dotnet/framework/ui-automation/ui-automation-support-for-the-splitbutton-control-type
- 3: https://learn.microsoft.com/en-us/dotnet/framework/ui-automation/implementing-the-ui-automation-invoke-control-pattern
- 4: https://admhelp.microfocus.com/uft/en/26.1/UFT_Help/Subsystems/OMRHelp/Content/UIAutomation/UIAToolkitLib~UIASplitButton.htm
- 5: https://learn.microsoft.com/en-us/windows/win32/winauto/uiauto-supportbuttoncontroltype
Remove UIA_SplitButtonControlTypeId from coord-independent allowlist
A UIA SplitButton’s InvokePattern is the primary action, while ExpandCollapsePattern is the dropdown/“more options” action. The current logic treats SplitButton as coord-independent and, when both patterns are present, prefers ExpandCollapse first—so a click aimed at the primary region can open the dropdown menu.
🛠️ Suggested fix
- UIA_RadioButtonControlTypeId, UIA_SplitButtonControlTypeId, UIA_TabItemControlTypeId,
+ UIA_RadioButtonControlTypeId, UIA_TabItemControlTypeId,
...
UIA_ButtonControlTypeId
| UIA_MenuItemControlTypeId
| UIA_HyperlinkControlTypeId
| UIA_TabItemControlTypeId
| UIA_ListItemControlTypeId
| UIA_CheckBoxControlTypeId
| UIA_RadioButtonControlTypeId
- | UIA_SplitButtonControlTypeId
| UIA_TreeItemControlTypeIdIf UIA_SplitButtonControlTypeId becomes unused after this change, drop it from the corresponding import list too.
🤖 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/uia/windows_enum.rs` around
lines 26 - 30, Remove UIA_SplitButtonControlTypeId from the coord-independent
allowlist so SplitButton is no longer treated as coord-independent (ensuring
InvokePattern remains the primary action instead of preferring ExpandCollapse);
update the declaration that lists control type constants to drop
UIA_SplitButtonControlTypeId from the allowlist and then remove the
UIA_SplitButtonControlTypeId import if it becomes unused elsewhere.
…ugh SendInput `PostMessage(WM_LBUTTONDOWN/UP)` to Chromium-based browsers' frame HWND (or Chrome_RenderWidgetHostHWND descendant) doesn't reach the DOM input pipeline — Chromium's input thread only accepts events with `SendInput`-queue origin (same architectural quirk that broke modifier-state hotkey delivery in #1614/#1618). After #1621 stopped the silent UIA Invoke reroute on canvases, x,y clicks on Chromium pages took the PostMessage path and silently no-op'd the DOM event handlers. ## Fix Add a third branch to `LeftClickTool::run`'s x,y dispatch, between UIA Invoke (for coord-independent control types per #1621) and PostMessage (for everything else): 1. UIA Invoke if `is_coord_independent_action(element)` — preserved. 2. **NEW**: if the target HWND is a Chromium frame, route through `send_click_synthesized` which uses `SendInput` against the system input queue. Surfaces an actionable error if it fails (typically non-UIAccess daemon — the call should land on the cua-driver-uia worker which already runs at UIAccess integrity). 3. PostMessage `post_click` otherwise — unchanged. ## New helpers (`crates/platform-windows/src/input/mouse.rs`) - **`is_chromium_target_window(hwnd)`** — `GetClassNameW` check for `Chrome_WidgetWin_*` (covers all Chromium-based browsers: Edge, Chrome, Brave, Vivaldi, Opera, Arc, Thorium, Iridium, etc.) and `CefBrowser*` (Electron / CEF apps). Cheap call (~one `GetClassNameW` to a 64-byte buffer); suitable inline in the click dispatch path. Emits a `tracing::debug!(target="click")` line with the observed class name so future debugging can see what the function actually decided. - **`send_click_synthesized(target, sx, sy, count, button)`** — mirror of `send_key_synthesized` for mouse input. Save previous foreground + cursor → `SetForegroundWindow(target)` (8ms settle) → `SetCursorPos` + `SendInput(MouseInputs)` → 40ms settle → restore previous foreground + cursor. Uses `MOUSEEVENTF_ABSOLUTE | MOUSEEVENTF_VIRTUALDESK` normalized coords so multi-monitor setups work correctly. Trade-offs: briefly steals foreground + visibly moves the cursor. There's no Chromium-native alternative that gets DOM events to fire without these tradeoffs short of `--remote-debugging-port` + CDP (separate work). The send_key_synthesized path makes the same trade-off for modifier-state hotkeys; this is the consistent answer. ## Why this branch ordering UIA Invoke runs first (no focus steal). Per #1621 it only fires for control types with coord-independent primary actions (Button, MenuItem, Hyperlink, etc.) — so when the click lands on a Chromium *button* or *link*, UIA Invoke wins and the user gets zero focus steal. Only when UIA Invoke isn't viable (canvases, paint surfaces, image maps, custom widgets) does the Chromium SendInput branch engage. This means the common Chromium interactions (clicking buttons, links, form controls) keep the no-focus-steal property. The focus steal + cursor jump only happens when the user explicitly asks for pixel precision on a custom-drawn surface — which is the exact case where they care about coords reaching the underlying element. ## Verification - `cargo check -p platform-windows` clean on the VM (2.44s incremental) - `cargo build --release -p cua-driver -p cua-driver-uia` clean (27.33s) - Pre-existing 8 unit tests under `chromium_flag_injection_tests` still pass - **E2E (#1620 + #1621 + #1623 chain)**: `click(pid, x, y)` on the "Click Me" button in `test_page.html` loaded in Edge — page DOM now exposed via UIA (per #1620 auto-injection), UIA Invoke takes the path (per #1621 whitelist — Button is coord-independent), counter increments. The SendInput branch only engages when UIA Invoke can't, which is the canvas case. - **Direct canvas verification deferred**: the canvas in `test_page.html` sits below the viewport in a 901px tall Edge window; verifying the SendInput path against a canvas requires the `scroll` tool which wasn't in the test harness allowlist. Structure verified through unit tests + the chain test above + the canvas's UIA control type (`Image`) being in the #1621 fall-through set. ## UIAccess constraint `send_click_synthesized` requires the daemon to have UIAccess integrity so `SetForegroundWindow` is permitted. When invoked from a non-UIAccess daemon, the function surfaces the actionable error `"SendInput inserted only 0 of 3 mouse events. Likely cause: the daemon is not at UIAccess integrity, so SetForegroundWindow was rejected and the events landed on the wrong window. Route Chromium coord clicks through the cua-driver-uia worker."` — same template as `send_key_synthesized`. The MCP proxy already auto-prefers the `cua-driver-uia` pipe over the regular pipe when both are running (cli.rs:407-408), so Chromium coord clicks on systems with the uia worker installed (the default) take the SendInput path. Systems without the uia worker get the diagnostic. Closes #1623. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
b8d2fba to
7ae60d7
Compare
Summary
PostMessage(WM_LBUTTONDOWN/UP)to Chromium-based browsers' frame HWND doesn't reach the DOM input pipeline — Chromium's input thread only accepts events withSendInput-queue origin (same architectural quirk that broke modifier-state hotkey delivery in #1614/#1618). After #1621 stopped the silent UIA Invoke reroute on canvases, x,y clicks on Chromium pages took the PostMessage path and silently no-op'd the DOM event handlers.This PR adds a third branch to
LeftClickTool::run's x,y dispatch, between UIA Invoke (for coord-independent control types per #1621) and PostMessage (for everything else):is_coord_independent_action(element)per cua-driver-rs Windows: click(x,y) silently rerouted to UIA Invoke when an actionable element is at that point #1621 — preserved (no focus steal, fast)post_clickotherwise — unchangedWhy this branch ordering matters
The common Chromium interactions (clicking buttons, links, form controls) keep the no-focus-steal property because UIA Invoke fires for Button / MenuItem / Hyperlink etc. — those are in the #1621 whitelist. The focus steal + cursor jump in step 2 only happens when the user explicitly asks for pixel precision on a custom-drawn surface (canvas, paint area, image map) — exactly the case where they care about coords reaching the underlying element.
New helpers (
input/mouse.rs)is_chromium_target_window(hwnd)—GetClassNameWcheck forChrome_WidgetWin_*(Edge, Chrome, Brave, Vivaldi, Opera, Arc, Thorium, Iridium, Yandex, …) andCefBrowser*(Electron / CEF apps). Includes atracing::debug!(target="click")line for future diagnosis.send_click_synthesized(target, sx, sy, count, button)— mirror ofsend_key_synthesizedfor mouse input. Saves previous foreground + cursor →SetForegroundWindow(target)(8ms settle) →SetCursorPos+SendInput(MouseInputs)→ 40ms settle → restore previous foreground + cursor. UsesMOUSEEVENTF_ABSOLUTE | MOUSEEVENTF_VIRTUALDESKnormalized coords so multi-monitor setups work correctly.UIAccess constraint
send_click_synthesizedrequires the daemon to have UIAccess integrity soSetForegroundWindowis permitted. When invoked from a non-UIAccess daemon, the function surfaces the actionable error:The MCP proxy already auto-prefers the
cua-driver-uiapipe over the regular pipe when both are running (cli.rs:407-408), so Chromium coord clicks on systems with the uia worker installed (the default) take the SendInput path.Verification
cargo check -p platform-windowsclean on the VMcargo build --release -p cua-driver -p cua-driver-uiaclean (27.33s)chromium_flag_injection_testsstill pass(#1620 + #1621 + #1623)chain:click(pid, x, y)on the "Click Me" button intest_page.htmlloaded in Edge — page DOM now exposed via UIA (per cua-driver-rs Windows: launch_app should auto-inject anti-throttling flags for hidden Chromium browsers #1620 auto-injection), UIA Invoke takes the path (per cua-driver-rs Windows: click(x,y) silently rerouted to UIA Invoke when an actionable element is at that point #1621 whitelist — Button is coord-independent), counter increments. The SendInput branch only engages when UIA Invoke can't, which is the canvas case.test_page.htmlsits below the viewport in a 901px tall Edge window; verifying the SendInput path against a canvas requires thescrolltool which wasn't in the test harness allowlist. Structure verified through unit tests + the chain test above + the canvas's UIA control type (Image) being in the cua-driver-rs Windows: click(x,y) silently rerouted to UIA Invoke when an actionable element is at that point #1621 fall-through set.Related
Closes #1623.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor