Skip to content

fix: use Chromium's native DevTools context menu and dialog paths#51805

Merged
MarshallOfSound merged 1 commit into
mainfrom
sam/devtools-native-context-menu
Jun 3, 2026
Merged

fix: use Chromium's native DevTools context menu and dialog paths#51805
MarshallOfSound merged 1 commit into
mainfrom
sam/devtools-native-context-menu

Conversation

@MarshallOfSound

Copy link
Copy Markdown
Member

Description of Change

Right-clicking in a detached DevTools window showed the correct context menu but focused the inspected page's window instead of the DevTools window.

This happened because DevTools context menus were implemented by overriding InspectorFrontendHost.showContextMenuAtPoint in JS and popping an Electron Menu on getOwnerBrowserWindow() — which for a detached DevTools window is the inspected page's BrowserWindow, since the detached window is a plain views::Widget and not an Electron window.

Rather than patching the override, this PR removes it and wires up the same native path Chromium uses:

  • Blink's DevToolsHost.showContextMenuAtPoint delivers the menu items to the browser as ContextMenuParams::custom_items via WebContentsDelegate::HandleContextMenu.
  • InspectableWebContents shows them as a native menu (new DevToolsContextMenu helper, views::MenuRunner) anchored to whichever widget actually hosts the DevTools view, so focus stays where it should.
  • Selections are routed back through ExecuteCustomContextMenuCommand / NotifyContextMenuClosed, which Blink forwards to DevToolsAPI.contextMenuItemSelected / contextMenuCleared — the same calls the JS override made manually.

The window.confirm override is likewise replaced with a JavaScriptDialogManager on InspectableWebContents, and the dead INSPECTOR_SELECT_FILE handler is removed. DevTools menus also now display accelerators, which the JS path dropped.

Refs #35651 and #49753, both symptoms of the previous approach.

Checklist

  • I have built and tested this change
  • I have filled out the PR description
  • I have reviewed and verified the changes
  • npm test passes
  • tests are changed or added

Release Notes

Notes: Fixed the context menu in a detached DevTools window focusing the inspected page's window instead of the DevTools window.

Previously DevTools context menus were implemented by overriding
InspectorFrontendHost.showContextMenuAtPoint in JS and popping an
Electron Menu on the inspected page's BrowserWindow. Since a detached
DevTools window is not an Electron window, the menu was always anchored
to (and focused) the inspected page's window.

Remove the JS overrides and let Blink's native
DevToolsHost.showContextMenuAtPoint path run instead: menu items arrive
via WebContentsDelegate::HandleContextMenu and are shown as a native
menu anchored to whichever widget hosts the DevTools view, with
selections routed back through ExecuteCustomContextMenuCommand /
NotifyContextMenuClosed. window.confirm in DevTools now works through a
JavaScriptDialogManager on InspectableWebContents.
@MarshallOfSound MarshallOfSound requested a review from a team as a code owner May 30, 2026 05:08
@MarshallOfSound MarshallOfSound added the semver/patch backwards-compatible bug fixes label May 30, 2026
@electron-cation electron-cation Bot added the new-pr 🌱 PR opened recently label May 30, 2026
@MarshallOfSound MarshallOfSound changed the title refactor: use Chromium's native DevTools context menu and dialog paths fix: use Chromium's native DevTools context menu and dialog paths May 30, 2026
@MarshallOfSound MarshallOfSound added target/41-x-y PR should also be added to the "41-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. target/43-x-y PR should also be added to the "43-x-y" branch. labels May 30, 2026
@electron-cation electron-cation Bot removed the new-pr 🌱 PR opened recently label May 31, 2026
@MarshallOfSound MarshallOfSound enabled auto-merge (squash) June 1, 2026 19:44
@MarshallOfSound MarshallOfSound merged commit e63f186 into main Jun 3, 2026
142 of 144 checks passed
@release-clerk

release-clerk Bot commented Jun 3, 2026

Copy link
Copy Markdown

Release Notes Persisted

Fixed the context menu in a detached DevTools window focusing the inspected page's window instead of the DevTools window.

@MarshallOfSound MarshallOfSound deleted the sam/devtools-native-context-menu branch June 3, 2026 16:48
@trop

trop Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

I was unable to backport this PR to "41-x-y" cleanly;
you will need to perform this backport manually.

@trop trop Bot removed the target/41-x-y PR should also be added to the "41-x-y" branch. label Jun 3, 2026
@trop

trop Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "43-x-y", please check out #51863

@trop

trop Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

I have automatically backported this PR to "42-x-y", please check out #51864

@trop trop Bot added in-flight/43-x-y in-flight/42-x-y merged/42-x-y PR was merged to the "42-x-y" branch. and removed target/43-x-y PR should also be added to the "43-x-y" branch. target/42-x-y PR should also be added to the "42-x-y" branch. in-flight/42-x-y in-flight/43-x-y labels Jun 3, 2026
@trop trop Bot added the merged/43-x-y PR was merged to the "43-x-y" branch. label Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merged/42-x-y PR was merged to the "42-x-y" branch. merged/43-x-y PR was merged to the "43-x-y" branch. needs-manual-bp/41-x-y semver/patch backwards-compatible bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants