Let browser pages handle key events first#304490
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Integrated Browser’s keyboard handling so embedded pages get first chance to consume key events (addressing shortcut interference reported in #303908), and adds smoke coverage for basic browser open/security expectations.
Changes:
- Move key handling from Electron
before-input-eventinterception to a preload-driven “forward only when unhandled” approach. - Remove the now-unneeded
dispatchKeyEventplumbing across the browser view service/model. - Add smoke tests for Integrated Browser open/title behavior and a Node-integration sanity check.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/smoke/src/main.ts | Wires the new Integrated Browser smoke suite into the Electron (non-remote) smoke run. |
| test/smoke/src/areas/browser/browser.test.ts | Adds smoke tests for loading a page title and verifying lack of Node globals in main frame + subframe. |
| src/vs/workbench/contrib/browserView/electron-browser/browserEditor.ts | Stops “forwarding back” key events; always dispatches received key events to keybinding service. |
| src/vs/workbench/contrib/browserView/common/browserView.ts | Removes model-level dispatchKeyEvent method as key event routing changes. |
| src/vs/platform/browserView/electron-main/browserViewMainService.ts | Removes main service dispatchKeyEvent method and related import. |
| src/vs/platform/browserView/electron-main/browserView.ts | Switches key event forwarding to an IPC signal from the preload; enables preload in subframes. |
| src/vs/platform/browserView/electron-browser/preload-browserView.ts | Adds DOM keydown listener to forward “unhandled” keys to the main process for workbench shortcut dispatch. |
| src/vs/platform/browserView/common/browserView.ts | Removes service-level dispatchKeyEvent API. |
deepak1556
left a comment
deepak1556
left a comment
There was a problem hiding this comment.
nodeIntegrationInSubFrames will cause sandbox to be disabled for cross origin sub frame processes, only same origin sub frames will respect embedding frames sandbox properties. This is not desired and a security concern.
Can you rely on session api https://github.com/electron/electron/blob/main/docs/api/session.md#sesregisterpreloadscriptscript to inject browser view sessions preloadscript into frames
Fixes #303908