Skip to content

Fix shortcuts double-emitting from browser on Mac#306198

Merged
kycutler merged 1 commit intomainfrom
kycutler/kbmac
Mar 30, 2026
Merged

Fix shortcuts double-emitting from browser on Mac#306198
kycutler merged 1 commit intomainfrom
kycutler/kbmac

Conversation

@kycutler
Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings March 30, 2026 02:05
@kycutler kycutler enabled auto-merge (squash) March 30, 2026 02:05
@vs-code-engineering
Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

The following users are being notified based on files changed in this PR:

@jruales

Matched files:

  • src/vs/platform/browserView/electron-browser/preload-browserView.ts

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adjusts keyboard event forwarding from the Integrated Browser (BrowserView) preload script to avoid shortcut handling being triggered both by the embedded page/browser and by VS Code—particularly on macOS.

Changes:

  • Refines which keydown events are forwarded to the workbench (modifier-based + “non-editing” keys).
  • Adds special-casing to keep common clipboard/editing shortcuts handled natively by the browser.
  • Prevents/stops propagation for forwarded events to avoid native handling (and potential double-execution).
Comments suppressed due to low confidence (3)

src/vs/platform/browserView/electron-browser/preload-browserView.ts:61

  • The Alt-key filtering only excludes altKey when ctrlKey/metaKey are false. On Windows/Linux keyboard layouts, AltGr is commonly reported as Ctrl+Alt; with the current logic those keydowns will be forwarded and preventDefault() will run, likely breaking typing of AltGr characters in page inputs. Consider detecting AltGraph via event.getModifierState?.('AltGraph') (or a similar AltGr heuristic) and treating it like plain character input (do not forward / do not preventDefault).
		// Alt+Key special character handling (Alt + Numpad keys on Windows/Linux, Alt + any key on Mac)
		if (event.altKey && !event.ctrlKey && !event.metaKey) {
			if (isMac || /^Numpad\d+$/.test(event.code)) {
				return;
			}
		}

src/vs/platform/browserView/electron-browser/preload-browserView.ts:77

  • The allowlist for “native shortcuts” is currently limited to a/c/v/x/z (+shift variants) and redo on Win/Linux. This means common text-editing/navigation shortcuts like Cmd/Ctrl+Arrow (home/end/word navigation), Cmd/Ctrl+Backspace/Delete, etc. will now be forwarded and preventDefault()'d, which can break expected editing behavior in web page inputs/contenteditable elements. Consider skipping forwarding/preventDefault when the event target is editable, or expand the native-shortcut allowlist to include these editing/navigation combinations.
		// Allow native shortcuts (copy, paste, cut, undo, redo, select all) to be handled by the browser
		const ctrlCmd = isMac ? event.metaKey : event.ctrlKey;
		if (ctrlCmd && !event.altKey) {
			const key = event.key.toLowerCase();
			if (!event.shiftKey && (key === 'a' || key === 'c' || key === 'v' || key === 'x' || key === 'z')) {
				return;
			}
			if (event.shiftKey && (key === 'v' || key === 'z')) {
				return;
			}
			// Ctrl+Y is redo on Windows/Linux
			if (!event.shiftKey && key === 'y' && !isMac) {
				return;
			}
		}

src/vs/platform/browserView/electron-browser/preload-browserView.ts:55

  • isMac detection uses navigator.platform, which is deprecated and can be influenced by the page environment. Since this preload already has Node/Electron access, prefer process.platform === 'darwin' for a stable platform check.
		const isMac = navigator.platform.indexOf('Mac') >= 0;

@kycutler kycutler merged commit ee6bfc5 into main Mar 30, 2026
22 checks passed
@kycutler kycutler deleted the kycutler/kbmac branch March 30, 2026 03:21
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.

3 participants