fix(cua-driver-rs)(windows): screenshot clips bottom row of VCL/SAL dialogs (use GetWindowRect not GetClientRect) - #1696
Conversation
…indowRect, not GetClientRect `screenshot_window_bytes_with_occlusion_unsafe`'s non-XAML path allocated its capture bitmap from `GetClientRect`, then called `PrintWindow(hwnd, mem_dc, PW_RENDERFULLCONTENT)`. PrintWindow draws the entire window at 1:1 starting at (0,0), but the destination bitmap is only client-sized, so any non-client content past the client rect gets clipped silently. This bites VCL/SAL dialogs hard. LibreOffice's "Document Recovery" (the post-discard summary modal) is 607x271 with a Save/Cancel button row at the bottom. GetClientRect reports 605x239 — the button strip lives in VCL's bottom command area which sits OUTSIDE the standard Win32 client area. The returned screenshot showed title bar + body + the directory picker, but the buttons were just gone. Users (and LLM agents) couldn't see what they were supposed to click. Fix: switch to GetWindowRect so the buffer matches the actual rendered window dimensions. PrintWindow with PW_RENDERFULLCONTENT then draws the full window content into a correctly-sized bitmap. Verified end-to-end: same dialog now returns 621x278, both Save and Cancel are visible in the bottom-right corner of the image. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Caution Review failedPull request was closed or merged during review No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PrintWindow capture code for the occlusion-aware window capture path is updated to capture the entire window instead of the client area only. ChangesWhole-window capture sizing
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
…ps + Rust integration tests (#1698) * feat(cua-driver-rs)(test-harness): WPF + WinUI3 deterministic test apps with Rust integration tests Background: The Windows automation surface is hard to test deterministically because the apps under test (Word, Chrome, LibreOffice, ...) drift between versions, ship localized strings, and have layout variations across themes. Bugs like the GetClientRect-vs-GetWindowRect crop (PR #1696) and the DWM-extended-frame inset (PR #1697) were caught only after they shipped to a real user-facing capture. This commit adds a pair of minimal .NET 8 host apps that present a fixed scenario set, plus Rust integration tests that drive them via cua-driver. The harness gives us a stable target for regression guards on the Windows hosting patterns the agent has to handle. What's in: libs/cua-driver/test-harness/ CuaTestHarness.sln scenarios/scenarios.json # single source of truth (AutomationIds, # window titles, expected markers) CuaTestHarness.Wpf/ # .NET 8 WPF, self-contained, x64 counter / text_body / message_box / bottom_strip (Save+Cancel) child_hwnd (HwndHost wrapping a native Win32 BUTTON) owned_popup / layered_popup (WS_EX_LAYERED via AllowsTransparency) accelerator (Ctrl+Shift+H KeyBinding) CuaTestHarness.WinUI3/ # .NET 8 WinUI3 unpackaged, x64 counter / text_body / command_bar_flyout / xaml_popup / exit build.ps1 # `dotnet publish` -> rust/test-apps/ libs/cua-driver/rust/crates/cua-driver/tests/ harness_wpf_test.rs # #[ignore]'d smoke + UIA Invoke # roundtrip (counter increments) harness_winui3_test.rs # #[ignore]'d smoke libs/cua-driver/rust/sandbox/{run-tests-in-sandbox,sandbox-runner}.ps1 Build the harness (if dotnet available on host), stage to %TEMP% inside the sandbox, and run with --ignored. Local verification (Win11 host): cd test-harness && ./build.ps1 cd ../rust && cargo test --test harness_wpf_test --test harness_winui3_test \ -- --ignored --nocapture --test-threads=1 -> 3 passed; 0 failed (2 WPF + 1 WinUI3) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat(cua-driver-rs)(test-harness): full behavioral coverage — type_text, scroll, modal, popup, hotkey, right/double-click Expands the harness from structural (UIA tree present?) to behavioral (does the action actually take effect?) coverage. New scenarios: WPF: text_input - TextBox + mirror label (drives type_text + set_value) click_target - Button with MouseRightButtonDown / MouseDoubleClick scroll_target - ScrollViewer + WM_VSCROLL WndProc hook so the cua-driver scroll tool's PostMessage(WM_VSCROLL, SB_LINEDOWN) actually advances the offset. Without the hook, WPF's purely-routed-event input system ignores WM_VSCROLL entirely. accelerator - F5 binding added alongside Ctrl+Shift+H. Modifier bindings need the cua-driver-uia SendInput path (modifier state isn't carried by PostMessage); F5 works on the PostMessage path that tests use. WinUI3: text_input - TextBox + mirror (UIA ValuePattern.SetValue route) New Rust integration tests (all passing locally): WPF (11/11): smoke, counter_invoke, type_text, set_value, right_click, double_click, scroll, press_key_accelerator, modal_messagebox, owned_popup, layered_popup_capture WinUI3 (3/3): smoke, type_text, xaml_popup_open Foreground-lock workarounds documented in the test file: Tests that drive WPF's input event chain rely on dispatch:"foreground" for reliable handler firing. Windows' system-wide foreground lock activates after ~30s without real user input, blocking SetForegroundWindow for non-UIAccess processes. Mitigated by running WPF and WinUI3 suites as separate `cargo test --test ...` invocations (the sandbox runner already does this, one .exe per suite). Local Win11 verification: cargo test --test harness_wpf_test -- --ignored --test-threads=1 -> 11 passed; 0 failed cargo test --test harness_winui3_test -- --ignored --test-threads=1 -> 3 passed; 0 failed Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(cua-driver-rs)(test-harness): address CodeRabbit review Six actionable fixes from the PR #1698 review: 1. README.md - add `text` language tag on the layout fenced block (markdownlint MD040). 2. WinUI3 Program.cs - capture Bootstrap.TryInitialize's bool + HRESULT and only call Bootstrap.Shutdown when Init actually succeeded. Continue starting the app on Init failure since the harness publishes self-contained (<WindowsAppSDKSelfContained>true</...>) and runs from its local SDK copy when the system-wide bootstrap is absent. 3. WinUI3 MainWindow.xaml.cs - OnExitClick: use Application.Current.Exit() in place of System.Environment.Exit(0) so Program.Main's finally (Bootstrap.Shutdown) actually runs. 4. run-tests-in-sandbox.ps1 - wrap `& $harnessBuild` in try/catch. test-harness/build.ps1 sets $ErrorActionPreference=Stop and throws on `dotnet publish` failure; without the catch that propagates up and aborts the runner before the [WARN] skip line can log. 5. sandbox-runner.ps1 - Test-Path on the staged exe before exporting HARNESS_*_EXE. An incomplete publish output would otherwise turn the expected "skip" into a hard launch failure inside the Rust test. 6. harness_{wpf,winui3}_test.rs - replace the fixed 3s/5s Harness::launch sleep with a short cold-start settle plus polling in find_harness_window (bounded deadline, 150-200ms intervals). ~25s faster on warm runs, more robust under sandbox-cold-start load. Acknowledged but deferred: CodeRabbit also suggested making AutomationIds / marker strings in Rust tests and popup XAMLs manifest-driven from scenarios.json. That's a real "single source of truth" improvement but a more invasive refactor (popup AIDs are set declaratively in XAML; making them runtime-set requires constructor parameters from MainWindow + an extra JSON read in tests). Current arrangement still catches drift via the smoke test, which fails fast when AID/title literals diverge. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…_BOUNDS to remove invisible-shadow black trim (#1697) * fix(cua-driver-rs)(windows)(capture): crop to DWMWA_EXTENDED_FRAME_BOUNDS to drop the invisible-shadow black trim PR #1696 sized the PrintWindow capture buffer to GetWindowRect so non-client content (VCL/SAL bottom button strip) wouldn't clip. That fixed the clipping, but Win10+'s DWM-extended-frame returns a rectangle a few pixels LARGER than the actually-painted window — the extra margin is an invisible drop shadow that DWM composes separately. PrintWindow doesn't paint into that margin, so the captured bitmap had a thin dark border around the body (everything except the title bar, which spans the full width and so doesn't expose the artifact). Fix: after PrintWindow, query DwmGetWindowAttribute with DWMWA_EXTENDED_FRAME_BOUNDS for the rectangle WITHOUT the shadow margin, then crop the BGRA buffer to that inner rect before encoding. Best-effort — if the DWM call fails on older Windows, we keep the full-window bitmap as-is (small dark trim, no clipping). Verified end-to-end against LibreOffice's "Document Recovery" interrupted dialog: - Before #1696: 605x239, Save + Cancel CLIPPED - After #1696: 621x278, Save + Cancel visible BUT 7-14 px black margin around body - With this fix: 607x271, Save + Cancel visible, ZERO trim The crop produces a pixel-for-pixel match against the window's list_windows bounds (607x271), so downstream pixel-coord clicks hit exactly where the screenshot shows. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(cua-driver-rs)(windows)(capture): 1px inset on DWM crop to drop Win11 dialog edge hairline The DWMWA_EXTENDED_FRAME_BOUNDS crop removed the bulk of the invisible-shadow margin (commit 27a28b8), but Win11 dialogs paint a 1-2 px dark stroke at their rounded-corner edge that lands at the very edge of the DWM rect. Visible as a thin black hairline along the bottom of the captured bitmap. Add a 1-pixel inset (DWM_CROP_INSET_PX) on each side of the DWM crop so we shave that frame-chrome hairline. Anything that close to the edge is window-frame chrome, not content — losing 2 px per axis is invisible to the eye and removes the artifact entirely. Verified against the LibreOffice "Document Recovery" interrupted modal: previous capture was 607x271 with a thin black bottom edge; inset capture is 605x269 with zero residual trim. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
The non-XAML PrintWindow path in `capture.rs` allocated its destination bitmap with `GetClientRect` dimensions, but PrintWindow draws the entire window at 1:1 starting at (0, 0). For VCL/SAL dialogs (LibreOffice) the bottom button strip lives in the non-client area — outside what GetClientRect reports — so the captured bitmap silently clips the row containing Save / Cancel / OK.
Concrete repro
LibreOffice "Document Recovery" summary modal after discarding recovery data. `GetWindowRect` says 607×271. `GetClientRect` says 605×239. Screenshot returned the 605×239 client-sized bitmap with the bottom 32 px of buttons missing. The user couldn't see the buttons in the agent's vision input.
Fix
Switch the buffer-allocation rect from `GetClientRect` to `GetWindowRect`. PrintWindow draws into a correctly-sized buffer; full window content (title bar + body + non-client trim + bottom command bar) is captured.
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit