From 44cbc42e4c95565c776019bb06dc080509611cad Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 21 Jun 2021 12:19:19 +0200 Subject: [PATCH 1/4] fix: child window alwaysOnTop level --- shell/browser/native_window_mac.mm | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index 58939680b6e31..ebf2682aa72e2 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -1805,10 +1805,15 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { // Set new parent window. // Note that this method will force the window to become visible. - if (parent && attach) + if (parent && attach) { + // Attaching a window as a child window resets its window level, so + // save and restore it afterwards. + NSInteger level = window_.level; [parent->GetNativeWindow().GetNativeNSWindow() addChildWindow:window_ ordered:NSWindowAbove]; + [window_ setLevel:level]; + } } void NativeWindowMac::SetForwardMouseMessages(bool forward) { From bf37970dcae0b6813f5aad9d3d8f4f47b393ad36 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 22 Jun 2021 10:49:37 +0200 Subject: [PATCH 2/4] chore: add undocumented getAlwaysOnTopLevel --- shell/browser/api/electron_api_base_window.cc | 22 ++++++++++------ shell/browser/api/electron_api_base_window.h | 2 ++ shell/browser/native_window.h | 1 + shell/browser/native_window_mac.h | 1 + shell/browser/native_window_mac.mm | 25 +++++++++++++++++++ shell/browser/native_window_views.h | 1 + 6 files changed, 44 insertions(+), 8 deletions(-) diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc index fd11478d790b0..fd6b226d4dfff 100644 --- a/shell/browser/api/electron_api_base_window.cc +++ b/shell/browser/api/electron_api_base_window.cc @@ -559,6 +559,14 @@ bool BaseWindow::IsAlwaysOnTop() { return window_->GetZOrderLevel() != ui::ZOrderLevel::kNormal; } +std::string BaseWindow::GetAlwaysOnTopLevel() { + return window_->GetAlwaysOnTopLevel(); +} + +int BaseWindow::GetZOrderLevel() { + return static_cast(window_->GetZOrderLevel()); +} + void BaseWindow::Center() { window_->Center(); } @@ -1226,6 +1234,8 @@ void BaseWindow::BuildPrototype(v8::Isolate* isolate, .SetMethod("isClosable", &BaseWindow::IsClosable) .SetMethod("setAlwaysOnTop", &BaseWindow::SetAlwaysOnTop) .SetMethod("isAlwaysOnTop", &BaseWindow::IsAlwaysOnTop) + .SetMethod("getAlwaysOnTopLevel", &BaseWindow::GetAlwaysOnTopLevel) + .SetMethod("getZOrderLevel", &BaseWindow::GetZOrderLevel) .SetMethod("center", &BaseWindow::Center) .SetMethod("setPosition", &BaseWindow::SetPosition) .SetMethod("getPosition", &BaseWindow::GetPosition) @@ -1270,20 +1280,16 @@ void BaseWindow::BuildPrototype(v8::Isolate* isolate, &BaseWindow::SetVisibleOnAllWorkspaces) .SetMethod("isVisibleOnAllWorkspaces", &BaseWindow::IsVisibleOnAllWorkspaces) -#if defined(OS_MAC) - .SetMethod("setAutoHideCursor", &BaseWindow::SetAutoHideCursor) -#endif .SetMethod("setVibrancy", &BaseWindow::SetVibrancy) + .SetMethod("_setTouchBarItems", &BaseWindow::SetTouchBar) + .SetMethod("_refreshTouchBarItem", &BaseWindow::RefreshTouchBarItem) + .SetMethod("_setEscapeTouchBarItem", &BaseWindow::SetEscapeTouchBarItem) #if defined(OS_MAC) + .SetMethod("setAutoHideCursor", &BaseWindow::SetAutoHideCursor) .SetMethod("setTrafficLightPosition", &BaseWindow::SetTrafficLightPosition) .SetMethod("getTrafficLightPosition", &BaseWindow::GetTrafficLightPosition) -#endif - .SetMethod("_setTouchBarItems", &BaseWindow::SetTouchBar) - .SetMethod("_refreshTouchBarItem", &BaseWindow::RefreshTouchBarItem) - .SetMethod("_setEscapeTouchBarItem", &BaseWindow::SetEscapeTouchBarItem) -#if defined(OS_MAC) .SetMethod("selectPreviousTab", &BaseWindow::SelectPreviousTab) .SetMethod("selectNextTab", &BaseWindow::SelectNextTab) .SetMethod("mergeAllWindows", &BaseWindow::MergeAllWindows) diff --git a/shell/browser/api/electron_api_base_window.h b/shell/browser/api/electron_api_base_window.h index d302ed8ea0298..9306b0e7c77b3 100644 --- a/shell/browser/api/electron_api_base_window.h +++ b/shell/browser/api/electron_api_base_window.h @@ -142,6 +142,8 @@ class BaseWindow : public gin_helper::TrackableObject, bool IsClosable(); void SetAlwaysOnTop(bool top, gin_helper::Arguments* args); bool IsAlwaysOnTop(); + std::string GetAlwaysOnTopLevel(); + int GetZOrderLevel(); void Center(); void SetPosition(int x, int y, gin_helper::Arguments* args); std::vector GetPosition(); diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index 0c166612ab1e5..e9ec5362cef1e 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -129,6 +129,7 @@ class NativeWindow : public base::SupportsUserData, virtual void SetAlwaysOnTop(ui::ZOrderLevel z_order, const std::string& level = "floating", int relativeLevel = 0) = 0; + virtual std::string GetAlwaysOnTopLevel() = 0; virtual ui::ZOrderLevel GetZOrderLevel() = 0; virtual void Center() = 0; virtual void Invalidate() = 0; diff --git a/shell/browser/native_window_mac.h b/shell/browser/native_window_mac.h index a7201646f4fb4..1bbe3862f7ae2 100644 --- a/shell/browser/native_window_mac.h +++ b/shell/browser/native_window_mac.h @@ -78,6 +78,7 @@ class NativeWindowMac : public NativeWindow, void SetAlwaysOnTop(ui::ZOrderLevel z_order, const std::string& level, int relative_level) override; + std::string GetAlwaysOnTopLevel() override; ui::ZOrderLevel GetZOrderLevel() override; void Center() override; void Invalidate() override; diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index ebf2682aa72e2..32dfd555485e8 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -869,6 +869,31 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { SetWindowLevel(level + relative_level); } +std::string NativeWindowMac::GetAlwaysOnTopLevel() { + std::string level_name = "normal"; + + int level = [window_ level]; + if (level == NSFloatingWindowLevel) { + level_name = "floating"; + } else if (level == NSTornOffMenuWindowLevel) { + level_name = "torn-off-menu"; + } else if (level == NSModalPanelWindowLevel) { + level_name = "modal-panel"; + } else if (level == NSMainMenuWindowLevel) { + level_name = "main-menu"; + } else if (level == NSStatusWindowLevel) { + level_name = "status"; + } else if (level == NSPopUpMenuWindowLevel) { + level_name = "pop-up-menu"; + } else if (level == NSScreenSaverWindowLevel) { + level_name = "screen-saver"; + } else if (level == NSDockWindowLevel) { + level_name = "dock"; + } + + return level_name; +} + void NativeWindowMac::SetWindowLevel(int unbounded_level) { int level = std::min( std::max(unbounded_level, CGWindowLevelForKey(kCGMinimumWindowLevelKey)), diff --git a/shell/browser/native_window_views.h b/shell/browser/native_window_views.h index f48ce5c2739cc..781ddd59ecf87 100644 --- a/shell/browser/native_window_views.h +++ b/shell/browser/native_window_views.h @@ -93,6 +93,7 @@ class NativeWindowViews : public NativeWindow, void SetAlwaysOnTop(ui::ZOrderLevel z_order, const std::string& level, int relativeLevel) override; + std::string GetAlwaysOnTopLevel() override; ui::ZOrderLevel GetZOrderLevel() override; void Center() override; void Invalidate() override; From fd6b251326194655955f13e3fe0ed04c86d5f608 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 22 Jun 2021 10:49:49 +0200 Subject: [PATCH 3/4] test: add test for level persistence --- shell/browser/api/electron_api_base_window.cc | 19 +++++++++---------- shell/browser/api/electron_api_base_window.h | 1 - spec-main/api-browser-window-spec.ts | 17 ++++++++++++----- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc index fd6b226d4dfff..0c9f1e746dc30 100644 --- a/shell/browser/api/electron_api_base_window.cc +++ b/shell/browser/api/electron_api_base_window.cc @@ -563,10 +563,6 @@ std::string BaseWindow::GetAlwaysOnTopLevel() { return window_->GetAlwaysOnTopLevel(); } -int BaseWindow::GetZOrderLevel() { - return static_cast(window_->GetZOrderLevel()); -} - void BaseWindow::Center() { window_->Center(); } @@ -1234,8 +1230,6 @@ void BaseWindow::BuildPrototype(v8::Isolate* isolate, .SetMethod("isClosable", &BaseWindow::IsClosable) .SetMethod("setAlwaysOnTop", &BaseWindow::SetAlwaysOnTop) .SetMethod("isAlwaysOnTop", &BaseWindow::IsAlwaysOnTop) - .SetMethod("getAlwaysOnTopLevel", &BaseWindow::GetAlwaysOnTopLevel) - .SetMethod("getZOrderLevel", &BaseWindow::GetZOrderLevel) .SetMethod("center", &BaseWindow::Center) .SetMethod("setPosition", &BaseWindow::SetPosition) .SetMethod("getPosition", &BaseWindow::GetPosition) @@ -1280,16 +1274,21 @@ void BaseWindow::BuildPrototype(v8::Isolate* isolate, &BaseWindow::SetVisibleOnAllWorkspaces) .SetMethod("isVisibleOnAllWorkspaces", &BaseWindow::IsVisibleOnAllWorkspaces) - .SetMethod("setVibrancy", &BaseWindow::SetVibrancy) - .SetMethod("_setTouchBarItems", &BaseWindow::SetTouchBar) - .SetMethod("_refreshTouchBarItem", &BaseWindow::RefreshTouchBarItem) - .SetMethod("_setEscapeTouchBarItem", &BaseWindow::SetEscapeTouchBarItem) #if defined(OS_MAC) + .SetMethod("getAlwaysOnTopLevel", &BaseWindow::GetAlwaysOnTopLevel) .SetMethod("setAutoHideCursor", &BaseWindow::SetAutoHideCursor) +#endif + .SetMethod("setVibrancy", &BaseWindow::SetVibrancy) +#if defined(OS_MAC) .SetMethod("setTrafficLightPosition", &BaseWindow::SetTrafficLightPosition) .SetMethod("getTrafficLightPosition", &BaseWindow::GetTrafficLightPosition) +#endif + .SetMethod("_setTouchBarItems", &BaseWindow::SetTouchBar) + .SetMethod("_refreshTouchBarItem", &BaseWindow::RefreshTouchBarItem) + .SetMethod("_setEscapeTouchBarItem", &BaseWindow::SetEscapeTouchBarItem) +#if defined(OS_MAC) .SetMethod("selectPreviousTab", &BaseWindow::SelectPreviousTab) .SetMethod("selectNextTab", &BaseWindow::SelectNextTab) .SetMethod("mergeAllWindows", &BaseWindow::MergeAllWindows) diff --git a/shell/browser/api/electron_api_base_window.h b/shell/browser/api/electron_api_base_window.h index 9306b0e7c77b3..7613fd38d1c9e 100644 --- a/shell/browser/api/electron_api_base_window.h +++ b/shell/browser/api/electron_api_base_window.h @@ -143,7 +143,6 @@ class BaseWindow : public gin_helper::TrackableObject, void SetAlwaysOnTop(bool top, gin_helper::Arguments* args); bool IsAlwaysOnTop(); std::string GetAlwaysOnTopLevel(); - int GetZOrderLevel(); void Center(); void SetPosition(int x, int y, gin_helper::Arguments* args); std::vector GetPosition(); diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 42da9aa40885a..8fa32c1d55f50 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -1430,15 +1430,12 @@ describe('BrowserWindow module', () => { describe('BrowserWindow.setAlwaysOnTop(flag, level)', () => { let w = null as unknown as BrowserWindow; + afterEach(closeAllWindows); + beforeEach(() => { w = new BrowserWindow({ show: true }); }); - afterEach(async () => { - await closeWindow(w); - w = null as unknown as BrowserWindow; - }); - it('sets the window as always on top', () => { expect(w.isAlwaysOnTop()).to.be.false('is alwaysOnTop'); w.setAlwaysOnTop(true, 'screen-saver'); @@ -1466,6 +1463,16 @@ describe('BrowserWindow module', () => { const [, alwaysOnTop] = await alwaysOnTopChanged; expect(alwaysOnTop).to.be.true('is not alwaysOnTop'); }); + + ifit(process.platform === 'darwin')('honors the alwaysOnTop level of a child window', () => { + w = new BrowserWindow({ show: false }); + const c = new BrowserWindow({ parent: w }); + c.setAlwaysOnTop(true, 'screen-saver'); + + expect(w.isAlwaysOnTop()).to.be.false(); + expect(c.isAlwaysOnTop()).to.be.true('child is not always on top'); + expect((c as any).getAlwaysOnTopLevel()).to.equal('screen-saver'); + }); }); describe('preconnect feature', () => { From 4cc5d30fd8b3be59c7247b8ee91786cc34f27fee Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 22 Jun 2021 19:28:29 +0200 Subject: [PATCH 4/4] Address feedback from review --- shell/browser/api/electron_api_base_window.cc | 10 +++++----- shell/browser/api/electron_api_base_window.h | 2 +- shell/browser/native_window.h | 2 +- shell/browser/native_window_views.h | 1 - spec-main/api-browser-window-spec.ts | 2 +- 5 files changed, 8 insertions(+), 9 deletions(-) diff --git a/shell/browser/api/electron_api_base_window.cc b/shell/browser/api/electron_api_base_window.cc index 0c9f1e746dc30..5cf2196535f08 100644 --- a/shell/browser/api/electron_api_base_window.cc +++ b/shell/browser/api/electron_api_base_window.cc @@ -559,10 +559,6 @@ bool BaseWindow::IsAlwaysOnTop() { return window_->GetZOrderLevel() != ui::ZOrderLevel::kNormal; } -std::string BaseWindow::GetAlwaysOnTopLevel() { - return window_->GetAlwaysOnTopLevel(); -} - void BaseWindow::Center() { window_->Center(); } @@ -886,6 +882,10 @@ void BaseWindow::SetVibrancy(v8::Isolate* isolate, v8::Local value) { } #if defined(OS_MAC) +std::string BaseWindow::GetAlwaysOnTopLevel() { + return window_->GetAlwaysOnTopLevel(); +} + void BaseWindow::SetWindowButtonVisibility(bool visible) { window_->SetWindowButtonVisibility(visible); } @@ -1275,7 +1275,7 @@ void BaseWindow::BuildPrototype(v8::Isolate* isolate, .SetMethod("isVisibleOnAllWorkspaces", &BaseWindow::IsVisibleOnAllWorkspaces) #if defined(OS_MAC) - .SetMethod("getAlwaysOnTopLevel", &BaseWindow::GetAlwaysOnTopLevel) + .SetMethod("_getAlwaysOnTopLevel", &BaseWindow::GetAlwaysOnTopLevel) .SetMethod("setAutoHideCursor", &BaseWindow::SetAutoHideCursor) #endif .SetMethod("setVibrancy", &BaseWindow::SetVibrancy) diff --git a/shell/browser/api/electron_api_base_window.h b/shell/browser/api/electron_api_base_window.h index 7613fd38d1c9e..2adb90b1c694d 100644 --- a/shell/browser/api/electron_api_base_window.h +++ b/shell/browser/api/electron_api_base_window.h @@ -142,7 +142,6 @@ class BaseWindow : public gin_helper::TrackableObject, bool IsClosable(); void SetAlwaysOnTop(bool top, gin_helper::Arguments* args); bool IsAlwaysOnTop(); - std::string GetAlwaysOnTopLevel(); void Center(); void SetPosition(int x, int y, gin_helper::Arguments* args); std::vector GetPosition(); @@ -195,6 +194,7 @@ class BaseWindow : public gin_helper::TrackableObject, virtual void SetVibrancy(v8::Isolate* isolate, v8::Local value); #if defined(OS_MAC) + std::string GetAlwaysOnTopLevel(); void SetWindowButtonVisibility(bool visible); bool GetWindowButtonVisibility() const; void SetTrafficLightPosition(const gfx::Point& position); diff --git a/shell/browser/native_window.h b/shell/browser/native_window.h index e9ec5362cef1e..0731f5c0b3274 100644 --- a/shell/browser/native_window.h +++ b/shell/browser/native_window.h @@ -129,13 +129,13 @@ class NativeWindow : public base::SupportsUserData, virtual void SetAlwaysOnTop(ui::ZOrderLevel z_order, const std::string& level = "floating", int relativeLevel = 0) = 0; - virtual std::string GetAlwaysOnTopLevel() = 0; virtual ui::ZOrderLevel GetZOrderLevel() = 0; virtual void Center() = 0; virtual void Invalidate() = 0; virtual void SetTitle(const std::string& title) = 0; virtual std::string GetTitle() = 0; #if defined(OS_MAC) + virtual std::string GetAlwaysOnTopLevel() = 0; virtual void SetActive(bool is_key) = 0; virtual bool IsActive() const = 0; #endif diff --git a/shell/browser/native_window_views.h b/shell/browser/native_window_views.h index 781ddd59ecf87..f48ce5c2739cc 100644 --- a/shell/browser/native_window_views.h +++ b/shell/browser/native_window_views.h @@ -93,7 +93,6 @@ class NativeWindowViews : public NativeWindow, void SetAlwaysOnTop(ui::ZOrderLevel z_order, const std::string& level, int relativeLevel) override; - std::string GetAlwaysOnTopLevel() override; ui::ZOrderLevel GetZOrderLevel() override; void Center() override; void Invalidate() override; diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index 8fa32c1d55f50..11af618723729 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -1471,7 +1471,7 @@ describe('BrowserWindow module', () => { expect(w.isAlwaysOnTop()).to.be.false(); expect(c.isAlwaysOnTop()).to.be.true('child is not always on top'); - expect((c as any).getAlwaysOnTopLevel()).to.equal('screen-saver'); + expect((c as any)._getAlwaysOnTopLevel()).to.equal('screen-saver'); }); });