From 208dc568d8541d3cdd0fbebc17c348447ba0ae0d Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 4 Dec 2024 22:35:28 +0100 Subject: [PATCH] fix: `context-menu` event with `BaseWindows` (#44940) fix: context-menu event with BaseWindows --- docs/api/menu.md | 10 ++-- .../ui/cocoa/delayed_native_view_host.h | 2 - shell/browser/ui/cocoa/electron_ns_window.mm | 41 +++++++------- spec/api-web-contents-spec.ts | 53 ++++++++++++++++++- 4 files changed, 79 insertions(+), 27 deletions(-) diff --git a/docs/api/menu.md b/docs/api/menu.md index 9be1eedb39d0d..b9a8a7b09e07f 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -72,7 +72,7 @@ The `menu` object has the following instance methods: #### `menu.popup([options])` * `options` Object (optional) - * `window` [BrowserWindow](browser-window.md) (optional) - Default is the focused window. + * `window` [BaseWindow](base-window.md) (optional) - Default is the focused window. * `x` number (optional) - Default is the current mouse cursor position. Must be declared if `y` is declared. * `y` number (optional) - Default is the current mouse cursor position. @@ -86,13 +86,13 @@ The `menu` object has the following instance methods: Can be `none`, `mouse`, `keyboard`, `touch`, `touchMenu`, `longPress`, `longTap`, `touchHandle`, `stylus`, `adjustSelection`, or `adjustSelectionReset`. * `callback` Function (optional) - Called when menu is closed. -Pops up this menu as a context menu in the [`BrowserWindow`](browser-window.md). +Pops up this menu as a context menu in the [`BaseWindow`](base-window.md). -#### `menu.closePopup([browserWindow])` +#### `menu.closePopup([window])` -* `browserWindow` [BrowserWindow](browser-window.md) (optional) - Default is the focused window. +* `window` [BaseWindow](base-window.md) (optional) - Default is the focused window. -Closes the context menu in the `browserWindow`. +Closes the context menu in the `window`. #### `menu.append(menuItem)` diff --git a/shell/browser/ui/cocoa/delayed_native_view_host.h b/shell/browser/ui/cocoa/delayed_native_view_host.h index 5d63fb5cf9413..e7020dba60715 100644 --- a/shell/browser/ui/cocoa/delayed_native_view_host.h +++ b/shell/browser/ui/cocoa/delayed_native_view_host.h @@ -24,8 +24,6 @@ class DelayedNativeViewHost : public views::NativeViewHost { void ViewHierarchyChanged( const views::ViewHierarchyChangedDetails& details) override; - gfx::NativeView GetNativeView() { return native_view_; } - private: gfx::NativeView native_view_; }; diff --git a/shell/browser/ui/cocoa/electron_ns_window.mm b/shell/browser/ui/cocoa/electron_ns_window.mm index ded8d8bcf989f..8da35c0bb75f5 100644 --- a/shell/browser/ui/cocoa/electron_ns_window.mm +++ b/shell/browser/ui/cocoa/electron_ns_window.mm @@ -202,26 +202,29 @@ - (void)sendEvent:(NSEvent*)event { if (event.type == NSEventTypeRightMouseDown || (event.type == NSEventTypeLeftMouseDown && ([event modifierFlags] & NSEventModifierFlagControl))) { - // The WebContentsView is added a sibling of BaseWindow's contentView at - // index 0 before it in the paint order - see - // https://github.com/electron/electron/pull/41256. + // We're looking for the NativeViewHost that contains the WebContentsView. + // There can be two possible NativeViewHosts - one containing the + // WebContentsView (present for BrowserWindows) and the one containing the + // VibrantView (present when vibrancy is set). We want the one containing + // the WebContentsView if it exists. const auto& children = shell_->GetContentsView()->children(); - if (children.empty()) - return; - - auto* wcv = children.front().get(); - if (!wcv) - return; - - auto ns_view = static_cast(wcv) - ->GetNativeView() - .GetNativeNSView(); - if (!ns_view) - return; - - [static_cast(ns_view) - redispatchContextMenuEvent:base::apple::OwnedNSEvent(event)]; - return; + const auto it = std::ranges::find_if(children, [&](views::View* child) { + if (std::strcmp(child->GetClassName(), "NativeViewHost") == 0) { + auto* nvh = static_cast(child); + return nvh->native_view().GetNativeNSView() != [self vibrantView]; + } + return false; + }); + + if (it != children.end()) { + auto ns_view = static_cast(*it) + ->native_view() + .GetNativeNSView(); + if (ns_view) { + [static_cast(ns_view) + redispatchContextMenuEvent:base::apple::OwnedNSEvent(event)]; + } + } } [super sendEvent:event]; diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts index 3a2eb81aac0dc..8fc502c2ca582 100644 --- a/spec/api-web-contents-spec.ts +++ b/spec/api-web-contents-spec.ts @@ -1,4 +1,4 @@ -import { BrowserWindow, ipcMain, webContents, session, app, BrowserView, WebContents } from 'electron/main'; +import { BrowserWindow, ipcMain, webContents, session, app, BrowserView, WebContents, BaseWindow, WebContentsView } from 'electron/main'; import { expect } from 'chai'; @@ -2749,6 +2749,57 @@ describe('webContents module', () => { expect(params.x).to.be.a('number'); expect(params.y).to.be.a('number'); }); + + it('emits when right clicked in a WebContentsView', async () => { + const w = new BaseWindow({ show: false }); + + const mainView = new WebContentsView({ + webPreferences: { + preload: path.join(__dirname, 'preload.js') + } + }); + + const draggablePage = path.join(fixturesPath, 'pages', 'draggable-page.html'); + await mainView.webContents.loadFile(draggablePage); + + w.contentView.addChildView(mainView); + + const { width, height } = w.getContentBounds(); + mainView.setBounds({ x: 0, y: 0, width, height }); + + const promise = once(mainView.webContents, 'context-menu') as Promise<[any, Electron.ContextMenuParams]>; + + // Simulate right-click to create context-menu event. + const opts = { x: 0, y: 0, button: 'right' as const }; + mainView.webContents.sendInputEvent({ ...opts, type: 'mouseDown' }); + mainView.webContents.sendInputEvent({ ...opts, type: 'mouseUp' }); + + const [, params] = await promise; + + expect(params.pageURL).to.equal(mainView.webContents.getURL()); + expect(params.frame).to.be.an('object'); + expect(params.x).to.be.a('number'); + expect(params.y).to.be.a('number'); + }); + + it('emits when right clicked in a BrowserWindow with vibrancy', async () => { + const w = new BrowserWindow({ show: false, vibrancy: 'titlebar' }); + await w.loadFile(path.join(fixturesPath, 'pages', 'draggable-page.html')); + + const promise = once(w.webContents, 'context-menu') as Promise<[any, Electron.ContextMenuParams]>; + + // Simulate right-click to create context-menu event. + const opts = { x: 0, y: 0, button: 'right' as const }; + w.webContents.sendInputEvent({ ...opts, type: 'mouseDown' }); + w.webContents.sendInputEvent({ ...opts, type: 'mouseUp' }); + + const [, params] = await promise; + + expect(params.pageURL).to.equal(w.webContents.getURL()); + expect(params.frame).to.be.an('object'); + expect(params.x).to.be.a('number'); + expect(params.y).to.be.a('number'); + }); }); describe('close() method', () => {