Repository: electron/electron
Pull request: fix: `context-menu` event with `BaseWindows`

#### Description of Change

Closes https://github.com/electron/electron/issues/44898.
Closes https://github.com/electron/electron/issues/44896.
Closes https://github.com/electron/electron/issues/44943.

Fixes an possible crash when using draggable regions and BaseWindows to get the `context-menu` event. My previous fix did not take into account that the `WebContentsView` was only added as as a sibling view in a `BrowserWindow` context. On an implementation level `menu.popup([options])` already takes a BaseWindows, so also update documentation to reflect this.

Tested with https://gist.github.com/09dd58e289571d3956cc95ea92b7d73a

#### Checklist
<!-- Remove items that do not apply. For completed items, change [ ] to [x]. -->

- [x] PR description included and stakeholders cc'd
- [x] `npm test` passes
- [x] tests are [changed or added](https://github.com/electron/electron/blob/main/docs/development/testing.md)
- [x] relevant documentation, tutorials, templates and examples are changed or added
- [x] [PR release notes](https://github.com/electron/clerk/blob/main/README.md) describe the change in a way relevant to app developers, and are [capitalized, punctuated, and past tense](https://github.com/electron/clerk/blob/main/README.md#examples).

#### Release Notes

Notes: Fixed an possible crash when using draggable regions and BaseWindows to get the `context-menu` event.

Changed files:
filename: docs/api/menu.md
patch: @@ -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)`
 

filename: shell/browser/ui/cocoa/delayed_native_view_host.h
patch: @@ -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_;
 };

filename: shell/browser/ui/cocoa/electron_ns_window.mm
patch: @@ -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];

filename: spec/api-web-contents-spec.ts
patch: @@ -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', () => {