fix(cua-driver/windows): route DoubleClick/RightClick on Chromium targets via SendInput (#1984) - #1995
Conversation
…gets via SendInput (#1984) ClickTool auto-detects Chromium/Electron target HWNDs and routes clicks through SendInput because Chromium's input thread silently drops PostMessage mouse events (#1623). DoubleClickTool and RightClickTool lacked this short-circuit, so element_index/pixel double- and right-clicks on Electron apps (Obsidian, VS Code, Slack, …) fell through to post_click_screen (PostMessage) and no-op'd — matching the #1984 candidate B audit hypothesis. - Add chromium_click_short_circuit() helper (mirrors the ClickTool branch: detect Chromium HWND, deliver via send_click_synthesized with async foreground restore, else return None to keep the PostMessage path). - Wire it into both dispatch paths (element_index + x/y) of DoubleClickTool and RightClickTool, before the default PostMessage call. DragTool has the same gap on its default path; left as a fast-follow since its press-move-release SendInput path (send_drag_synthesized) needs separate handling. Candidate A (stale element-cache center) is a deeper audit, tracked separately. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KMXCW4M5uK1HRGjjH4wueZ
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
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:
📝 WalkthroughWalkthroughAdds a new private async helper ChangesChromium/Electron SendInput short-circuit for DoubleClick and RightClick
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs (1)
3514-3560: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffHelper logic is sound; consider consolidating the repeated SendInput pattern.
The detection-then-
SendInput-then-restore flow correctly mirrors the existingdispatch:foregroundbranches, and returningSome(ToolResult::error(...))on a failedsend_click_synthesizedis the right call — it surfaces the diagnostic instead of silently falling through to thePostMessageno-op.The
prev_fgcapture →spawn_blocking(send_click_synthesized)→tokio::spawn(restore_foreground_polling_best_effort)→ result-match sequence is now duplicated across this helper plus the fourdispatch:foregroundbranches (Lines 3686-3700, 3744-3758, 3904-3918, 3961-3975). Extracting a singlesend_click_via_sendinput(...)core that both this helper and those branches delegate to would remove the repetition; the only delta is the success-message text.🤖 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/rust/crates/platform-windows/src/tools/impl_.rs` around lines 3514 - 3560, The SendInput pattern used in chromium_click_short_circuit (capturing prev_fg_addr, calling spawn_blocking on send_click_synthesized, spawning restore_foreground_polling_best_effort, and matching on the result) is duplicated across four dispatch:foreground branches. Extract this repeated logic into a new helper function (e.g., send_click_via_sendinput) that accepts the hwnd, screen coordinates, click count, button, pid, and a success message template as parameters, then refactor both chromium_click_short_circuit and the four duplicate dispatch:foreground branch implementations to call this single helper instead, eliminating the repetition while preserving each branch's custom success message text.
🤖 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/rust/crates/platform-windows/src/tools/impl_.rs`:
- Around line 3514-3560: The SendInput pattern used in
chromium_click_short_circuit (capturing prev_fg_addr, calling spawn_blocking on
send_click_synthesized, spawning restore_foreground_polling_best_effort, and
matching on the result) is duplicated across four dispatch:foreground branches.
Extract this repeated logic into a new helper function (e.g.,
send_click_via_sendinput) that accepts the hwnd, screen coordinates, click
count, button, pid, and a success message template as parameters, then refactor
both chromium_click_short_circuit and the four duplicate dispatch:foreground
branch implementations to call this single helper instead, eliminating the
repetition while preserving each branch's custom success message text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02d74cf0-c473-4b5c-ac74-fc09665ed7b0
📒 Files selected for processing (1)
libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs
Linux visual regression artifactsMatrix jobs now run independently. Download visual artifacts from this workflow run.
|
…nd dispatch, not just auto (#1984) Runtime testing on a real desktop (Edge) revealed the first cut was ineffective in the DEFAULT path: double_click/right_click default to dispatch:background, where the pre-existing 'would_be_silently_dropped' guard returned background_unavailable_error for Chromium BEFORE the auto-only SendInput short-circuit could run — so a Chromium double-click just errored. Mirror ClickTool exactly: in the background branch, route Chromium/GTK targets through inject_click_screen (coordinate injection into the system input queue, NO foreground swap) and only fall back to background_unavailable_error if the actuator can't express the click (e.g. right/middle). The auto-path SendInput short-circuit is retained for dispatch:auto. Covers both addressing modes (element_index + x/y) of DoubleClickTool and RightClickTool. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KMXCW4M5uK1HRGjjH4wueZ
…etic pen device (#1984) Runtime testing showed single click injected fine in background but double_click errored: inject_click_screen looped pen_tap, which creates AND destroys a synthetic pen device per tap. The second CreateSyntheticPointerDevice in quick succession fails, so the second tap (and thus every double/triple click) returned Err -> background_unavailable_error. Rename pen_tap -> pen_taps(count): create ONE device and emit count down/up cycles (70ms apart) on it, then destroy once. A real double-click is two taps from one digitizer, so this is also more correct. inject_click_screen now calls it once. Fixes background double_click / right-double scenarios for Chromium/Electron/GTK targets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KMXCW4M5uK1HRGjjH4wueZ
Runtime-verified on a real desktop ✅Validated this branch ( Both now deliver via the coordinate actuator (system input queue, no foreground swap) instead of failing. Before this branch, a default Notes:
|
1 participant
Problem
double_clickandright_clickagainst anelement_index(or pixel) on a Chromium/Electron window (Obsidian, VS Code, Slack, …) report success but do nothing — the DOM never sees the click. Addresses #1984, candidate B.Root cause
Chromium's input thread only honors SendInput-origin events; it silently drops
PostMessage(WM_*BUTTON*)(#1623).ClickToolalready auto-detects a Chromium target HWND and routes through SendInput — butDoubleClickToolandRightClickToolhad no such check (verified:ClickToolhas 3is_chromium_target_windowcalls, these tools had 0). Their default (Auto) dispatch fell straight topost_click_screen(PostMessage) and no-op'd.Fix
chromium_click_short_circuit()helper mirrors theClickToolbranch: detect a Chromium HWND, delivercountclicks ofbuttonat screen(sx, sy)viasend_click_synthesizedwith async foreground restore, and returnSome(result); returnNonefor non-Chromium targets so the normal PostMessage path is unchanged.DoubleClickToolandRightClickTool, before the default PostMessage call. Explicitdispatch:foreground/dispatch:backgroundpaths are untouched.Scope notes
send_drag_synthesized) needs separate handling — left as a fast-follow rather than bundled here.element_cache.get_element_centerafter a window move) is a deeper cache-lifetime audit and is not addressed in this PR.Verification
windowscrate):cargo build -p platform-windowsrecompiled green at commitc308990(helper + all 4 wired sites present).🤖 Generated with Claude Code
Summary by CodeRabbit