Skip to content

Commit

Permalink
fix: context-menu event with BaseWindows (electron#44940)
Browse files Browse the repository at this point in the history
fix: context-menu event with BaseWindows
  • Loading branch information
codebytere authored Dec 4, 2024
1 parent 687a59b commit 208dc56
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 27 deletions.
10 changes: 5 additions & 5 deletions docs/api/menu.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)`

Expand Down
2 changes: 0 additions & 2 deletions shell/browser/ui/cocoa/delayed_native_view_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
};
Expand Down
41 changes: 22 additions & 19 deletions shell/browser/ui/cocoa/electron_ns_window.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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<electron::DelayedNativeViewHost*>(wcv)
->GetNativeView()
.GetNativeNSView();
if (!ns_view)
return;

[static_cast<ElectronInspectableWebContentsView*>(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<views::NativeViewHost*>(child);
return nvh->native_view().GetNativeNSView() != [self vibrantView];
}
return false;
});

if (it != children.end()) {
auto ns_view = static_cast<electron::DelayedNativeViewHost*>(*it)
->native_view()
.GetNativeNSView();
if (ns_view) {
[static_cast<ElectronInspectableWebContentsView*>(ns_view)
redispatchContextMenuEvent:base::apple::OwnedNSEvent(event)];
}
}
}

[super sendEvent:event];
Expand Down
53 changes: 52 additions & 1 deletion spec/api-web-contents-spec.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -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', () => {
Expand Down

0 comments on commit 208dc56

Please sign in to comment.