From 7a0a87a6f33d2227d6c396be057bb8a37d0c6e2b Mon Sep 17 00:00:00 2001 From: leethomas Date: Mon, 23 Jan 2017 20:36:09 -0800 Subject: [PATCH 1/9] implement relative window levels, closes #8153 --- atom/browser/api/atom_api_window.cc | 4 +++- atom/browser/native_window.h | 3 ++- atom/browser/native_window_mac.h | 3 ++- atom/browser/native_window_mac.mm | 11 +++++++++-- atom/browser/native_window_views.cc | 3 ++- atom/browser/native_window_views.h | 3 ++- 6 files changed, 20 insertions(+), 7 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index efbd31bc65..44126cf20d 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -512,8 +512,10 @@ bool Window::IsClosable() { void Window::SetAlwaysOnTop(bool top, mate::Arguments* args) { std::string level = "floating"; + int relativeLevel = 0; args->GetNext(&level); - window_->SetAlwaysOnTop(top, level); + args->GetNext(&relativeLevel); + window_->SetAlwaysOnTop(top, level, relativeLevel); } bool Window::IsAlwaysOnTop() { diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index 3422db0ad6..05c06dfbfa 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -119,7 +119,8 @@ class NativeWindow : public base::SupportsUserData, virtual void SetClosable(bool closable) = 0; virtual bool IsClosable() = 0; virtual void SetAlwaysOnTop(bool top, - const std::string& level = "floating") = 0; + const std::string& level = "floating", + int relativeLevel = 0) = 0; virtual bool IsAlwaysOnTop() = 0; virtual void Center() = 0; virtual void SetTitle(const std::string& title) = 0; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 759e51e362..37786be352 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -67,7 +67,8 @@ class NativeWindowMac : public NativeWindow, bool IsFullScreenable() override; void SetClosable(bool closable) override; bool IsClosable() override; - void SetAlwaysOnTop(bool top, const std::string& level) override; + void SetAlwaysOnTop(bool top, const std::string& level, + int relativeLevel) override; bool IsAlwaysOnTop() override; void Center() override; void SetTitle(const std::string& title) override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 037d0be487..6962611a1c 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -1056,8 +1056,9 @@ bool NativeWindowMac::IsClosable() { return [window_ styleMask] & NSClosableWindowMask; } -void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level) { +void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level, int relativeLevel) { int windowLevel = NSNormalWindowLevel; + if (top) { if (level == "floating") { windowLevel = NSFloatingWindowLevel; @@ -1078,7 +1079,13 @@ void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level) { windowLevel = NSDockWindowLevel; } } - [window_ setLevel:windowLevel]; + + NSInteger newLevel = windowLevel + relativeLevel; + if (newLevel >= 0 && newLevel < CGWindowLevelForKey(kCGMaximumWindowLevelKey)) { + [window_ setLevel:newLevel]; + } else { + [window_ setLevel:windowLevel]; + } } bool NativeWindowMac::IsAlwaysOnTop() { diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index d5dbbaa4f7..e13608f466 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -682,7 +682,8 @@ bool NativeWindowViews::IsClosable() { #endif } -void NativeWindowViews::SetAlwaysOnTop(bool top, const std::string& level) { +void NativeWindowViews::SetAlwaysOnTop(bool top, const std::string& level, + int relativeLevel) { window_->SetAlwaysOnTop(top); } diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index 110a4e9d43..f70904b25d 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -86,7 +86,8 @@ class NativeWindowViews : public NativeWindow, bool IsFullScreenable() override; void SetClosable(bool closable) override; bool IsClosable() override; - void SetAlwaysOnTop(bool top, const std::string& level) override; + void SetAlwaysOnTop(bool top, const std::string& level, + int relativeLevel) override; bool IsAlwaysOnTop() override; void Center() override; void SetTitle(const std::string& title) override; From fb741285c673b856ee05e2e419ea6a2effd97c12 Mon Sep 17 00:00:00 2001 From: leethomas Date: Mon, 23 Jan 2017 21:28:12 -0800 Subject: [PATCH 2/9] update documentation --- docs/api/browser-window.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 306a365a58..38fe60ed32 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -846,13 +846,16 @@ Returns `Boolean` - Whether the window can be manually closed by user. On Linux always returns `true`. -#### `win.setAlwaysOnTop(flag[, level])` +#### `win.setAlwaysOnTop(flag[, level][, relativeLevel])` * `flag` Boolean * `level` String (optional) _macOS_ - Values include `normal`, `floating`, `torn-off-menu`, `modal-panel`, `main-menu`, `status`, `pop-up-menu`, `screen-saver`, and ~~`dock`~~ (Deprecated). The default is `floating`. See the [macOS docs][window-levels] for more details. +* `relativeLevel` Integer (optional) _macOS_ - The number of layers higher to set +this window relative to the given `level`. The default is `0`. Note that Apple +discourages setting levels higher than 1 above `screen-saver`. Sets whether the window should show always on top of other windows. After setting this, the window is still a normal window, not a toolbox window which From 1f5518b91efad274e75000313727f936c4fbfbe8 Mon Sep 17 00:00:00 2001 From: leethomas Date: Tue, 24 Jan 2017 20:08:08 -0800 Subject: [PATCH 3/9] throw an error for out of bounds window levels --- atom/browser/api/atom_api_window.cc | 9 ++++++++- atom/browser/native_window.h | 3 ++- atom/browser/native_window_mac.h | 2 +- atom/browser/native_window_mac.mm | 10 +++++++--- atom/browser/native_window_views.cc | 2 +- atom/browser/native_window_views.h | 2 +- 6 files changed, 20 insertions(+), 8 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 44126cf20d..2f97a88faa 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -513,9 +513,16 @@ bool Window::IsClosable() { void Window::SetAlwaysOnTop(bool top, mate::Arguments* args) { std::string level = "floating"; int relativeLevel = 0; + std::string error; + args->GetNext(&level); args->GetNext(&relativeLevel); - window_->SetAlwaysOnTop(top, level, relativeLevel); + + window_->SetAlwaysOnTop(top, level, relativeLevel, &error); + + if (!error.empty()) { + args->ThrowError(error); + } } bool Window::IsAlwaysOnTop() { diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index 05c06dfbfa..6f2d525469 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -120,7 +120,8 @@ class NativeWindow : public base::SupportsUserData, virtual bool IsClosable() = 0; virtual void SetAlwaysOnTop(bool top, const std::string& level = "floating", - int relativeLevel = 0) = 0; + int relativeLevel = 0, + std::string* error = nullptr) = 0; virtual bool IsAlwaysOnTop() = 0; virtual void Center() = 0; virtual void SetTitle(const std::string& title) = 0; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 37786be352..186051200a 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -68,7 +68,7 @@ class NativeWindowMac : public NativeWindow, void SetClosable(bool closable) override; bool IsClosable() override; void SetAlwaysOnTop(bool top, const std::string& level, - int relativeLevel) override; + int relativeLevel, std::string* error) override; bool IsAlwaysOnTop() override; void Center() override; void SetTitle(const std::string& title) override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 6962611a1c..589accfedb 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -1056,8 +1056,10 @@ bool NativeWindowMac::IsClosable() { return [window_ styleMask] & NSClosableWindowMask; } -void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level, int relativeLevel) { +void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level, int relativeLevel, + std::string* error) { int windowLevel = NSNormalWindowLevel; + CGWindowLevel maxWindowLevel = CGWindowLevelForKey(kCGMaximumWindowLevelKey); if (top) { if (level == "floating") { @@ -1081,10 +1083,12 @@ void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level, int rel } NSInteger newLevel = windowLevel + relativeLevel; - if (newLevel >= 0 && newLevel < CGWindowLevelForKey(kCGMaximumWindowLevelKey)) { + + if (newLevel >= 0 && newLevel <= maxWindowLevel) { [window_ setLevel:newLevel]; } else { - [window_ setLevel:windowLevel]; + *error = std::string([[NSString stringWithFormat: + @"relativeLevel must be between 0 and %d", maxWindowLevel] UTF8String]); } } diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index e13608f466..16a10b3532 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -683,7 +683,7 @@ bool NativeWindowViews::IsClosable() { } void NativeWindowViews::SetAlwaysOnTop(bool top, const std::string& level, - int relativeLevel) { + int relativeLevel, std::string* error) { window_->SetAlwaysOnTop(top); } diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index f70904b25d..a6a43b31d9 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -87,7 +87,7 @@ class NativeWindowViews : public NativeWindow, void SetClosable(bool closable) override; bool IsClosable() override; void SetAlwaysOnTop(bool top, const std::string& level, - int relativeLevel) override; + int relativeLevel, std::string& error) override; bool IsAlwaysOnTop() override; void Center() override; void SetTitle(const std::string& title) override; From 1234b61fa6d3ee3c6245c5d5dee54dfd2e8654e5 Mon Sep 17 00:00:00 2001 From: leethomas Date: Wed, 25 Jan 2017 19:39:57 -0800 Subject: [PATCH 4/9] spec to ensure an error is thrown when relativeLevel is set out of bounds --- spec/api-browser-window-spec.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index a07f301c8b..4e13a14fca 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -513,6 +513,18 @@ describe('BrowserWindow module', function () { w.setAlwaysOnTop(true) assert.equal(w.isAlwaysOnTop(), true) }) + + it('raises an error when relativeLevel is out of bounds', function() { + if (process.platform !== 'darwin') return; + + assert.throws(function() { + w.setAlwaysOnTop(true, '', -1) + }) + + assert.throws(function() { + w.setAlwaysOnTop(true, '', 2147483632) + }) + }) }) describe('BrowserWindow.setAutoHideCursor(autoHide)', () => { From 9e189b9d2d434470fc69a43a35e5d4cec0028fd2 Mon Sep 17 00:00:00 2001 From: leethomas Date: Thu, 26 Jan 2017 07:14:47 -0800 Subject: [PATCH 5/9] fix js lint errors --- spec/api-browser-window-spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 4e13a14fca..2d52ec27e5 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -514,14 +514,14 @@ describe('BrowserWindow module', function () { assert.equal(w.isAlwaysOnTop(), true) }) - it('raises an error when relativeLevel is out of bounds', function() { - if (process.platform !== 'darwin') return; + it('raises an error when relativeLevel is out of bounds', function () { + if (process.platform !== 'darwin') return - assert.throws(function() { + assert.throws(function () { w.setAlwaysOnTop(true, '', -1) }) - assert.throws(function() { + assert.throws(function () { w.setAlwaysOnTop(true, '', 2147483632) }) }) From b67d515a76832af6234aa011abd169cf62293bbd Mon Sep 17 00:00:00 2001 From: leethomas Date: Thu, 26 Jan 2017 07:24:20 -0800 Subject: [PATCH 6/9] fix signature for SetAlwaysOnTop in native window views --- atom/browser/native_window_views.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index a6a43b31d9..7d983b89a5 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -87,7 +87,7 @@ class NativeWindowViews : public NativeWindow, void SetClosable(bool closable) override; bool IsClosable() override; void SetAlwaysOnTop(bool top, const std::string& level, - int relativeLevel, std::string& error) override; + int relativeLevel, std::string* error) override; bool IsAlwaysOnTop() override; void Center() override; void SetTitle(const std::string& title) override; From edbb786fb44223f777cb015c12564f6ffa0e2304 Mon Sep 17 00:00:00 2001 From: leethomas Date: Thu, 26 Jan 2017 19:12:10 -0800 Subject: [PATCH 7/9] use kCGMinimumWindowLevelKey to get the minimum level --- atom/browser/native_window_mac.mm | 7 ++++--- spec/api-browser-window-spec.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 589accfedb..57f8eae1af 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -1060,6 +1060,7 @@ void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level, int rel std::string* error) { int windowLevel = NSNormalWindowLevel; CGWindowLevel maxWindowLevel = CGWindowLevelForKey(kCGMaximumWindowLevelKey); + CGWindowLevel minWindowLevel = CGWindowLevelForKey(kCGMinimumWindowLevelKey); if (top) { if (level == "floating") { @@ -1083,12 +1084,12 @@ void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level, int rel } NSInteger newLevel = windowLevel + relativeLevel; - - if (newLevel >= 0 && newLevel <= maxWindowLevel) { + if (newLevel >= minWindowLevel && newLevel <= maxWindowLevel) { [window_ setLevel:newLevel]; } else { *error = std::string([[NSString stringWithFormat: - @"relativeLevel must be between 0 and %d", maxWindowLevel] UTF8String]); + @"relativeLevel must be between %d and %d", minWindowLevel, + maxWindowLevel] UTF8String]); } } diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 2d52ec27e5..6666a41545 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -518,7 +518,7 @@ describe('BrowserWindow module', function () { if (process.platform !== 'darwin') return assert.throws(function () { - w.setAlwaysOnTop(true, '', -1) + w.setAlwaysOnTop(true, '', -2147483644) }) assert.throws(function () { From 2cacaa443a857daea8cde0d12fd1982d42145214 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 30 Jan 2017 15:27:51 -0800 Subject: [PATCH 8/9] Indent relativeLevel docs --- docs/api/browser-window.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 38fe60ed32..1dbb92508a 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -854,8 +854,8 @@ On Linux always returns `true`. `screen-saver`, and ~~`dock`~~ (Deprecated). The default is `floating`. See the [macOS docs][window-levels] for more details. * `relativeLevel` Integer (optional) _macOS_ - The number of layers higher to set -this window relative to the given `level`. The default is `0`. Note that Apple -discourages setting levels higher than 1 above `screen-saver`. + this window relative to the given `level`. The default is `0`. Note that Apple + discourages setting levels higher than 1 above `screen-saver`. Sets whether the window should show always on top of other windows. After setting this, the window is still a normal window, not a toolbox window which From 9b2b6da3a3e159df459c19c7c39bb3410a7212c2 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 30 Jan 2017 15:32:05 -0800 Subject: [PATCH 9/9] Wrap params at 80 characters --- atom/browser/native_window_mac.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 57f8eae1af..430fe48ee9 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -1056,8 +1056,8 @@ bool NativeWindowMac::IsClosable() { return [window_ styleMask] & NSClosableWindowMask; } -void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level, int relativeLevel, - std::string* error) { +void NativeWindowMac::SetAlwaysOnTop(bool top, const std::string& level, + int relativeLevel, std::string* error) { int windowLevel = NSNormalWindowLevel; CGWindowLevel maxWindowLevel = CGWindowLevelForKey(kCGMaximumWindowLevelKey); CGWindowLevel minWindowLevel = CGWindowLevelForKey(kCGMinimumWindowLevelKey);