From d2bafe870e7bd983f27dadb90e01fe4ecddbfa09 Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Fri, 22 Nov 2024 09:28:12 -0500 Subject: [PATCH] fix: ensure `context-menu` emitted for draggable regions (#44799) * fix: ensure context-menu emitted for draggable regions Co-authored-by: Shelley Vohr * chore: address suggestions from review Co-authored-by: Shelley Vohr --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr --- shell/browser/native_window_mac.h | 4 +- .../ui/cocoa/delayed_native_view_host.h | 3 +- .../ui/cocoa/delayed_native_view_host.mm | 19 ---------- shell/browser/ui/cocoa/electron_ns_window.mm | 38 +++++++++++++++++++ spec/api-web-contents-spec.ts | 19 ++++++++++ spec/fixtures/pages/draggable-page.html | 22 +++++++++++ 6 files changed, 84 insertions(+), 21 deletions(-) create mode 100644 spec/fixtures/pages/draggable-page.html diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index a841f2dd5f..0444631873 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -169,6 +169,9 @@ class NativeWindowMac : public NativeWindow, void NotifyWindowDidFailToEnterFullScreen(); void NotifyWindowWillLeaveFullScreen(); + // views::WidgetDelegate: + views::View* GetContentsView() override; + // Cleanup observers when window is getting closed. Note that the destructor // can be called much later after window gets closed, so we should not do // cleanup in destructor. @@ -220,7 +223,6 @@ class NativeWindowMac : public NativeWindow, protected: // views::WidgetDelegate: - views::View* GetContentsView() override; bool CanMaximize() const override; std::unique_ptr CreateNonClientFrameView( views::Widget* widget) override; diff --git a/shell/browser/ui/cocoa/delayed_native_view_host.h b/shell/browser/ui/cocoa/delayed_native_view_host.h index eb02826c2b..5d63fb5cf9 100644 --- a/shell/browser/ui/cocoa/delayed_native_view_host.h +++ b/shell/browser/ui/cocoa/delayed_native_view_host.h @@ -23,7 +23,8 @@ class DelayedNativeViewHost : public views::NativeViewHost { // views::View: void ViewHierarchyChanged( const views::ViewHierarchyChangedDetails& details) override; - bool OnMousePressed(const ui::MouseEvent& event) override; + + gfx::NativeView GetNativeView() { return native_view_; } private: gfx::NativeView native_view_; diff --git a/shell/browser/ui/cocoa/delayed_native_view_host.mm b/shell/browser/ui/cocoa/delayed_native_view_host.mm index 78a5668a4d..38b0796555 100644 --- a/shell/browser/ui/cocoa/delayed_native_view_host.mm +++ b/shell/browser/ui/cocoa/delayed_native_view_host.mm @@ -3,8 +3,6 @@ // found in the LICENSE file. #include "shell/browser/ui/cocoa/delayed_native_view_host.h" -#include "base/apple/owned_objc.h" -#include "shell/browser/ui/cocoa/electron_inspectable_web_contents_view.h" namespace electron { @@ -22,21 +20,4 @@ void DelayedNativeViewHost::ViewHierarchyChanged( Attach(native_view_); } -bool DelayedNativeViewHost::OnMousePressed(const ui::MouseEvent& ui_event) { - // NativeViewHost::OnMousePressed normally isn't called, but - // NativeWidgetMacNSWindow specifically carves out an event here for - // right-mouse-button clicks. We want to forward them to the web content, so - // handle them here. - // See: - // https://source.chromium.org/chromium/chromium/src/+/main:components/remote_cocoa/app_shim/native_widget_mac_nswindow.mm;l=415-421;drc=a5af91924bafb85426e091c6035801990a6dc697 - - ElectronInspectableWebContentsView* inspectable_web_contents_view = - (ElectronInspectableWebContentsView*)native_view_.GetNativeNSView(); - [inspectable_web_contents_view - redispatchContextMenuEvent:base::apple::OwnedNSEvent( - ui_event.native_event())]; - - return true; -} - } // namespace electron diff --git a/shell/browser/ui/cocoa/electron_ns_window.mm b/shell/browser/ui/cocoa/electron_ns_window.mm index 573f4de577..ded8d8bcf9 100644 --- a/shell/browser/ui/cocoa/electron_ns_window.mm +++ b/shell/browser/ui/cocoa/electron_ns_window.mm @@ -7,6 +7,8 @@ #include "base/strings/sys_string_conversions.h" #include "electron/mas.h" #include "shell/browser/native_window_mac.h" +#include "shell/browser/ui/cocoa/delayed_native_view_host.h" +#include "shell/browser/ui/cocoa/electron_inspectable_web_contents_view.h" #include "shell/browser/ui/cocoa/electron_preview_item.h" #include "shell/browser/ui/cocoa/electron_touch_bar.h" #include "shell/browser/ui/cocoa/root_view_mac.h" @@ -189,6 +191,42 @@ void SwizzleSwipeWithEvent(NSView* view, SEL swiz_selector) { // NSWindow overrides. +- (void)sendEvent:(NSEvent*)event { + // Draggable regions only respond to left-click dragging, but the system will + // still suppress right-clicks in a draggable region. Forwarding right-clicks + // and ctrl+left-clicks allows the underlying views to respond to right-click + // to potentially bring up a frame context menu. WebContentsView is now a + // sibling view of the NSWindow contentView, so we need to intercept the event + // here as NativeWidgetMacNSWindow won't forward it to the WebContentsView + // anymore. + 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. + 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; + } + + [super sendEvent:event]; +} + - (void)rotateWithEvent:(NSEvent*)event { shell_->NotifyWindowRotateGesture(event.rotation); } diff --git a/spec/api-web-contents-spec.ts b/spec/api-web-contents-spec.ts index 2e584c309c..91e2979830 100644 --- a/spec/api-web-contents-spec.ts +++ b/spec/api-web-contents-spec.ts @@ -2730,6 +2730,25 @@ describe('webContents module', () => { expect(params.x).to.be.a('number'); expect(params.y).to.be.a('number'); }); + + it('emits when right-clicked in page in a draggable region', async () => { + const w = new BrowserWindow({ show: false }); + 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', () => { diff --git a/spec/fixtures/pages/draggable-page.html b/spec/fixtures/pages/draggable-page.html new file mode 100644 index 0000000000..7b106e5cea --- /dev/null +++ b/spec/fixtures/pages/draggable-page.html @@ -0,0 +1,22 @@ + + + + + + Draggable Page + + + + +
+ + +