Fix Peek Ctrl+W shortcut not working after clicking preview#48293
Open
MuyuanMS wants to merge 11 commits into
Open
Fix Peek Ctrl+W shortcut not working after clicking preview#48293MuyuanMS wants to merge 11 commits into
MuyuanMS wants to merge 11 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses Peek shortcuts (Ctrl+W, Escape, arrow navigation) failing after clicking into preview content by ensuring key handling still works when focus is inside WebView2 or native preview handler HWNDs.
Changes:
- Installs/uninstalls a low-level keyboard hook while the Peek window is active to handle close and navigation shortcuts regardless of focused child control.
- Adds required Win32 P/Invoke declarations for keyboard hook setup and modifier state checks.
- Disables WebView2 browser accelerator keys to prevent Chromium from consuming shortcuts like Ctrl+W.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/modules/peek/Peek.UI/PeekXAML/MainWindow.xaml.cs |
Adds global low-level keyboard hook wiring and hook callback to close/navigate when Peek is foreground. |
src/modules/peek/Peek.UI/Native/NativeMethods.cs |
Adds SetWindowsHookEx/UnhookWindowsHookEx/CallNextHookEx/GetAsyncKeyState P/Invokes and hook struct/delegate. |
src/modules/peek/Peek.FilePreviewer/Controls/BrowserControl.xaml.cs |
Disables WebView2 browser accelerator keys to reduce shortcut conflicts (e.g., Ctrl+W). |
Comments suppressed due to low confidence (1)
src/modules/peek/Peek.UI/PeekXAML/MainWindow.xaml.cs:414
MainWindownow owns an unmanaged global hook handle;Dispose()currently only disposesthemeListener. IfDispose()is ever called without going throughUninitialize()first (e.g., shutdown/crash paths), the hook may remain installed until process exit. It’s safer forDispose()to always uninstall the hook as part of cleanup.
public void Dispose()
{
themeListener?.Dispose();
}
22ae8b5 to
6e073d5
Compare
MuyuanMS
commented
Jun 5, 2026
MuyuanMS
left a comment
Contributor
Author
There was a problem hiding this comment.
Addressed all feedback in 4bd5cdf: log Win32 error codes on hook install/uninstall failure, validate UnhookWindowsHookEx return value, pass IntPtr.Zero to CallNextHookEx per low-level hook convention, and reworded BrowserControl comment to clarify defense-in-depth role.
When the user clicks inside a file preview in Peek (PDF, text/code files, markdown, HTML), the WebView2 control or shell preview handler captures keyboard focus. Since these controls handle input in their own process/ message loop, the Ctrl+W KeyboardAccelerator defined on the parent XAML Grid never fires, making it impossible to close the Peek window with the keyboard shortcut. This fix adds a low-level keyboard hook (WH_KEYBOARD_LL) that intercepts Ctrl+W, Escape, and arrow navigation keys at the OS level when the Peek window is in the foreground. This works regardless of which child control has focus (WebView2, native shell preview handlers, etc.). Additionally, browser-level accelerator keys are disabled on the WebView2 control (AreBrowserAcceleratorKeysEnabled = false) as defense-in-depth, preventing Chromium from consuming shortcuts like Ctrl+W internally. Fixes #48274 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… for CallNextHookEx, fix comment - Log Marshal.GetLastWin32Error() when SetWindowsHookEx fails - Check UnhookWindowsHookEx return value before clearing handle - Pass IntPtr.Zero to CallNextHookEx (standard for low-level hooks) - Reword BrowserControl comment to clarify defense-in-depth role Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The XAML accelerators are defined without Shift, so the hook should not intercept Shift+key combos (e.g., Shift+Arrow for text selection in Monaco, Shift+Escape, Ctrl+Shift+W). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove/re-add Content_KeyUp to prevent accumulating duplicate handlers when Initialize() is called multiple times - Cache the HWND once during Initialize() and reuse it in the hook callback to avoid per-keypress allocations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Early-return for keys we never handle, avoiding GetAsyncKeyState and GetForegroundWindow calls on every system keypress - Remove 'using' on ProcessModule (unnecessary disposal) and add null- forgiving operator on _keyboardHookProc to suppress nullable warning Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Define VK_CONTROL, VK_ALT, VK_SHIFT, and KEY_PRESSED_MASK as named constants for readability and maintainability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Check foreground window before marshaling KBDLLHOOKSTRUCT to avoid per-keystroke overhead when Peek is not active - Only swallow the key if TryEnqueue succeeds, so if the dispatcher is shutting down the key passes through instead of being lost Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add VK_LWIN/VK_RWIN check so Win+Arrow (window snapping), Win+Ctrl+W, and other Win-key combos pass through to the OS instead of being intercepted by the hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use _keyboardHookHandle instead of IntPtr.Zero in all CallNextHookEx calls to match other hook implementations in the repo (e.g., ColorPickerUI MouseHook). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Wrap hook callback in try/catch to prevent exceptions from escaping into the Win32 hook chain (which could crash the process) - Guard against null lParam before marshaling - Pass keys through when delete confirmation dialog is active - Only intercept arrow keys when multiple items are available for navigation (single item or CLI mode lets arrows reach the preview) - Re-check _isDeleteInProgress inside close lambdas to handle race between enqueue and execution - Clear _keyboardHookProc on install failure for consistent state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserves stack trace and inner exceptions in hook error logging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
407a9dd to
6786fca
Compare
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #48274
When previewing a file with Peek, clicking inside the preview content (PDF, text/code, markdown, HTML) makes Ctrl+W unable to close the window. This happens because the preview controls (WebView2 for most file types, native shell handlers for others) capture keyboard focus in their own message loop, bypassing the XAML keyboard accelerator system entirely.
Problem
Peek defines keyboard shortcuts (Ctrl+W to close, Escape to close, arrow keys to navigate) as
KeyboardAcceleratorelements on the main XAML Grid. These only fire when keyboard input flows through the XAML input system. However:Once either of these controls gets focus via a mouse click, keyboard shortcuts stop working.
Solution
Added a low-level keyboard hook (
WH_KEYBOARD_LL) that intercepts key events at the OS level, regardless of which control has focus:The hook is installed only while the Peek window is visible and only acts when Peek is the foreground window, so it has no impact on other applications.
As additional defense-in-depth,
AreBrowserAcceleratorKeysEnabledis set tofalseon the WebView2 control, preventing Chromium from consuming browser-specific shortcuts like Ctrl+W.Changes
Peek.UI/PeekXAML/MainWindow.xaml.csPeek.UI/Native/NativeMethods.csSetWindowsHookEx,UnhookWindowsHookEx,CallNextHookEx,GetAsyncKeyStatePeek.FilePreviewer/Controls/BrowserControl.xaml.csValidation