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: do not activate app when showing a panel on Mac #41844

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions shell/browser/native_window_mac.h
Expand Up @@ -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);
Expand Down
52 changes: 24 additions & 28 deletions shell/browser/native_window_mac.mm
Expand Up @@ -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"
Expand Down Expand Up @@ -117,11 +118,6 @@ static bool FromV8(v8::Isolate* isolate,
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
Expand Down Expand Up @@ -428,30 +424,22 @@ void ReorderChildWindowAbove(NSWindow* child_window, NSWindow* other_window) {
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];
Expand Down Expand Up @@ -479,10 +467,14 @@ void ReorderChildWindowAbove(NSWindow* child_window, NSWindow* other_window) {
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];
}

Expand Down Expand Up @@ -624,6 +616,10 @@ void ReorderChildWindowAbove(NSWindow* child_window, NSWindow* other_window) {
return [window_ isMiniaturized];
}

bool NativeWindowMac::IsPanel() {
return [window_ isKindOfClass:[ElectronNSPanel class]];
}

bool NativeWindowMac::HandleDeferredClose() {
if (has_deferred_window_close_) {
SetHasDeferredWindowClose(false);
Expand Down
3 changes: 1 addition & 2 deletions spec/api-browser-window-spec.ts
Expand Up @@ -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();
Expand All @@ -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;
Expand Down
3 changes: 1 addition & 2 deletions spec/disabled-tests.json
Expand Up @@ -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"
]