Skip to content

fix(hud): stop being born click-through, wait for the renderer to ask - #269

Merged
EtienneLescot merged 3 commits into
mainfrom
fix/hud-born-click-through
Aug 5, 2026
Merged

fix(hud): stop being born click-through, wait for the renderer to ask#269
EtienneLescot merged 3 commits into
mainfrom
fix/hud-born-click-through

Conversation

@EtienneLescot

@EtienneLescot EtienneLescot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Refs #266 — the HUD painted on screen, and every click, drag and button dead from the very first launch.

What was wrong

createHudOverlayWindow() asked for click-through twice: once at construction, and once from the renderer's mount effect (hud-overlay-ignore-mouse-events). Only the second one is safe.

On Windows, setIgnoreMouseEvents(true, { forward: true }) is not a Chromium hit-test — Electron installs a global WH_MOUSE_LL hook and reposts WM_MOUSEMOVE by hand (native_window_views_win.cc:678):

void NativeWindowViews::SetForwardMouseMessages(bool forward) {
  if (forward && !forwarding_mouse_messages_) {
    forwarding_mouse_messages_ = true;
    ...
    if (!mouse_hook_) {
      mouse_hook_ = SetWindowsHookEx(WH_MOUSE_LL, MouseHookProc, nullptr, 0);
    }
  }

That hook is the only route out of the state, and its install is latched:

  • Chromium delivers no pointermove to a window it has made input-transparent, so the renderer can never ask to leave click-through on its own.
  • The install sits behind !forwarding_mouse_messages_, so a later forward: true is a no-op. Electron only retries after a setIgnoreMouseEvents(false) (the ERROR_INVALID_HOOK_HANDLE branch, l.706) — which is precisely the call a dead hook prevents.

Deadlock. One hook refused, or silently revoked by Windows for overrunning LowLevelHooksTimeout (300 ms), and the bar is painted, inert, forever — with no user-reachable way back.

And construction is the worst moment to ask: that hook callback runs on the main thread, which at that point is still booting the app.

The change

One statement deleted. The renderer already asks, on mount, a frame or two later — over IPC, on a main thread that is provably pumping messages.

The failure mode inverts: if the ask never arrives, the bar stays clickable instead of becoming a ghost.

Cost — measured, ~85 ms

An invisible 820×560 rectangle can swallow one desktop click between ready-to-show and LaunchWindow's mount effect. Timed in the real app: 83 ms and 90 ms over two runs (a third read negative — an IPC still in flight from the destroyed HUD landing on the new window — and is discarded). LaunchWindow is a static import, not lazy, so that window does not stretch.

It can be closed to ~0 by asking from src/main.tsx at module scope, before React mounts. Deliberately not done: that moves the SetWindowsHookEx install earlier into the boot, back toward the busy main thread this PR is trying to get it away from. One swallowed click is recoverable; the deadlock is not.

One swallowed click, not a dragged HUD. The obvious worry is 2bc9d70 — the drag region that sat on the window root, which on Linux (no click-through) meant pressing empty space dragged the HUD from a spot the user was aiming past. That needed two ingredients, and the second is gone: the region now lives on the grab handle, and it is gated nativeDrag={isLinuxHud}, so on Windows/macOS there is no -webkit-app-region: drag in the HUD at all. .hudAnchor is pointer-events: none on top of that. Nothing in those 85 ms can move the window.

Not Windows-only, and what each platform sees

The deleted line ran on all three platforms. What changes:

  • Windows / macOS — the HUD is a normal input target for those ~85 ms, then identical to before.
  • Linux — end state unchanged. LaunchWindow gates on isLinuxHud and never requests click-through there, so this line was the only thing making the HUD input-transparent on Linux, transiently, until mount. Removing it removes a transient hole rather than opening one.

Nothing else depended on the state: setHudOverlayIgnoreMouseEvents has exactly two call sites, both in LaunchWindow.tsx. git log -S dates the deleted line to a bulk "ui revamp" commit — it was never a targeted fix for anything. The countdown overlay keeps its own setIgnoreMouseEvents(true) at construction on purpose: no forward, so no hook, no latch, and it is meant to stay click-through for its whole life.

Not verifiable here: macOS. No machine.

What this does not cover

A hook revoked mid-session still deadlocks. Closing that needs an independent wake-up in the main process (poll screen.getCursorScreenPoint() while click-through, ~12 lines). Left out on purpose — it addresses a symptom nobody has reported yet, and this PR targets the RC window.

Getting this into 1.9.0

Targets main, per AGENTS.md § PR & commit conventions. The RC window is open (1.9.0-rc.2), so shipping it in 1.9.0 means a cherry-pick onto release/v1.9.0 once this lands — which is what § Release branches asks for.

Verification

  • biome check clean, tsc --noEmit clean.
  • Playwright e2e on Windows — results in a comment below.
  • The deadlock itself is not reproducible on demand (it needs the hook to be refused or revoked), so the e2e evidence is non-regression of the normal path: the HUD still goes click-through after mount, and the bar still takes clicks.

Summary by CodeRabbit

  • Bug Fixes

    • Improved HUD mouse interaction handling during startup.
    • Click-through behavior is now enabled after the HUD is ready, helping prevent missed or unexpected mouse interactions.
  • Tests

    • Added Windows-native coverage to verify the HUD’s click-through behavior and startup sequence.

@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The HUD no longer enables click-through during construction. The renderer now triggers the existing IPC flow, and a Windows-native E2E test verifies the deferred setIgnoreMouseEvents call.

Changes

HUD click-through handling

Layer / File(s) Summary
Defer native mouse handling
electron/windows.ts
HUD creation no longer calls setIgnoreMouseEvents before renderer initialization.
Verify renderer-driven click-through
tests/e2e/windows-native-checklist.spec.ts
The Windows E2E test records native calls, verifies no construction-time call, checks the renderer request, and restores the patched method during cleanup.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant HUDCreation
  participant HUDRenderer
  participant IPC
  participant BrowserWindow
  HUDCreation->>BrowserWindow: Create HUD without mouse-event ignoring
  HUDRenderer->>IPC: Request click-through
  IPC->>BrowserWindow: setIgnoreMouseEvents(true, { forward: true })
Loading

Possibly related PRs

Suggested reviewers: siddharthvaddem

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main HUD behavior change: delaying click-through until the renderer requests it.
Description check ✅ Passed The description thoroughly covers the problem, solution, platform impact, risks, issue reference, and verification results.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hud-born-click-through

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@EtienneLescot

Copy link
Copy Markdown
Collaborator Author

e2e run — Windows 11, real Electron app (dist-electron/main.js, HEADLESS=true)

Suite Result Signal for this PR
windows-native-checklist.spec.ts 2 passed, 2 failed yes — real Electron main process
v4-shell.spec.ts + editor-smoke.spec.ts 5 passed none (browser shim, no Electron main)
gif-export.spec.ts not run none

The 2 failures are #130, pre-existing. launch-open-video-button / launch-open-project-button appear nowhere in src/ — those two tests cannot pass on any commit.

gif-export could not run here, not "failed": this worktree has no native compositor addon (no *.node) and no vendored ffmpeg (no thirdparty/), so the export path has nothing to call. This PR touches no export code.

The test added in c6c6fca

The existing suite covers the HUD only from the renderer's side, which is exactly the side that cannot see this bug. The new one asserts both halves of the contract:

  1. Nothing calls setIgnoreMouseEvents while the HUD is being constructed. The window is recreated through the app's own path (second-instanceshowMainWindowcreateHudOverlayWindow) with the native call taped, and the tape is read back with no await in between — so no renderer IPC can contaminate what is meant to be construction only.
  2. The renderer still asks for it after mount[true, { forward: true }] must land within 20 s.

Ablated, because a test that passes both ways proves nothing: restoring the deleted line turns duringConstruction into [[true, { forward: true }]] and assertion 1 fails.

✓ the HUD asks for click-through instead of being born with it (3.2s)   # with the fix
✗ expect(duringConstruction).toEqual([])                                # line restored

What the e2e cannot prove

Click-through is an OS window attribute; Playwright synthesizes input inside Chromium and never crosses the WS_EX_TRANSPARENT boundary. No Playwright test can observe a real click passing through — or failing to. And the deadlock itself needs SetWindowsHookEx to be refused or revoked, which is not reproducible on demand.

So the evidence here is: the construction-time call is gone (asserted, ablated), the renderer path that replaces it works in the real app (asserted), and nothing else regressed.

Note

No CI workflow runs Playwright (.github/workflows/ has no test:e2e / playwright reference), so this test is a local guard only. If you want a merge-blocking one, it belongs in the vitest suite — which needs an electron module mock the repo does not have yet (vi.mock("electron"): zero occurrences).

The HUD asked to be input-transparent twice: once here, at construction,
and once from the renderer's mount effect. The first one is the whole bug
in #266 — the app running, the bar painted, and every click, drag and
button dead, forever, from the very first launch.

On Windows `setIgnoreMouseEvents(true, { forward: true })` is a global
WH_MOUSE_LL hook, and that hook is the only route back: Chromium delivers
no pointermove to a window it has made input-transparent, so the renderer
cannot ask to leave the state it is stuck in. Electron latches the install
behind `forwarding_mouse_messages_` and only retries after a
setIgnoreMouseEvents(false) — the call the dead hook prevents. So one hook
that is refused, or that Windows revokes for overrunning the 300 ms
LowLevelHooksTimeout, bricks the UI with no way out.

Construction time is the worst possible moment to ask for it: that hook
callback runs on the main thread, which is still booting the app. The
renderer asks a frame or two later, over IPC, on a thread that is provably
pumping messages — and if that ask never comes, the bar stays clickable
instead of turning into a ghost.

Costs an invisible rectangle that can swallow one desktop click in the two
frames between show and mount. Refs #266.
Two halves, because both are load-bearing after #266: nothing may call
setIgnoreMouseEvents while the HUD window is being constructed, and the
renderer must still ask for it once it has mounted.

The window is recreated through the app's own path (second-instance →
showMainWindow → createHudOverlayWindow) with the native call taped, and
the tape is read back synchronously — no await between arming it and
snapshotting, so no renderer IPC can slip into what is meant to be
construction only. The source selector is opened first purely to keep the
window list non-empty while the HUD is destroyed: emptying it fires
window-all-closed, which quits the app under the test.

Ablated: restoring the deleted line turns duringConstruction into
[[true, {forward: true}]] and the first assertion fails, so the test does
cover the regression it claims to.
"A frame or two" was an assumption. Timing the real app from ready-to-show
to the renderer's first hud-overlay-ignore-mouse-events puts it at 83 and
90 ms over two clean runs — roughly forty times what the comment claimed,
and the number a reviewer should be weighing against #266.

(A third run read -403 ms: the tape catches setIgnoreMouseEvents on the
prototype, so an IPC still in flight from the destroyed HUD's renderer
lands on the new window and dates the ask before the window exists. The
probe was throwaway; the two clean runs are the ones quoted.)
@EtienneLescot
EtienneLescot force-pushed the fix/hud-born-click-through branch from 0c6b68c to 8f84822 Compare August 5, 2026 08:53
@EtienneLescot
EtienneLescot changed the base branch from release/v1.9.0 to main August 5, 2026 08:53
@EtienneLescot
EtienneLescot merged commit 7049644 into main Aug 5, 2026
32 of 35 checks passed
@EtienneLescot
EtienneLescot deleted the fix/hud-born-click-through branch August 5, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant