fix: do not activate app when showing a panel on Mac (#41844)

* fix: do not activate app when showing or focusing a panel on Mac

Co-authored-by: Mitchell Cohen <[email protected]>

* restored panel activation test

Co-authored-by: Mitchell Cohen <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Mitchell Cohen <[email protected]>

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);

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<electron::NativeWindowMac::VisualEffectState> {
 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
@@ -428,30 +424,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];
@@ -479,10 +467,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];
 }
 
@@ -624,6 +616,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);

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;

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"
 ]