Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ensure context-menu emitted for draggable regions #44799

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion shell/browser/native_window_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -220,7 +223,6 @@ class NativeWindowMac : public NativeWindow,

protected:
// views::WidgetDelegate:
views::View* GetContentsView() override;
bool CanMaximize() const override;
std::unique_ptr<views::NonClientFrameView> CreateNonClientFrameView(
views::Widget* widget) override;
Expand Down
3 changes: 2 additions & 1 deletion shell/browser/ui/cocoa/delayed_native_view_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down
19 changes: 0 additions & 19 deletions shell/browser/ui/cocoa/delayed_native_view_host.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -22,21 +20,4 @@
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
38 changes: 38 additions & 0 deletions shell/browser/ui/cocoa/electron_ns_window.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -189,6 +191,42 @@ - (NSTouchBar*)makeTouchBar {

// 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<electron::DelayedNativeViewHost*>(wcv)
->GetNativeView()
.GetNativeNSView();
if (!ns_view)
return;

[static_cast<ElectronInspectableWebContentsView*>(ns_view)
redispatchContextMenuEvent:base::apple::OwnedNSEvent(event)];
return;
}

[super sendEvent:event];
}

- (void)rotateWithEvent:(NSEvent*)event {
shell_->NotifyWindowRotateGesture(event.rotation);
}
Expand Down
19 changes: 19 additions & 0 deletions spec/api-web-contents-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
22 changes: 22 additions & 0 deletions spec/fixtures/pages/draggable-page.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!DOCTYPE html>
<html>

<head>
<meta charset="UTF-8">
<title>Draggable Page</title>
<style>
.draggable {
-webkit-app-region: drag;
height: 100vh;
width: 100vw;
margin: 0;
background-color: #f0f0f0;
}
</style>
</head>

<body>
<div class="draggable"></div>
</body>

</html>