mirror of
https://github.com/electron/electron.git
synced 2026-10-28 11:44:46 +08:00
fix: handle menu shortcuts without preferences (#54014)
* fix: handle menu shortcuts without preferences DevTools contents legitimately lack WebContentsPreferences, so resetting menu shortcuts dereferences null. Keep runtime state on the source API WebContents to preserve forwarded shortcut handling. * test: fix macOS menu shortcut input macOS menu dispatch requires a native NSEvent. sendInputEvent does not create one, so the shortcut tests time out. Use CDP to supply native F13 input on macOS.
This commit is contained in:
1 parent
c3485835c2
commit
bbe4cdfea4
5 files changed
+125
-18
No files matched your search
@@ -1256,9 +1256,13 @@ void WebContents::InitWithSessionAndOptions(
|
||||
// Save the preferences in C++.
|
||||
// If there's already a WebContentsPreferences object, we created it as part
|
||||
// of the webContents.setWindowOpenHandler path, so don't overwrite it.
|
||||
if (!WebContentsPreferences::From(web_contents())) {
|
||||
new WebContentsPreferences(web_contents(), options);
|
||||
}
|
||||
// WebContentsPreferences transfers ownership to WebContents in its
|
||||
// constructor. NOLINTBEGIN(clang-analyzer-cplusplus.NewDeleteLeaks)
|
||||
auto* web_preferences = WebContentsPreferences::From(web_contents());
|
||||
if (!web_preferences)
|
||||
web_preferences = new WebContentsPreferences(web_contents(), options);
|
||||
ignore_menu_shortcuts_ = web_preferences->ShouldIgnoreMenuShortcuts();
|
||||
// NOLINTEND(clang-analyzer-cplusplus.NewDeleteLeaks)
|
||||
// Trigger re-calculation of webkit prefs.
|
||||
web_contents()->NotifyPreferencesChanged();
|
||||
|
||||
@@ -1853,9 +1857,7 @@ bool WebContents::HandleKeyboardEvent(
|
||||
bool WebContents::PlatformHandleKeyboardEvent(
|
||||
content::WebContents* source,
|
||||
const input::NativeWebKeyboardEvent& event) {
|
||||
// Check if the webContents has preferences and to ignore shortcuts
|
||||
auto* web_preferences = WebContentsPreferences::From(source);
|
||||
if (web_preferences && web_preferences->ShouldIgnoreMenuShortcuts())
|
||||
if (ShouldIgnoreMenuShortcutsFor(source))
|
||||
return false;
|
||||
|
||||
// Let the NativeWindow handle other parts.
|
||||
@@ -3919,10 +3921,14 @@ void WebContents::InspectServiceWorker() {
|
||||
}
|
||||
}
|
||||
|
||||
void WebContents::SetIgnoreMenuShortcuts(bool ignore) {
|
||||
auto* web_preferences = WebContentsPreferences::From(web_contents());
|
||||
DCHECK(web_preferences);
|
||||
web_preferences->SetIgnoreMenuShortcuts(ignore);
|
||||
bool WebContents::ShouldIgnoreMenuShortcutsFor(
|
||||
content::WebContents* const source) {
|
||||
const auto* source_contents = From(source);
|
||||
return source_contents && source_contents->ignore_menu_shortcuts_;
|
||||
}
|
||||
|
||||
void WebContents::SetIgnoreMenuShortcuts(const bool ignore) {
|
||||
ignore_menu_shortcuts_ = ignore;
|
||||
}
|
||||
|
||||
void WebContents::SetAudioMuted(bool muted) {
|
||||
@@ -5847,6 +5853,8 @@ gin_helper::Handle<WebContents> WebContents::CreateFromWebPreferences(
|
||||
if (gin::ConvertFromV8(isolate, web_preferences.GetHandle(),
|
||||
&web_preferences_dict)) {
|
||||
existing_preferences->SetFromDictionary(web_preferences_dict);
|
||||
web_contents->SetIgnoreMenuShortcuts(
|
||||
existing_preferences->ShouldIgnoreMenuShortcuts());
|
||||
web_contents->SetBackgroundColor(
|
||||
existing_preferences->GetBackgroundColor());
|
||||
|
||||
|
||||
@@ -860,6 +860,9 @@ class WebContents final : public ExclusiveAccessContext,
|
||||
|
||||
[[nodiscard]] bool CanGoToIndex(int index) const;
|
||||
|
||||
[[nodiscard]] static bool ShouldIgnoreMenuShortcutsFor(
|
||||
content::WebContents* source);
|
||||
|
||||
cppgc::Persistent<api::Session> session_;
|
||||
v8::Global<v8::Value> devtools_web_contents_;
|
||||
cppgc::Persistent<api::Debugger> debugger_;
|
||||
@@ -891,6 +894,8 @@ class WebContents final : public ExclusiveAccessContext,
|
||||
// Whether background throttling is disabled.
|
||||
bool background_throttling_ = true;
|
||||
|
||||
bool ignore_menu_shortcuts_ = false;
|
||||
|
||||
// Kept by JS while 'console-message' has listeners.
|
||||
bool console_message_observed_ = false;
|
||||
|
||||
|
||||
@@ -5,7 +5,6 @@
|
||||
#include "content/public/browser/render_widget_host_view.h"
|
||||
#include "shell/browser/api/electron_api_web_contents.h"
|
||||
#include "shell/browser/ui/cocoa/event_dispatching_window.h"
|
||||
#include "shell/browser/web_contents_preferences.h"
|
||||
#include "ui/base/cocoa/command_dispatcher.h"
|
||||
#include "ui/base/cocoa/nsmenu_additions.h"
|
||||
#include "ui/base/cocoa/nsmenuitem_additions.h"
|
||||
@@ -42,9 +41,7 @@ bool WebContents::PlatformHandleKeyboardEvent(
|
||||
event.GetType() == input::NativeWebKeyboardEvent::Type::kChar)
|
||||
return false;
|
||||
|
||||
// Check if the webContents has preferences and to ignore shortcuts
|
||||
auto* web_preferences = WebContentsPreferences::From(source);
|
||||
if (web_preferences && web_preferences->ShouldIgnoreMenuShortcuts())
|
||||
if (ShouldIgnoreMenuShortcutsFor(source))
|
||||
return false;
|
||||
|
||||
NSEvent* ns_event = event.os_event.Get();
|
||||
|
||||
@@ -107,9 +107,6 @@ class WebContentsPreferences
|
||||
bool ShouldUsePreferredSizeMode() const {
|
||||
return enable_preferred_size_mode_;
|
||||
}
|
||||
void SetIgnoreMenuShortcuts(bool ignore_menu_shortcuts) {
|
||||
ignore_menu_shortcuts_ = ignore_menu_shortcuts;
|
||||
}
|
||||
bool ShouldIgnoreMenuShortcuts() const { return ignore_menu_shortcuts_; }
|
||||
bool SetImageAnimationPolicy(std::string policy);
|
||||
bool ShouldDisableHtmlFullscreenWindowResize() const {
|
||||
|
||||
@@ -7,7 +7,8 @@ import {
|
||||
BrowserView,
|
||||
WebContents,
|
||||
BaseWindow,
|
||||
WebContentsView
|
||||
WebContentsView,
|
||||
Menu
|
||||
} from 'electron/main';
|
||||
|
||||
import { assert, expect } from 'chai';
|
||||
@@ -3953,6 +3954,43 @@ describe('webContents module', () => {
|
||||
|
||||
describe('setIgnoreMenuShortcuts(ignore)', () => {
|
||||
afterEach(closeAllWindows);
|
||||
|
||||
const trackShortcutInvocations = (contents: WebContents) => {
|
||||
const previousMenu = Menu.getApplicationMenu();
|
||||
let invocations = 0;
|
||||
Menu.setApplicationMenu(
|
||||
Menu.buildFromTemplate([
|
||||
{
|
||||
label: 'Test',
|
||||
submenu: [{ label: 'Shortcut', accelerator: 'F13', click: () => invocations++ }]
|
||||
}
|
||||
])
|
||||
);
|
||||
contents.debugger.attach();
|
||||
defer(() => {
|
||||
Menu.setApplicationMenu(previousMenu);
|
||||
if (!contents.isDestroyed() && contents.debugger.isAttached()) contents.debugger.detach();
|
||||
});
|
||||
|
||||
return async (expectedInvocations: number) => {
|
||||
const base = { key: 'F13', code: 'F13', windowsVirtualKeyCode: 124 };
|
||||
if (process.platform === 'darwin') {
|
||||
// Native macOS menu input requires kVK_F13 and NSF13FunctionKey.
|
||||
await contents.debugger.sendCommand('Input.dispatchKeyEvent', {
|
||||
...base,
|
||||
type: 'rawKeyDown',
|
||||
nativeVirtualKeyCode: 0x69,
|
||||
text: '\uF710'
|
||||
});
|
||||
} else {
|
||||
contents.sendInputEvent({ type: 'keyDown', keyCode: 'F13' });
|
||||
}
|
||||
await contents.debugger.sendCommand('Input.dispatchKeyEvent', { ...base, type: 'keyUp' });
|
||||
await waitUntil(() => invocations >= expectedInvocations);
|
||||
return invocations;
|
||||
};
|
||||
};
|
||||
|
||||
it('does not throw', () => {
|
||||
const w = new BrowserWindow({ show: false });
|
||||
expect(() => {
|
||||
@@ -3960,6 +3998,68 @@ describe('webContents module', () => {
|
||||
w.webContents.setIgnoreMenuShortcuts(false);
|
||||
}).to.not.throw();
|
||||
});
|
||||
|
||||
it('honors the initial ignoreMenuShortcuts preference', async () => {
|
||||
const window = new BrowserWindow({
|
||||
show: true,
|
||||
webPreferences: { ignoreMenuShortcuts: true } as Electron.WebPreferences
|
||||
});
|
||||
await window.loadURL('about:blank');
|
||||
window.webContents.focus();
|
||||
const sendShortcut = trackShortcutInvocations(window.webContents);
|
||||
expect(await sendShortcut(0)).to.equal(0);
|
||||
window.webContents.setIgnoreMenuShortcuts(false);
|
||||
expect(await sendShortcut(1)).to.equal(1);
|
||||
});
|
||||
|
||||
it('does not crash for detached DevTools without preferences', async () => {
|
||||
const window = new BrowserWindow({ show: false });
|
||||
await window.loadURL('about:blank');
|
||||
const devToolsOpened = once(window.webContents, 'devtools-opened');
|
||||
window.webContents.openDevTools({ mode: 'detach', activate: false });
|
||||
await devToolsOpened;
|
||||
|
||||
const devTools = window.webContents.devToolsWebContents!;
|
||||
expect(devTools.getLastWebPreferences()).to.equal(null);
|
||||
devTools.setIgnoreMenuShortcuts(false);
|
||||
devTools.setIgnoreMenuShortcuts(true);
|
||||
devTools.setIgnoreMenuShortcuts(false);
|
||||
});
|
||||
|
||||
for (const target of ['webview', 'docked DevTools'] as const) {
|
||||
it(`uses the source settings for ${target}`, async () => {
|
||||
const window = new BrowserWindow({ show: true, webPreferences: { webviewTag: true } });
|
||||
let source: WebContents;
|
||||
if (target === 'webview') {
|
||||
const attached = once(window.webContents, 'did-attach-webview') as Promise<[any, WebContents]>;
|
||||
await window.loadFile(path.join(fixturesPath, 'pages', 'webview-zoom-factor.html'));
|
||||
[, source] = await attached;
|
||||
await source.loadURL('about:blank');
|
||||
} else {
|
||||
await window.loadURL('about:blank');
|
||||
const devToolsOpened = once(window.webContents, 'devtools-opened');
|
||||
const devToolsFocused = once(window.webContents, 'devtools-focused');
|
||||
window.webContents.openDevTools({ mode: 'right', activate: true });
|
||||
await Promise.all([devToolsOpened, devToolsFocused]);
|
||||
source = window.webContents.devToolsWebContents!;
|
||||
expect(source.getLastWebPreferences()).to.equal(null);
|
||||
source.setIgnoreMenuShortcuts(false);
|
||||
}
|
||||
|
||||
const sendShortcut = trackShortcutInvocations(source);
|
||||
let expectedInvocations = 0;
|
||||
for (const ignore of [true, false, true, false]) {
|
||||
if (target === 'webview') {
|
||||
window.focus();
|
||||
await window.webContents.executeJavaScript("document.querySelector('webview').focus()");
|
||||
}
|
||||
window.webContents.setIgnoreMenuShortcuts(!ignore);
|
||||
source.setIgnoreMenuShortcuts(ignore);
|
||||
if (!ignore) expectedInvocations++;
|
||||
expect(await sendShortcut(expectedInvocations)).to.equal(expectedInvocations);
|
||||
}
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
const crashPrefs = [
|
||||
|
||||
Reference in new issue
Block a user