Skip to content

fix(dialog): the Windows picker takes the keyboard, not just the z-order - #142

Merged
danyaberezun merged 2 commits into
mainfrom
fix/windows-picker-focus
Jul 28, 2026
Merged

fix(dialog): the Windows picker takes the keyboard, not just the z-order#142
danyaberezun merged 2 commits into
mainfrom
fix/windows-picker-focus

Conversation

@danyaberezun

@danyaberezun danyaberezun commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #141 (merged). The dialog now opens in front of the browser, but without focus — you have to click it before typing or arrowing around.

Why top-most wasn't enough

TopMost wins the z-order; it doesn't make the window active. The browser stays the foreground window, so the folder dialog opens on top of it with no keyboard.

SetForegroundWindow is blocked by the same foreground lock that put the dialog behind the browser to begin with — a process that isn't already foreground can't just take it. The documented way past it is to attach our input thread to the current foreground thread first:

$fg = GetWindowThreadProcessId(GetForegroundWindow(), $null)
$me = GetCurrentThreadId()
AttachThreadInput($me, $fg, $true)
SetForegroundWindow($owner.Handle)
AttachThreadInput($me, $fg, $false)

Best-effort, in a try/catch: a host that can't compile the P/Invoke (ConstrainedLanguage, no compiler) still gets the top-most dialog — just unfocused, as today — rather than no dialog at all.

-Command-EncodedCommand

That block needs C# string literals, and quoting embedded double quotes through a Windows command line into PowerShell's own parser is exactly the minefield apps/cli/src/powershell.ts documents and avoids with a temp .ps1. The script now travels as -EncodedCommand (base64 UTF-16LE): one quote-proof argv element, no temp file to write or clean up. Newlines come free with it, so the script reads as a script instead of a ;-joined one-liner.

Verified on Windows 11

Measured on a real Windows 11 host, driving selectDirectory() from a background Bun process with an unrelated app holding the foreground — the same conditions that make the bug visible:

dialog visible focused keyboard
main (#141, top-most owner only) yes no — 3/3 runs goes to the browser
this PR yes yes — 3/3 runs goes to the dialog

Focus was read with GetForegroundWindow() against the dialog's hwnd, filtered to a live picker process (so windows left behind by earlier runs can't be mistaken for a fresh one). Keyboard delivery was checked end to end, not just inferred from the focus flag: {ENTER} sent to no particular window returned the highlighted folder ({"path":"C:\Users\danya\Desktop"}), and {ESC} returned {"path":null} at exit 0 — cancel still reads as cancel, not as a failure.

Also green: typecheck, lint, unit tests (10/10 in the dialog suite), and — from #141's run — 115/115 e2e on Linux.

Scope

Three files. The two source files, plus dialog/SPEC.md — taking you up on the offer in the last revision: the spec had credited the invisible top-most owner for getting the picker in front, which the table above shows isn't what does it. It now records the foreground lock, both measurements, and that the grab is best-effort by design, so the next person doesn't re-derive it from the P/Invoke.

The unit test decodes the base64 payload before asserting on the script, and pins AttachThreadInput / SetForegroundWindow($owner.Handle), the paired attach/detach (a stuck attachment would hand us another app's keyboard state for as long as the picker lives), and the catch that keeps the grab optional.

danyaberezun and others added 2 commits July 28, 2026 00:56
Top-most won the z-order but not the *active* window: the browser stayed
foreground, so the dialog opened on top with no keyboard focus — the user had
to click it before typing or arrowing around.

`SetForegroundWindow` is blocked by the same foreground lock that put the
dialog behind the browser in the first place, so the script attaches our input
thread to the current foreground thread first (the documented way past the
lock), sets the owner form foreground, then detaches. Best-effort in a
try/catch: a host that can't compile the P/Invoke still gets the top-most
dialog, just unfocused, rather than no dialog at all.

That block needs C# string literals, and quoting embedded double quotes
through a Windows command line into PowerShell's own parser is the minefield
`apps/cli/src/powershell.ts` avoids with a temp `.ps1`. The script now travels
as `-EncodedCommand` (base64 UTF-16LE) instead of `-Command` — one quote-proof
argv element, no temp file. Newlines come free with it, so the script reads as
a script.
The module spec still credited the invisible top-most owner for getting the picker
in front — but top-most only wins the z-order. Measured on Windows 11 with an
unrelated app in front, that dialog came up unfocused in 3/3 runs: visible, with
the keyboard still in the browser. Record what actually works (AttachThreadInput
to the foreground thread, then SetForegroundWindow on the owner form), the
measurements for both, and that the grab is best-effort by design.

Two test assertions to match: the attach/detach calls are paired (a stuck
attachment would hand us another app's keyboard state for the picker's lifetime),
and the grab stays inside its catch so a host that can't compile the P/Invoke
still gets a dialog.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jetbrains-air[bot]
jetbrains-air Bot previously approved these changes Jul 27, 2026

@jetbrains-air jetbrains-air Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — ready to merge.

@jetbrains-air jetbrains-air Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — ready to merge.

@danyaberezun
danyaberezun disabled auto-merge July 28, 2026 00:37
@danyaberezun
danyaberezun merged commit 8a2c87f into main Jul 28, 2026
4 checks passed
@danyaberezun
danyaberezun deleted the fix/windows-picker-focus branch July 28, 2026 00:40
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