From 05fba85aa38dba54e513cca786ed8ffac1f9f6a5 Mon Sep 17 00:00:00 2001 From: Mitchell Cohen Date: Fri, 12 Apr 2024 08:27:59 -0400 Subject: [PATCH] fix: do not activate app when showing a panel on Mac (#41750) * fix: do not activate app when showing or focusing a panel on Mac * restored panel activation test --- shell/browser/native_window_mac.h | 2 ++ shell/browser/native_window_mac.mm | 52 ++++++++++++++---------------- spec/api-browser-window-spec.ts | 3 +- spec/disabled-tests.json | 3 +- 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index fd359e9cce..cf6f0c1d9d 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -176,6 +176,8 @@ class NativeWindowMac : public NativeWindow, void UpdateWindowOriginalFrame(); + bool IsPanel(); + // Set the attribute of NSWindow while work around a bug of zoom button. bool HasStyleMask(NSUInteger flag) const; void SetStyleMask(bool on, NSUInteger flag); diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index f870ba64f9..7fab6b14bd 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -25,6 +25,7 @@ #include "shell/browser/browser.h" #include "shell/browser/javascript_environment.h" #include "shell/browser/ui/cocoa/electron_native_widget_mac.h" +#include "shell/browser/ui/cocoa/electron_ns_panel.h" #include "shell/browser/ui/cocoa/electron_ns_window.h" #include "shell/browser/ui/cocoa/electron_ns_window_delegate.h" #include "shell/browser/ui/cocoa/electron_preview_item.h" @@ -117,11 +118,6 @@ struct Converter { namespace electron { namespace { - -bool IsPanel(NSWindow* window) { - return [window isKindOfClass:[NSPanel class]]; -} - // -[NSWindow orderWindow] does not handle reordering for children // windows. Their order is fixed to the attachment order (the last attached // window is on the top). Therefore, work around it by re-parenting in our @@ -429,30 +425,22 @@ void NativeWindowMac::Focus(bool focus) { if (focus) { // If we're a panel window, we do not want to activate the app, // which enables Electron-apps to build Spotlight-like experiences. - // - // On macOS < Sonoma, "activateIgnoringOtherApps:NO" would not - // activate apps if focusing a window that is inActive. That - // changed with macOS Sonoma, which also deprecated - // "activateIgnoringOtherApps". For the panel-specific usecase, - // we can simply replace "activateIgnoringOtherApps:NO" with - // "activate". For details on why we cannot replace all calls 1:1, - // please see - // https://github.com/electron/electron/pull/40307#issuecomment-1801976591. - // - // There's a slim chance we should have never called - // activateIgnoringOtherApps, but we tried that many years ago - // and saw weird focus bugs on other macOS versions. So, to make - // this safe, we're gating by versions. - if (@available(macOS 14.0, *)) { - if (!IsPanel(window_)) { + if (!IsPanel()) { + // On macOS < Sonoma, "activateIgnoringOtherApps:NO" would not + // activate apps if focusing a window that is inActive. That + // changed with macOS Sonoma, which also deprecated + // "activateIgnoringOtherApps". + // + // There's a slim chance we should have never called + // activateIgnoringOtherApps, but we tried that many years ago + // and saw weird focus bugs on other macOS versions. So, to make + // this safe, we're gating by versions. + if (@available(macOS 14.0, *)) { [[NSApplication sharedApplication] activate]; } else { [[NSApplication sharedApplication] activateIgnoringOtherApps:NO]; } - } else { - [[NSApplication sharedApplication] activateIgnoringOtherApps:NO]; } - [window_ makeKeyAndOrderFront:nil]; } else { [window_ orderOut:nil]; @@ -480,10 +468,14 @@ void NativeWindowMac::Show() { if (parent()) InternalSetParentWindow(parent(), true); - // This method is supposed to put focus on window, however if the app does not - // have focus then "makeKeyAndOrderFront" will only show the window. - [NSApp activateIgnoringOtherApps:YES]; - + // Panels receive key focus when shown but should not activate the app. + if (!IsPanel()) { + if (@available(macOS 14.0, *)) { + [[NSApplication sharedApplication] activate]; + } else { + [[NSApplication sharedApplication] activateIgnoringOtherApps:YES]; + } + } [window_ makeKeyAndOrderFront:nil]; } @@ -625,6 +617,10 @@ bool NativeWindowMac::IsMinimized() const { return [window_ isMiniaturized]; } +bool NativeWindowMac::IsPanel() { + return [window_ isKindOfClass:[ElectronNSPanel class]]; +} + bool NativeWindowMac::HandleDeferredClose() { if (has_deferred_window_close_) { SetHasDeferredWindowClose(false); diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts index 64c794163f..85f690d639 100644 --- a/spec/api-browser-window-spec.ts +++ b/spec/api-browser-window-spec.ts @@ -1246,7 +1246,6 @@ describe('BrowserWindow module', () => { } }); - // FIXME: disabled in `disabled-tests.json` ifit(process.platform === 'darwin')('it does not activate the app if focusing an inactive panel', async () => { // Show to focus app, then remove existing window w.show(); @@ -1269,7 +1268,7 @@ describe('BrowserWindow module', () => { const isShow = once(w, 'show'); const isFocus = once(w, 'focus'); - w.showInactive(); + w.show(); w.focus(); await isShow; diff --git a/spec/disabled-tests.json b/spec/disabled-tests.json index 7419c28138..873ca2415c 100644 --- a/spec/disabled-tests.json +++ b/spec/disabled-tests.json @@ -7,6 +7,5 @@ "session module ses.cookies should set cookie for standard scheme", "webFrameMain module WebFrame.visibilityState should match window state", "reporting api sends a report for a deprecation", - "chromium features SpeechSynthesis should emit lifecycle events", - "BrowserWindow module focus and visibility BrowserWindow.focus() it does not activate the app if focusing an inactive panel" + "chromium features SpeechSynthesis should emit lifecycle events" ] \ No newline at end of file