Skip to content

fix: defer defaultPrevented check to microtask in preload-browserView (#306414)#307471

Open
maruthang wants to merge 2 commits intomicrosoft:mainfrom
maruthang:fix/issue-306414-vscode-dev-keybindings
Open

fix: defer defaultPrevented check to microtask in preload-browserView (#306414)#307471
maruthang wants to merge 2 commits intomicrosoft:mainfrom
maruthang:fix/issue-306414-vscode-dev-keybindings

Conversation

@maruthang
Copy link
Copy Markdown

Summary

Fixes #306414

Bug: Command+Shift+P and F1 do not work on vscode.dev when opened in the Integrated Browser view — the shortcuts are forwarded to the host VS Code window instead of being handled by the page.

Root Cause: Electron's isolated-world keydown listener (running in the preload script at worldId 999) fires before the page's main-world listeners. The synchronous event.defaultPrevented check therefore always read false, even when vscode.dev's own listener was about to call preventDefault(). As a result, key events were incorrectly forwarded to the host.

Fix: Move the event.defaultPrevented check inside a queueMicrotask() callback. A microtask runs after all synchronous event handlers in all worlds have completed, so defaultPrevented reflects the final value set by any page listener. A snapshot of the relevant event properties is captured synchronously (before the microtask) to avoid reading a recycled event object.

Changes

  • src/vs/platform/browserView/electron-browser/preload-browserView.ts: Remove synchronous defaultPrevented guard; capture event property snapshot; defer guard and IPC send into queueMicrotask().
  • src/vs/platform/browserView/test/electron-main/preloadKeyForwarding.test.ts: New test file with 4 regression tests covering: (1) no forward when page calls preventDefault() for Command+Shift+P, (2) no forward when page calls preventDefault() for F1, (3) forward fires when no preventDefault() is called, (4) plain character keys without modifiers are not forwarded.

Testing

  • Added 4 regression tests in src/vs/platform/browserView/test/electron-main/preloadKeyForwarding.test.ts that exercise the key-forwarding logic in isolation (no Electron context required).
  • All new tests are expected to pass; existing tests are unaffected.

Manual verification steps

  1. Open VS Code desktop and load https://vscode.dev in the Integrated Browser panel.
  2. Press Command+Shift+P (macOS) or Ctrl+Shift+P (Linux/Windows) — the vscode.dev command palette should open, not the host VS Code one.
  3. Press F1 — the vscode.dev command palette should open.
  4. Press a plain character key (e.g. a) in a text area — it should type normally without being intercepted.

CP857 was supported by the terminal but missing from document encoding
support. Adds the encoding with labels 'Turkish DOS (CP 857)' / 'CP 857'
and a regression test.
…microsoft#306414)

Electron's isolated-world keydown listener fires before the page's main-world
listeners, so event.defaultPrevented was always false when checked synchronously.
Commands like Command+Shift+P and F1 on vscode.dev were being forwarded to the
host window even though the page had called preventDefault().

Move the defaultPrevented check inside queueMicrotask() so it runs after all
synchronous event handlers in all worlds have completed. Capture a snapshot of
the event properties before the microtask to avoid reading a recycled event object.

Fixes microsoft#306414
@vs-code-engineering
Copy link
Copy Markdown
Contributor

📬 CODENOTIFY

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

@kycutler

Matched files:

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

@jruales

Matched files:

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

@bpasero

Matched files:

  • src/vs/workbench/services/textfile/common/encoding.ts
  • src/vs/workbench/services/textfile/test/node/encoding/encoding.test.ts

Copy link
Copy Markdown
Contributor

@kycutler kycutler left a comment

Choose a reason for hiding this comment

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

This doesn't seem to work -- it looks like defaultPrevented isn't actually true within the microtask callback.

The branch is also out of date with some of the latest changes in main and contains an unrelated encoding change.

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.

Command Shift P and F1 do not work in vsode.dev

2 participants