From fd42e3dc84ce1fe488e8e25172f58c050b485f13 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 17 Jun 2016 15:28:43 +0900 Subject: [PATCH 01/23] mac: Add win.setParentWindow(parent) API --- atom/browser/api/atom_api_window.cc | 5 +++++ atom/browser/api/atom_api_window.h | 1 + atom/browser/native_window.h | 1 + atom/browser/native_window_mac.h | 1 + atom/browser/native_window_mac.mm | 10 ++++++++++ atom/browser/native_window_views.cc | 3 +++ atom/browser/native_window_views.h | 1 + 7 files changed, 22 insertions(+) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 0d72a74f7d5..90d4a086091 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -650,6 +650,10 @@ void Window::SetAspectRatio(double aspect_ratio, mate::Arguments* args) { window_->SetAspectRatio(aspect_ratio, extra_size); } +void Window::SetParentWindow(NativeWindow* parent) { + window_->SetParentWindow(parent); +} + v8::Local Window::GetNativeWindowHandle() { gfx::AcceleratedWidget handle = window_->GetAcceleratedWidget(); return ToBuffer( @@ -697,6 +701,7 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("setFullScreen", &Window::SetFullScreen) .SetMethod("isFullScreen", &Window::IsFullscreen) .SetMethod("setAspectRatio", &Window::SetAspectRatio) + .SetMethod("setParentWindow", &Window::SetParentWindow) .SetMethod("getNativeWindowHandle", &Window::GetNativeWindowHandle) .SetMethod("getBounds", &Window::GetBounds) .SetMethod("setBounds", &Window::SetBounds) diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index 9d3f7ef2402..fb78b6deb55 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -159,6 +159,7 @@ class Window : public mate::TrackableObject, void SetMenuBarVisibility(bool visible); bool IsMenuBarVisible(); void SetAspectRatio(double aspect_ratio, mate::Arguments* args); + void SetParentWindow(NativeWindow* parent); v8::Local GetNativeWindowHandle(); #if defined(OS_WIN) diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index a644682e345..8dd3dc5f334 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -158,6 +158,7 @@ class NativeWindow : public base::SupportsUserData, virtual void SetFocusable(bool focusable); virtual void SetMenu(ui::MenuModel* menu); virtual bool HasModalDialog(); + virtual void SetParentWindow(NativeWindow* parent) = 0; virtual gfx::NativeWindow GetNativeWindow() = 0; virtual gfx::AcceleratedWidget GetAcceleratedWidget() = 0; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 899043fd6ac..2e0b769d2ad 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -78,6 +78,7 @@ class NativeWindowMac : public NativeWindow { bool IsDocumentEdited() override; void SetIgnoreMouseEvents(bool ignore) override; bool HasModalDialog() override; + void SetParentWindow(NativeWindow* parent) override; gfx::NativeWindow GetNativeWindow() override; gfx::AcceleratedWidget GetAcceleratedWidget() override; void SetProgressBar(double progress) override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index e15ff0c6cee..b7e71f749ec 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -911,6 +911,16 @@ bool NativeWindowMac::HasModalDialog() { return [window_ attachedSheet] != nil; } +void NativeWindowMac::SetParentWindow(NativeWindow* parent) { + // Remove current parent window. + if ([window_ parentWindow]) + [[window_ parentWindow] removeChildWindow:window_]; + + // Set new current window. + if (parent) + [parent->GetNativeWindow() addChildWindow:window_ ordered:NSWindowAbove]; +} + gfx::NativeWindow NativeWindowMac::GetNativeWindow() { return window_; } diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 021ceba005e..ca93bb08a46 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -772,6 +772,9 @@ void NativeWindowViews::SetMenu(ui::MenuModel* menu_model) { Layout(); } +void NativeWindowViews::SetParentWindow(NativeWindow* parent) { +} + gfx::NativeWindow NativeWindowViews::GetNativeWindow() { return window_->GetNativeWindow(); } diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index 53774203742..2c63270815e 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -94,6 +94,7 @@ class NativeWindowViews : public NativeWindow, void SetIgnoreMouseEvents(bool ignore) override; void SetFocusable(bool focusable) override; void SetMenu(ui::MenuModel* menu_model) override; + void SetParentWindow(NativeWindow* parent) override; gfx::NativeWindow GetNativeWindow() override; void SetOverlayIcon(const gfx::Image& overlay, const std::string& description) override; From fb7e7b315cc8c34a285b405c505e6cb94596af6f Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 17 Jun 2016 15:47:05 +0900 Subject: [PATCH 02/23] mac: Close all child windows before closing current window --- atom/browser/native_window_mac.mm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index b7e71f749ec..8dd6457f87f 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -604,6 +604,10 @@ void NativeWindowMac::Close() { } void NativeWindowMac::CloseImmediately() { + // Close all child windows before closing this window. + for (NSWindow* child in [window_ childWindows]) + [child close]; + [window_ close]; } From 22513efd55e3ef8fe8ce59e2a5db90e2e3b85b28 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 17 Jun 2016 16:09:43 +0900 Subject: [PATCH 03/23] Add win.getParentWindow() API --- atom/browser/api/atom_api_window.cc | 20 ++++++++++++++++++-- atom/browser/api/atom_api_window.h | 4 +++- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 90d4a086091..3e470032803 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -650,8 +650,23 @@ void Window::SetAspectRatio(double aspect_ratio, mate::Arguments* args) { window_->SetAspectRatio(aspect_ratio, extra_size); } -void Window::SetParentWindow(NativeWindow* parent) { - window_->SetParentWindow(parent); +void Window::SetParentWindow(mate::Arguments* args) { + v8::Local value; + NativeWindow* parent; + if (args->GetNext(&value) && + mate::ConvertFromV8(isolate(), value, &parent)) { + parent_window_.Reset(isolate(), value); + window_->SetParentWindow(parent); + } else { + args->ThrowError("Must pass BrowserWindow instance or null"); + } +} + +v8::Local Window::GetParentWindow() { + if (parent_window_.IsEmpty()) + return v8::Null(isolate()); + else + return v8::Local::New(isolate(), parent_window_); } v8::Local Window::GetNativeWindowHandle() { @@ -702,6 +717,7 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("isFullScreen", &Window::IsFullscreen) .SetMethod("setAspectRatio", &Window::SetAspectRatio) .SetMethod("setParentWindow", &Window::SetParentWindow) + .SetMethod("getParentWindow", &Window::GetParentWindow) .SetMethod("getNativeWindowHandle", &Window::GetNativeWindowHandle) .SetMethod("getBounds", &Window::GetBounds) .SetMethod("setBounds", &Window::SetBounds) diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index fb78b6deb55..007854897ff 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -159,7 +159,8 @@ class Window : public mate::TrackableObject, void SetMenuBarVisibility(bool visible); bool IsMenuBarVisible(); void SetAspectRatio(double aspect_ratio, mate::Arguments* args); - void SetParentWindow(NativeWindow* parent); + void SetParentWindow(mate::Arguments* args); + v8::Local GetParentWindow(); v8::Local GetNativeWindowHandle(); #if defined(OS_WIN) @@ -189,6 +190,7 @@ class Window : public mate::TrackableObject, v8::Global web_contents_; v8::Global menu_; + v8::Global parent_window_; api::WebContents* api_web_contents_; From 6cef29e4eee13cf9096a00b79fa5497a0abff877 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 17 Jun 2016 16:57:03 +0900 Subject: [PATCH 04/23] Add win.getChildWindows() API --- atom/browser/api/atom_api_window.cc | 32 +++++++++++++++++++++++------ atom/browser/api/atom_api_window.h | 10 +++++++-- atom/common/key_weak_map.h | 4 ---- 3 files changed, 34 insertions(+), 12 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 3e470032803..3e1bb6a3464 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -136,6 +136,8 @@ void Window::OnWindowClosed() { Emit("closed"); + RemoveFromParentChildWindows(); + // Destroy the native class when window is closed. base::MessageLoop::current()->PostTask(FROM_HERE, GetDestroyClosure()); } @@ -650,13 +652,17 @@ void Window::SetAspectRatio(double aspect_ratio, mate::Arguments* args) { window_->SetAspectRatio(aspect_ratio, extra_size); } -void Window::SetParentWindow(mate::Arguments* args) { - v8::Local value; - NativeWindow* parent; - if (args->GetNext(&value) && - mate::ConvertFromV8(isolate(), value, &parent)) { +void Window::SetParentWindow(v8::Local value, + mate::Arguments* args) { + mate::Handle parent; + if (value->IsNull()) { + RemoveFromParentChildWindows(); + parent_window_.Reset(); + window_->SetParentWindow(nullptr); + } else if (mate::ConvertFromV8(isolate(), value, &parent)) { parent_window_.Reset(isolate(), value); - window_->SetParentWindow(parent); + window_->SetParentWindow(parent->window_.get()); + parent->child_windows_.Set(isolate(), ID(), GetWrapper()); } else { args->ThrowError("Must pass BrowserWindow instance or null"); } @@ -669,6 +675,10 @@ v8::Local Window::GetParentWindow() { return v8::Local::New(isolate(), parent_window_); } +std::vector> Window::GetChildWindows() { + return child_windows_.Values(isolate()); +} + v8::Local Window::GetNativeWindowHandle() { gfx::AcceleratedWidget handle = window_->GetAcceleratedWidget(); return ToBuffer( @@ -694,6 +704,15 @@ v8::Local Window::WebContents(v8::Isolate* isolate) { return v8::Local::New(isolate, web_contents_); } +void Window::RemoveFromParentChildWindows() { + if (parent_window_.IsEmpty()) + return; + + mate::Handle parent; + if (mate::ConvertFromV8(isolate(), GetParentWindow(), &parent)) + parent->child_windows_.Remove(ID()); +} + // static void Window::BuildPrototype(v8::Isolate* isolate, v8::Local prototype) { @@ -718,6 +737,7 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("setAspectRatio", &Window::SetAspectRatio) .SetMethod("setParentWindow", &Window::SetParentWindow) .SetMethod("getParentWindow", &Window::GetParentWindow) + .SetMethod("getChildWindows", &Window::GetChildWindows) .SetMethod("getNativeWindowHandle", &Window::GetNativeWindowHandle) .SetMethod("getBounds", &Window::GetBounds) .SetMethod("setBounds", &Window::SetBounds) diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index 007854897ff..0cc238afb07 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -6,15 +6,16 @@ #define ATOM_BROWSER_API_ATOM_API_WINDOW_H_ #include +#include #include #include -#include "base/memory/scoped_ptr.h" #include "ui/gfx/image/image.h" #include "atom/browser/api/trackable_object.h" #include "atom/browser/native_window.h" #include "atom/browser/native_window_observer.h" #include "atom/common/api/atom_api_native_image.h" +#include "atom/common/key_weak_map.h" #include "native_mate/handle.h" class GURL; @@ -159,8 +160,9 @@ class Window : public mate::TrackableObject, void SetMenuBarVisibility(bool visible); bool IsMenuBarVisible(); void SetAspectRatio(double aspect_ratio, mate::Arguments* args); - void SetParentWindow(mate::Arguments* args); + void SetParentWindow(v8::Local value, mate::Arguments* args); v8::Local GetParentWindow(); + std::vector> GetChildWindows(); v8::Local GetNativeWindowHandle(); #if defined(OS_WIN) @@ -183,6 +185,9 @@ class Window : public mate::TrackableObject, int32_t ID() const; v8::Local WebContents(v8::Isolate* isolate); + // Helpers. + void RemoveFromParentChildWindows(); + #if defined(OS_WIN) typedef std::map MessageCallbackMap; MessageCallbackMap messages_callback_map_; @@ -191,6 +196,7 @@ class Window : public mate::TrackableObject, v8::Global web_contents_; v8::Global menu_; v8::Global parent_window_; + KeyWeakMap child_windows_; api::WebContents* api_web_contents_; diff --git a/atom/common/key_weak_map.h b/atom/common/key_weak_map.h index bce34bfe0f6..9c66785c41d 100644 --- a/atom/common/key_weak_map.h +++ b/atom/common/key_weak_map.h @@ -14,10 +14,6 @@ namespace atom { -namespace internal { - -} // namespace internal - // Like ES6's WeakMap, but the key is Integer and the value is Weak Pointer. template class KeyWeakMap { From 214dd9716517031365ece671c95a450e174e5a6e Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 17 Jun 2016 16:57:18 +0900 Subject: [PATCH 05/23] spec: Test cases for parent window --- spec/api-browser-window-spec.js | 42 +++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 598a3470f75..6579da2b468 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -836,6 +836,48 @@ describe('browser-window module', function () { }) }) + describe('parent window', function () { + let c = null + + beforeEach(function () { + if (c != null) c.destroy() + c = new BrowserWindow({show: false}) + }) + + afterEach(function () { + if (c != null) c.destroy() + c = null + }) + + describe('win.setParentWindow(parent)', function () { + it('sets parent window', function () { + assert.equal(w.getParentWindow(), null) + assert.equal(c.getParentWindow(), null) + c.setParentWindow(w) + assert.equal(c.getParentWindow(), w) + c.setParentWindow(null) + assert.equal(c.getParentWindow(), null) + }) + + it('adds window to child windows of parent', function () { + assert.deepEqual(w.getChildWindows(), []) + c.setParentWindow(w) + assert.deepEqual(w.getChildWindows(), [c]) + c.setParentWindow(null) + assert.deepEqual(w.getChildWindows(), []) + }) + + it('removes from child windows of parent when window is closed', function (done) { + c.once('closed', () => { + assert.deepEqual(w.getChildWindows(), []) + done() + }) + c.setParentWindow(w) + c.close() + }) + }) + }) + describe('window.webContents.send(channel, args...)', function () { it('throws an error when the channel is missing', function () { assert.throws(function () { From 1a4b4a65c96ece6225edddee28560d372b87c9ed Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 17 Jun 2016 17:38:44 +0900 Subject: [PATCH 06/23] mac: Add win.disable/enable/isEnabled() API --- atom/browser/api/atom_api_window.cc | 15 +++++++++++ atom/browser/api/atom_api_window.h | 3 +++ atom/browser/native_window.h | 3 +++ atom/browser/native_window_mac.h | 3 +++ atom/browser/native_window_mac.mm | 39 +++++++++++++++++++++++++++++ atom/browser/native_window_views.cc | 10 ++++++++ atom/browser/native_window_views.h | 3 +++ 7 files changed, 76 insertions(+) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 3e1bb6a3464..30427d536be 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -293,6 +293,18 @@ bool Window::IsVisible() { return window_->IsVisible(); } +void Window::Disable() { + window_->Disable(); +} + +void Window::Enable() { + window_->Enable(); +} + +bool Window::IsEnabled() { + return window_->IsEnabled(); +} + void Window::Maximize() { window_->Maximize(); } @@ -726,6 +738,9 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("showInactive", &Window::ShowInactive) .SetMethod("hide", &Window::Hide) .SetMethod("isVisible", &Window::IsVisible) + .SetMethod("enable", &Window::Enable) + .SetMethod("disable", &Window::Disable) + .SetMethod("isEnabled", &Window::IsEnabled) .SetMethod("maximize", &Window::Maximize) .SetMethod("unmaximize", &Window::Unmaximize) .SetMethod("isMaximized", &Window::IsMaximized) diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index 0cc238afb07..3e53cbf65ce 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -95,6 +95,9 @@ class Window : public mate::TrackableObject, void ShowInactive(); void Hide(); bool IsVisible(); + void Disable(); + void Enable(); + bool IsEnabled(); void Maximize(); void Unmaximize(); bool IsMaximized(); diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index 8dd3dc5f334..a2adb5505c5 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -97,6 +97,9 @@ class NativeWindow : public base::SupportsUserData, virtual void ShowInactive() = 0; virtual void Hide() = 0; virtual bool IsVisible() = 0; + virtual void Disable() = 0; + virtual void Enable() = 0; + virtual bool IsEnabled() = 0; virtual void Maximize() = 0; virtual void Unmaximize() = 0; virtual bool IsMaximized() = 0; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 2e0b769d2ad..0a783c350e1 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -34,6 +34,9 @@ class NativeWindowMac : public NativeWindow { void ShowInactive() override; void Hide() override; bool IsVisible() override; + void Disable() override; + void Enable() override; + bool IsEnabled() override; void Maximize() override; void Unmaximize() override; bool IsMaximized() override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 8dd6457f87f..bad2fc7f243 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -279,6 +279,7 @@ bool ScopedDisableResize::disable_resize_ = false; @property BOOL acceptsFirstMouse; @property BOOL disableAutoHideCursor; @property BOOL disableKeyOrMainWindow; +@property BOOL disableMouseEvents; - (void)setShell:(atom::NativeWindowMac*)shell; - (void)setEnableLargerThanScreen:(bool)enable; @@ -348,6 +349,29 @@ bool ScopedDisableResize::disable_resize_ = false; return !self.disableKeyOrMainWindow; } +- (void)sendEvent:(NSEvent*)event { + // Drop all mouse events. + if (self.disableMouseEvents) { + switch([event type]) { + case NSLeftMouseUp: + case NSLeftMouseDown: + case NSRightMouseDown: + case NSRightMouseUp: + case NSOtherMouseUp: + case NSLeftMouseDragged: + case NSRightMouseDragged: + case NSOtherMouseDragged: + case NSMouseMoved: + case NSScrollWheel: + return; + default: + break; + } + } + + [super sendEvent:event]; +} + @end @interface ControlRegionView : NSView @@ -496,6 +520,7 @@ NativeWindowMac::NativeWindowMac( backing:NSBackingStoreBuffered defer:YES]); [window_ setShell:this]; + [window_ setDisableMouseEvents:NO]; [window_ setEnableLargerThanScreen:enable_larger_than_screen()]; window_delegate_.reset([[AtomNSWindowDelegate alloc] initWithShell:this]); @@ -647,6 +672,20 @@ bool NativeWindowMac::IsVisible() { return [window_ isVisible]; } +void NativeWindowMac::Disable() { + [window_ setDisableKeyOrMainWindow:YES]; + [window_ setDisableMouseEvents:YES]; +} + +void NativeWindowMac::Enable() { + [window_ setDisableKeyOrMainWindow:NO]; + [window_ setDisableMouseEvents:NO]; +} + +bool NativeWindowMac::IsEnabled() { + return ![window_ disableMouseEvents]; +} + void NativeWindowMac::Maximize() { if (IsMaximized()) return; diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index ca93bb08a46..b27f08aa233 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -373,6 +373,16 @@ bool NativeWindowViews::IsVisible() { return window_->IsVisible(); } +void NativeWindowViews::Disable() { +} + +void NativeWindowViews::Enable() { +} + +bool NativeWindowViews::IsEnabled() { + return true; +} + void NativeWindowViews::Maximize() { if (IsVisible()) window_->Maximize(); diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index 2c63270815e..f876a8bec24 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -54,6 +54,9 @@ class NativeWindowViews : public NativeWindow, void ShowInactive() override; void Hide() override; bool IsVisible() override; + void Disable() override; + void Enable() override; + bool IsEnabled() override; void Maximize() override; void Unmaximize() override; bool IsMaximized() override; From 3f34f80433482c57c457130113ce4d52ec9a252b Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Fri, 17 Jun 2016 17:49:35 +0900 Subject: [PATCH 07/23] Add win.setModal(modal) API --- lib/browser/api/browser-window.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index 08476499566..2a85a7cb0c7 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -97,6 +97,23 @@ BrowserWindow.prototype._init = function () { }) } +BrowserWindow.prototype.setModal = function (modal) { + const parent = this.getParentWindow() + if (!parent) { + throw new Error('setModal can only be called for child window') + } + + let closeListener = () => parent.enable() + if (modal) { + parent.disable() + this.once('closed', closeListener) + this.show() + } else { + parent.enable() + this.removeListener('closed', closeListener) + } +} + BrowserWindow.getFocusedWindow = () => { for (let window of BrowserWindow.getAllWindows()) { if (window.isFocused()) return window @@ -117,7 +134,6 @@ BrowserWindow.fromDevToolsWebContents = (webContents) => { } // Helpers. - Object.assign(BrowserWindow.prototype, { loadURL (...args) { return this.webContents.loadURL.apply(this.webContents, args) From 1104dded24d36b5b213939a6270ead85c8d52d1e Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 18 Jun 2016 09:42:18 +0900 Subject: [PATCH 08/23] Handle multiple modal windows correctly --- atom/browser/api/atom_api_window.cc | 54 +++++++++++++++++++++++++---- atom/browser/api/atom_api_window.h | 14 ++++++-- atom/common/key_weak_map.h | 2 +- lib/browser/api/browser-window.js | 17 --------- 4 files changed, 59 insertions(+), 28 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 30427d536be..d1263c7e583 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -76,7 +76,9 @@ v8::Local ToBuffer(v8::Isolate* isolate, void* val, int size) { } // namespace -Window::Window(v8::Isolate* isolate, const mate::Dictionary& options) { +Window::Window(v8::Isolate* isolate, const mate::Dictionary& options) + : disable_count_(0), + is_modal_(false) { // Use options.webPreferences to create WebContents. mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate); options.Get(options::kWebPreferences, &web_preferences); @@ -294,11 +296,15 @@ bool Window::IsVisible() { } void Window::Disable() { - window_->Disable(); + ++disable_count_; + if (disable_count_ == 1) + window_->Disable(); } void Window::Enable() { - window_->Enable(); + --disable_count_; + if (disable_count_ == 0) + window_->Enable(); } bool Window::IsEnabled() { @@ -680,17 +686,43 @@ void Window::SetParentWindow(v8::Local value, } } -v8::Local Window::GetParentWindow() { +v8::Local Window::GetParentWindow() const { if (parent_window_.IsEmpty()) return v8::Null(isolate()); else return v8::Local::New(isolate(), parent_window_); } -std::vector> Window::GetChildWindows() { +std::vector> Window::GetChildWindows() const { return child_windows_.Values(isolate()); } +void Window::SetModal(bool modal, mate::Arguments* args) { + if (parent_window_.IsEmpty()) { + args->ThrowError("setModal can only be called for child window"); + return; + } + + mate::Handle parent; + if (!mate::ConvertFromV8(isolate(), GetParentWindow(), &parent)) { + args->ThrowError("Invalid parent window"); // should never happen + return; + } + + if (modal == is_modal_) + return; + + if (modal) + parent->Disable(); + else + parent->Enable(); + is_modal_ = modal; +} + +bool Window::IsModal() const { + return is_modal_; +} + v8::Local Window::GetNativeWindowHandle() { gfx::AcceleratedWidget handle = window_->GetAcceleratedWidget(); return ToBuffer( @@ -721,8 +753,14 @@ void Window::RemoveFromParentChildWindows() { return; mate::Handle parent; - if (mate::ConvertFromV8(isolate(), GetParentWindow(), &parent)) - parent->child_windows_.Remove(ID()); + if (!mate::ConvertFromV8(isolate(), GetParentWindow(), &parent)) + return; + + parent->child_windows_.Remove(ID()); + if (IsModal()) { + parent->Enable(); + is_modal_ = false; + } } // static @@ -753,6 +791,8 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("setParentWindow", &Window::SetParentWindow) .SetMethod("getParentWindow", &Window::GetParentWindow) .SetMethod("getChildWindows", &Window::GetChildWindows) + .SetMethod("setModal", &Window::SetModal) + .SetMethod("isModal", &Window::IsModal) .SetMethod("getNativeWindowHandle", &Window::GetNativeWindowHandle) .SetMethod("getBounds", &Window::GetBounds) .SetMethod("setBounds", &Window::SetBounds) diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index 3e53cbf65ce..003fa5230e9 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -164,8 +164,10 @@ class Window : public mate::TrackableObject, bool IsMenuBarVisible(); void SetAspectRatio(double aspect_ratio, mate::Arguments* args); void SetParentWindow(v8::Local value, mate::Arguments* args); - v8::Local GetParentWindow(); - std::vector> GetChildWindows(); + v8::Local GetParentWindow() const; + std::vector> GetChildWindows() const; + void SetModal(bool modal, mate::Arguments* args); + bool IsModal() const; v8::Local GetNativeWindowHandle(); #if defined(OS_WIN) @@ -188,7 +190,7 @@ class Window : public mate::TrackableObject, int32_t ID() const; v8::Local WebContents(v8::Isolate* isolate); - // Helpers. + // Remove this window from parent window's |child_windows_|. void RemoveFromParentChildWindows(); #if defined(OS_WIN) @@ -201,6 +203,12 @@ class Window : public mate::TrackableObject, v8::Global parent_window_; KeyWeakMap child_windows_; + // How many times the Disable has been called. + int disable_count_; + + // Is current window modal. + bool is_modal_; + api::WebContents* api_web_contents_; std::unique_ptr window_; diff --git a/atom/common/key_weak_map.h b/atom/common/key_weak_map.h index 9c66785c41d..009ba099c99 100644 --- a/atom/common/key_weak_map.h +++ b/atom/common/key_weak_map.h @@ -53,7 +53,7 @@ class KeyWeakMap { } // Returns all objects. - std::vector> Values(v8::Isolate* isolate) { + std::vector> Values(v8::Isolate* isolate) const { std::vector> keys; keys.reserve(map_.size()); for (const auto& iter : map_) { diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index 2a85a7cb0c7..388e396844d 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -97,23 +97,6 @@ BrowserWindow.prototype._init = function () { }) } -BrowserWindow.prototype.setModal = function (modal) { - const parent = this.getParentWindow() - if (!parent) { - throw new Error('setModal can only be called for child window') - } - - let closeListener = () => parent.enable() - if (modal) { - parent.disable() - this.once('closed', closeListener) - this.show() - } else { - parent.enable() - this.removeListener('closed', closeListener) - } -} - BrowserWindow.getFocusedWindow = () => { for (let window of BrowserWindow.getAllWindows()) { if (window.isFocused()) return window From 2c5f4aadfb609689965b9ae4921f47cedcb4cfdf Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 18 Jun 2016 09:51:37 +0900 Subject: [PATCH 09/23] spec: Test cases for win.setModal(modal) --- spec/api-browser-window-spec.js | 51 +++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 6579da2b468..5b7794ab6ea 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -876,6 +876,57 @@ describe('browser-window module', function () { c.close() }) }) + + describe('win.setModal(modal)', function () { + it('disables parent window', function () { + assert.equal(w.isEnabled(), true) + c.setParentWindow(w) + c.setModal(true) + assert.equal(w.isEnabled(), false) + }) + + it('enables parent window when closed', function (done) { + c.once('closed', () => { + assert.equal(w.isEnabled(), true) + done() + }) + c.setParentWindow(w) + c.setModal(true) + c.close() + }) + + it('enables parent window when setting not modal', function () { + assert.equal(w.isEnabled(), true) + c.setParentWindow(w) + c.setModal(true) + assert.equal(w.isEnabled(), false) + c.setModal(false) + assert.equal(w.isEnabled(), true) + }) + + it('enables parent window when removing parent', function () { + assert.equal(w.isEnabled(), true) + c.setParentWindow(w) + c.setModal(true) + assert.equal(w.isEnabled(), false) + c.setParentWindow(null) + assert.equal(w.isEnabled(), true) + }) + + it('disables parent window recursively', function () { + let c2 = new BrowserWindow({show: false}) + c.setParentWindow(w) + c.setModal(true) + c2.setParentWindow(w) + c2.setModal(true) + assert.equal(w.isEnabled(), false) + c.setModal(false) + assert.equal(w.isEnabled(), false) + c2.setModal(false) + assert.equal(w.isEnabled(), true) + c2.destroy() + }) + }) }) describe('window.webContents.send(channel, args...)', function () { From f2cbd7cb36b3461d1e8727f0e91dbe7040a10405 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sat, 18 Jun 2016 22:53:41 +0900 Subject: [PATCH 10/23] mac: Add win.beginSheet(sheet)/endSheet(sheet) API --- atom/browser/api/atom_api_window.cc | 14 ++++++++++++++ atom/browser/api/atom_api_window.h | 2 ++ atom/browser/native_window.cc | 6 ++++++ atom/browser/native_window.h | 2 ++ atom/browser/native_window_mac.h | 2 ++ atom/browser/native_window_mac.mm | 12 ++++++++++++ 6 files changed, 38 insertions(+) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index d1263c7e583..308ff31dea9 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -723,6 +723,18 @@ bool Window::IsModal() const { return is_modal_; } +void Window::BeginSheet(mate::Handle sheet, mate::Arguments* args) { + if (sheet->IsVisible()) { + args->ThrowError("Sheet window must not be visible"); + return; + } + window_->BeginSheet(sheet->window_.get()); +} + +void Window::EndSheet(mate::Handle sheet) { + window_->EndSheet(sheet->window_.get()); +} + v8::Local Window::GetNativeWindowHandle() { gfx::AcceleratedWidget handle = window_->GetAcceleratedWidget(); return ToBuffer( @@ -793,6 +805,8 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("getChildWindows", &Window::GetChildWindows) .SetMethod("setModal", &Window::SetModal) .SetMethod("isModal", &Window::IsModal) + .SetMethod("beginSheet", &Window::BeginSheet) + .SetMethod("endSheet", &Window::EndSheet) .SetMethod("getNativeWindowHandle", &Window::GetNativeWindowHandle) .SetMethod("getBounds", &Window::GetBounds) .SetMethod("setBounds", &Window::SetBounds) diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index 003fa5230e9..92b41e775d6 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -168,6 +168,8 @@ class Window : public mate::TrackableObject, std::vector> GetChildWindows() const; void SetModal(bool modal, mate::Arguments* args); bool IsModal() const; + void BeginSheet(mate::Handle sheet, mate::Arguments* args); + void EndSheet(mate::Handle sheet); v8::Local GetNativeWindowHandle(); #if defined(OS_WIN) diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 844bbefa9e2..73698b7f412 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -292,6 +292,12 @@ bool NativeWindow::HasModalDialog() { return has_dialog_attached_; } +void NativeWindow::BeginSheet(NativeWindow* sheet) { +} + +void NativeWindow::EndSheet(NativeWindow* sheet) { +} + void NativeWindow::FocusOnWebView() { web_contents()->GetRenderViewHost()->GetWidget()->Focus(); } diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index a2adb5505c5..120d399ad70 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -162,6 +162,8 @@ class NativeWindow : public base::SupportsUserData, virtual void SetMenu(ui::MenuModel* menu); virtual bool HasModalDialog(); virtual void SetParentWindow(NativeWindow* parent) = 0; + virtual void BeginSheet(NativeWindow* sheet); + virtual void EndSheet(NativeWindow* sheet); virtual gfx::NativeWindow GetNativeWindow() = 0; virtual gfx::AcceleratedWidget GetAcceleratedWidget() = 0; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 0a783c350e1..357cbd55ff3 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -82,6 +82,8 @@ class NativeWindowMac : public NativeWindow { void SetIgnoreMouseEvents(bool ignore) override; bool HasModalDialog() override; void SetParentWindow(NativeWindow* parent) override; + void BeginSheet(NativeWindow* sheet) override; + void EndSheet(NativeWindow* sheet) override; gfx::NativeWindow GetNativeWindow() override; gfx::AcceleratedWidget GetAcceleratedWidget() override; void SetProgressBar(double progress) override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index bad2fc7f243..9e9a948f94c 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -964,6 +964,18 @@ void NativeWindowMac::SetParentWindow(NativeWindow* parent) { [parent->GetNativeWindow() addChildWindow:window_ ordered:NSWindowAbove]; } +void NativeWindowMac::BeginSheet(NativeWindow* sheet) { + [window_ beginSheet:sheet->GetNativeWindow() + completionHandler:^(NSModalResponse) { + }]; +} + +void NativeWindowMac::EndSheet(NativeWindow* sheet) { + sheet->Hide(); + [window_ endSheet:sheet->GetNativeWindow()]; + sheet->CloseImmediately(); +} + gfx::NativeWindow NativeWindowMac::GetNativeWindow() { return window_; } From 473413e874132eac838471b55289a383d75de811 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 19 Jun 2016 11:19:32 +0900 Subject: [PATCH 11/23] views: Initial implementation of win.setParentWindow(parent) But it doesn't work, aparrently Chromium doesn't support changing parent window dynamically on desktop. --- atom/browser/native_window_views.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index b27f08aa233..a1081da12a1 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -29,6 +29,7 @@ #include "ui/views/window/client_view.h" #include "ui/views/widget/native_widget_private.h" #include "ui/views/widget/widget.h" +#include "ui/wm/core/window_util.h" #include "ui/wm/core/shadow_types.h" #if defined(USE_X11) @@ -783,6 +784,21 @@ void NativeWindowViews::SetMenu(ui::MenuModel* menu_model) { } void NativeWindowViews::SetParentWindow(NativeWindow* parent) { + // Should work, but does not, it seems that the views toolkit doesn't support + // reparenting on desktop. +#if defined(DEBUG) + if (parent) { + ::SetParent(GetAcceleratedWidget(), parent->GetAcceleratedWidget()); + views::Widget::ReparentNativeView(GetNativeWindow(), + parent->GetNativeWindow()); + wm::AddTransientChild(parent->GetNativeWindow(), GetNativeWindow()); + } else { + if (!GetNativeWindow()->parent()) + return; + views::Widget::ReparentNativeView(GetNativeWindow(), nullptr); + wm::RemoveTransientChild(GetNativeWindow()->parent(), GetNativeWindow()); + } +#endif } gfx::NativeWindow NativeWindowViews::GetNativeWindow() { From 85ba38202734eff92813b5ed2cd1b5c2fcdbc359 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 19 Jun 2016 12:06:08 +0900 Subject: [PATCH 12/23] Add "parent" option for BrowserWindow --- atom/browser/api/atom_api_window.cc | 22 ++++++++++++++++++++-- atom/browser/api/atom_api_window.h | 3 +++ atom/browser/native_window.h | 3 ++- atom/browser/native_window_mac.h | 3 ++- atom/browser/native_window_mac.mm | 12 +++++++++--- atom/browser/native_window_views.cc | 8 +++++--- atom/browser/native_window_views.h | 3 ++- spec/api-browser-window-spec.js | 25 +++++++++++++++++++++++++ 8 files changed, 68 insertions(+), 11 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 308ff31dea9..b72c62704ed 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -97,9 +97,16 @@ Window::Window(v8::Isolate* isolate, const mate::Dictionary& options) mate::Dictionary(isolate, web_contents->GetWrapper()).Set( "browserWindowOptions", options); + // The parent window. + mate::Handle parent; + if (options.Get("parent", &parent)) + parent_window_.Reset(isolate, parent.ToV8()); + // Creates BrowserWindow. - window_.reset(NativeWindow::Create(web_contents->managed_web_contents(), - options)); + window_.reset(NativeWindow::Create( + web_contents->managed_web_contents(), + options, + parent.IsEmpty() ? nullptr : parent->window_.get())); web_contents->SetOwnerWindow(window_.get()); window_->InitFromOptions(options); window_->AddObserver(this); @@ -122,6 +129,17 @@ Window::~Window() { base::MessageLoop::current()->DeleteSoon(FROM_HERE, window_.release()); } +void Window::AfterInit(v8::Isolate* isolate) { + mate::TrackableObject::AfterInit(isolate); + + // We can only append this window to parent window's child windows after this + // window's JS wrapper gets initialized. + mate::Handle parent; + if (!parent_window_.IsEmpty() && + mate::ConvertFromV8(isolate, GetParentWindow(), &parent)) + parent->child_windows_.Set(isolate, ID(), GetWrapper()); +} + void Window::WillCloseWindow(bool* prevent_default) { *prevent_default = Emit("close"); } diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index 92b41e775d6..e5522f958e0 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -55,6 +55,9 @@ class Window : public mate::TrackableObject, Window(v8::Isolate* isolate, const mate::Dictionary& options); ~Window() override; + // TrackableObject: + void AfterInit(v8::Isolate* isolate) override; + // NativeWindowObserver: void WillCloseWindow(bool* prevent_default) override; void OnWindowClosed() override; diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index 120d399ad70..bcfa487b975 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -81,7 +81,8 @@ class NativeWindow : public base::SupportsUserData, // managing the window's live. static NativeWindow* Create( brightray::InspectableWebContents* inspectable_web_contents, - const mate::Dictionary& options); + const mate::Dictionary& options, + NativeWindow* parent = nullptr); // Find a window from its WebContents static NativeWindow* FromWebContents(content::WebContents* web_contents); diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 357cbd55ff3..0806cfa28c3 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -22,7 +22,8 @@ namespace atom { class NativeWindowMac : public NativeWindow { public: NativeWindowMac(brightray::InspectableWebContents* inspectable_web_contents, - const mate::Dictionary& options); + const mate::Dictionary& options, + NativeWindow* parent); ~NativeWindowMac() override; // NativeWindow: diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 9e9a948f94c..90b62cf7d2d 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -453,7 +453,8 @@ namespace atom { NativeWindowMac::NativeWindowMac( brightray::InspectableWebContents* web_contents, - const mate::Dictionary& options) + const mate::Dictionary& options, + NativeWindow* parent) : NativeWindow(web_contents, options), is_kiosk_(false), attention_request_id_(0), @@ -526,6 +527,10 @@ NativeWindowMac::NativeWindowMac( window_delegate_.reset([[AtomNSWindowDelegate alloc] initWithShell:this]); [window_ setDelegate:window_delegate_]; + if (parent) { + SetParentWindow(parent); + } + if (transparent()) { // Setting the background color to clear will also hide the shadow. [window_ setBackgroundColor:[NSColor clearColor]]; @@ -1197,8 +1202,9 @@ void NativeWindowMac::SetCollectionBehavior(bool on, NSUInteger flag) { // static NativeWindow* NativeWindow::Create( brightray::InspectableWebContents* inspectable_web_contents, - const mate::Dictionary& options) { - return new NativeWindowMac(inspectable_web_contents, options); + const mate::Dictionary& options, + NativeWindow* parent) { + return new NativeWindowMac(inspectable_web_contents, options, parent); } } // namespace atom diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index a1081da12a1..4000af5ce84 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -126,7 +126,8 @@ class NativeWindowClientView : public views::ClientView { NativeWindowViews::NativeWindowViews( brightray::InspectableWebContents* web_contents, - const mate::Dictionary& options) + const mate::Dictionary& options, + NativeWindow* parent) : NativeWindow(web_contents, options), window_(new views::Widget), web_view_(inspectable_web_contents()->GetView()->GetView()), @@ -1139,8 +1140,9 @@ ui::WindowShowState NativeWindowViews::GetRestoredState() { // static NativeWindow* NativeWindow::Create( brightray::InspectableWebContents* inspectable_web_contents, - const mate::Dictionary& options) { - return new NativeWindowViews(inspectable_web_contents, options); + const mate::Dictionary& options, + NativeWindow* parent) { + return new NativeWindowViews(inspectable_web_contents, options, parent); } } // namespace atom diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index f876a8bec24..95a95120f46 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -42,7 +42,8 @@ class NativeWindowViews : public NativeWindow, public views::WidgetObserver { public: NativeWindowViews(brightray::InspectableWebContents* inspectable_web_contents, - const mate::Dictionary& options); + const mate::Dictionary& options, + NativeWindow* parent); ~NativeWindowViews() override; // NativeWindow: diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 5b7794ab6ea..109789be146 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -849,7 +849,32 @@ describe('browser-window module', function () { c = null }) + describe('parent option', function () { + beforeEach(function () { + if (c != null) c.destroy() + c = new BrowserWindow({show: false, parent: w}) + }) + + it('sets parent window', function () { + assert.equal(c.getParentWindow(), w) + }) + + it('adds window to child windows of parent', function () { + assert.deepEqual(w.getChildWindows(), [c]) + }) + + it('removes from child windows of parent when window is closed', function (done) { + c.once('closed', () => { + assert.deepEqual(w.getChildWindows(), []) + done() + }) + c.close() + }) + }) + describe('win.setParentWindow(parent)', function () { + if (process.platform !== 'darwin') return + it('sets parent window', function () { assert.equal(w.getParentWindow(), null) assert.equal(c.getParentWindow(), null) From a6c4bf098bdd45b9fc20a39e074ccac1c078fec6 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 19 Jun 2016 12:10:25 +0900 Subject: [PATCH 13/23] spec: Avoid calling setParentWindow for unrelated tests --- spec/api-browser-window-spec.js | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 109789be146..51f7d251485 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -841,7 +841,7 @@ describe('browser-window module', function () { beforeEach(function () { if (c != null) c.destroy() - c = new BrowserWindow({show: false}) + c = new BrowserWindow({show: false, parent: w}) }) afterEach(function () { @@ -850,11 +850,6 @@ describe('browser-window module', function () { }) describe('parent option', function () { - beforeEach(function () { - if (c != null) c.destroy() - c = new BrowserWindow({show: false, parent: w}) - }) - it('sets parent window', function () { assert.equal(c.getParentWindow(), w) }) @@ -875,6 +870,11 @@ describe('browser-window module', function () { describe('win.setParentWindow(parent)', function () { if (process.platform !== 'darwin') return + beforeEach(function () { + if (c != null) c.destroy() + c = new BrowserWindow({show: false}) + }) + it('sets parent window', function () { assert.equal(w.getParentWindow(), null) assert.equal(c.getParentWindow(), null) @@ -905,7 +905,6 @@ describe('browser-window module', function () { describe('win.setModal(modal)', function () { it('disables parent window', function () { assert.equal(w.isEnabled(), true) - c.setParentWindow(w) c.setModal(true) assert.equal(w.isEnabled(), false) }) @@ -915,14 +914,12 @@ describe('browser-window module', function () { assert.equal(w.isEnabled(), true) done() }) - c.setParentWindow(w) c.setModal(true) c.close() }) it('enables parent window when setting not modal', function () { assert.equal(w.isEnabled(), true) - c.setParentWindow(w) c.setModal(true) assert.equal(w.isEnabled(), false) c.setModal(false) @@ -930,8 +927,9 @@ describe('browser-window module', function () { }) it('enables parent window when removing parent', function () { + if (process.platform !== 'darwin') return + assert.equal(w.isEnabled(), true) - c.setParentWindow(w) c.setModal(true) assert.equal(w.isEnabled(), false) c.setParentWindow(null) @@ -939,10 +937,8 @@ describe('browser-window module', function () { }) it('disables parent window recursively', function () { - let c2 = new BrowserWindow({show: false}) - c.setParentWindow(w) + let c2 = new BrowserWindow({show: false, parent: w}) c.setModal(true) - c2.setParentWindow(w) c2.setModal(true) assert.equal(w.isEnabled(), false) c.setModal(false) From 4c3c4437dad5f79275b0328fe59b9c9283c84ea0 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 19 Jun 2016 12:18:48 +0900 Subject: [PATCH 14/23] win: Implement win.disable/enable/isEnabled() API --- atom/browser/native_window_views.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 4000af5ce84..803a3e21537 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -187,6 +187,9 @@ NativeWindowViews::NativeWindowViews( if (options.Get(options::kFocusable, &focusable) && !focusable) params.activatable = views::Widget::InitParams::ACTIVATABLE_NO; + if (parent) + params.parent = parent->GetNativeWindow(); + #if defined(OS_WIN) params.native_widget = new views::DesktopNativeWidgetAura(window_.get()); @@ -376,13 +379,15 @@ bool NativeWindowViews::IsVisible() { } void NativeWindowViews::Disable() { + ::EnableWindow(GetAcceleratedWidget(), FALSE); } void NativeWindowViews::Enable() { + ::EnableWindow(GetAcceleratedWidget(), TRUE); } bool NativeWindowViews::IsEnabled() { - return true; + return ::IsWindowEnabled(GetAcceleratedWidget()); } void NativeWindowViews::Maximize() { From f4bec78ccbed8a96042b74b44c3420c97631c24c Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 19 Jun 2016 15:47:27 +0900 Subject: [PATCH 15/23] linux: Implement win.setParentWindow(parent) --- atom/browser/api/atom_api_window.cc | 12 +++++++++++ atom/browser/api/atom_api_window.h | 1 + atom/browser/native_window.cc | 3 +++ atom/browser/native_window.h | 1 + atom/browser/native_window_mac.h | 1 + atom/browser/native_window_mac.mm | 7 +++--- atom/browser/native_window_observer.h | 3 +++ atom/browser/native_window_views.cc | 31 +++++++++++++++++++++++++-- atom/browser/native_window_views.h | 1 + spec/api-browser-window-spec.js | 2 +- 10 files changed, 55 insertions(+), 7 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index b72c62704ed..a3df8f9c08d 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -144,6 +144,17 @@ void Window::WillCloseWindow(bool* prevent_default) { *prevent_default = Emit("close"); } +void Window::WillDestoryNativeObject() { + // Close all child windows before closing current window. + v8::Locker locker(isolate()); + v8::HandleScope handle_scope(isolate()); + for (v8::Local value : child_windows_.Values(isolate())) { + mate::Handle child; + if (mate::ConvertFromV8(isolate(), value, &child)) + child->window_->CloseImmediately(); + } +} + void Window::OnWindowClosed() { api_web_contents_->DestroyWebContents(); @@ -734,6 +745,7 @@ void Window::SetModal(bool modal, mate::Arguments* args) { parent->Disable(); else parent->Enable(); + window_->SetModal(modal); is_modal_ = modal; } diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index e5522f958e0..238e3ad7e1f 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -60,6 +60,7 @@ class Window : public mate::TrackableObject, // NativeWindowObserver: void WillCloseWindow(bool* prevent_default) override; + void WillDestoryNativeObject() override; void OnWindowClosed() override; void OnWindowBlur() override; void OnWindowFocus() override; diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 73698b7f412..2ce6779d620 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -403,6 +403,9 @@ void NativeWindow::CloseContents(content::WebContents* source) { inspectable_web_contents_ = nullptr; Observe(nullptr); + FOR_EACH_OBSERVER(NativeWindowObserver, observers_, + WillDestoryNativeObject()); + // When the web contents is gone, close the window immediately, but the // memory will not be freed until you call delete. // In this way, it would be safe to manage windows via smart pointers. If you diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index bcfa487b975..d20146819d4 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -165,6 +165,7 @@ class NativeWindow : public base::SupportsUserData, virtual void SetParentWindow(NativeWindow* parent) = 0; virtual void BeginSheet(NativeWindow* sheet); virtual void EndSheet(NativeWindow* sheet); + virtual void SetModal(bool modal) = 0; virtual gfx::NativeWindow GetNativeWindow() = 0; virtual gfx::AcceleratedWidget GetAcceleratedWidget() = 0; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 0806cfa28c3..3061663520a 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -85,6 +85,7 @@ class NativeWindowMac : public NativeWindow { void SetParentWindow(NativeWindow* parent) override; void BeginSheet(NativeWindow* sheet) override; void EndSheet(NativeWindow* sheet) override; + void SetModal(bool modal) override; gfx::NativeWindow GetNativeWindow() override; gfx::AcceleratedWidget GetAcceleratedWidget() override; void SetProgressBar(double progress) override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 90b62cf7d2d..f68d49929ab 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -634,10 +634,6 @@ void NativeWindowMac::Close() { } void NativeWindowMac::CloseImmediately() { - // Close all child windows before closing this window. - for (NSWindow* child in [window_ childWindows]) - [child close]; - [window_ close]; } @@ -981,6 +977,9 @@ void NativeWindowMac::EndSheet(NativeWindow* sheet) { sheet->CloseImmediately(); } +void NativeWindowMac::SetModal(bool modal) { +} + gfx::NativeWindow NativeWindowMac::GetNativeWindow() { return window_; } diff --git a/atom/browser/native_window_observer.h b/atom/browser/native_window_observer.h index 73bf4362f57..16541d09090 100644 --- a/atom/browser/native_window_observer.h +++ b/atom/browser/native_window_observer.h @@ -33,6 +33,9 @@ class NativeWindowObserver { // Called when the window is gonna closed. virtual void WillCloseWindow(bool* prevent_default) {} + // Called before the native window object is going to be destroyed. + virtual void WillDestoryNativeObject() {} + // Called when the window is closed. virtual void OnWindowClosed() {} diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 803a3e21537..f5c21bed74e 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -187,10 +187,10 @@ NativeWindowViews::NativeWindowViews( if (options.Get(options::kFocusable, &focusable) && !focusable) params.activatable = views::Widget::InitParams::ACTIVATABLE_NO; +#if defined(OS_WIN) if (parent) params.parent = parent->GetNativeWindow(); -#if defined(OS_WIN) params.native_widget = new views::DesktopNativeWidgetAura(window_.get()); atom_desktop_window_tree_host_win_ = new AtomDesktopWindowTreeHostWin( @@ -248,6 +248,9 @@ NativeWindowViews::NativeWindowViews( std::string window_type; if (options.Get(options::kType, &window_type)) SetWindowType(GetAcceleratedWidget(), window_type); + + if (parent) + SetParentWindow(parent); #endif // Add web view. @@ -379,15 +382,25 @@ bool NativeWindowViews::IsVisible() { } void NativeWindowViews::Disable() { +#if defined(OS_WIN) ::EnableWindow(GetAcceleratedWidget(), FALSE); +#elif defined(USE_X11) +#endif } void NativeWindowViews::Enable() { +#if defined(OS_WIN) ::EnableWindow(GetAcceleratedWidget(), TRUE); +#elif defined(USE_X11) +#endif } bool NativeWindowViews::IsEnabled() { +#if defined(OS_WIN) return ::IsWindowEnabled(GetAcceleratedWidget()); +#else + return false; +#endif } void NativeWindowViews::Maximize() { @@ -790,9 +803,14 @@ void NativeWindowViews::SetMenu(ui::MenuModel* menu_model) { } void NativeWindowViews::SetParentWindow(NativeWindow* parent) { +#if defined(USE_X11) + XDisplay* xdisplay = gfx::GetXDisplay(); + XSetTransientForHint( + xdisplay, GetAcceleratedWidget(), + parent? parent->GetAcceleratedWidget() : DefaultRootWindow(xdisplay)); +#elif defined(OS_WIN) && defined(DEBUG) // Should work, but does not, it seems that the views toolkit doesn't support // reparenting on desktop. -#if defined(DEBUG) if (parent) { ::SetParent(GetAcceleratedWidget(), parent->GetAcceleratedWidget()); views::Widget::ReparentNativeView(GetNativeWindow(), @@ -801,12 +819,21 @@ void NativeWindowViews::SetParentWindow(NativeWindow* parent) { } else { if (!GetNativeWindow()->parent()) return; + ::SetParent(GetAcceleratedWidget(), NULL); views::Widget::ReparentNativeView(GetNativeWindow(), nullptr); wm::RemoveTransientChild(GetNativeWindow()->parent(), GetNativeWindow()); } #endif } +void NativeWindowViews::SetModal(bool modal) { +#if defined(USE_X11) + SetWMSpecState(GetAcceleratedWidget(), modal, + GetAtom("_NET_WM_STATE_MODAL")); + SetWindowType(GetAcceleratedWidget(), modal ? "dialog" : "normal"); +#endif +} + gfx::NativeWindow NativeWindowViews::GetNativeWindow() { return window_->GetNativeWindow(); } diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index 95a95120f46..b38572d9d2a 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -99,6 +99,7 @@ class NativeWindowViews : public NativeWindow, void SetFocusable(bool focusable) override; void SetMenu(ui::MenuModel* menu_model) override; void SetParentWindow(NativeWindow* parent) override; + void SetModal(bool modal) override; gfx::NativeWindow GetNativeWindow() override; void SetOverlayIcon(const gfx::Image& overlay, const std::string& description) override; diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 51f7d251485..e9813af5b75 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -868,7 +868,7 @@ describe('browser-window module', function () { }) describe('win.setParentWindow(parent)', function () { - if (process.platform !== 'darwin') return + if (process.platform === 'win32') return beforeEach(function () { if (c != null) c.destroy() From 074903ca338aa51577c4c599d586f835dbb7cac5 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 19 Jun 2016 16:55:45 +0900 Subject: [PATCH 16/23] Do not emit "unresponsive" when there is modal dialog --- atom/browser/native_window.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 2ce6779d620..6db53df28ac 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -620,7 +620,7 @@ void NativeWindow::ScheduleUnresponsiveEvent(int ms) { void NativeWindow::NotifyWindowUnresponsive() { window_unresposive_closure_.Cancel(); - if (!is_closed_ && !HasModalDialog()) + if (!is_closed_ && !HasModalDialog() && IsEnabled()) FOR_EACH_OBSERVER(NativeWindowObserver, observers_, OnRendererUnresponsive()); From 9aa7291627fd0f0bdcd5fa17f3ab1d6befff773d Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 19 Jun 2016 17:35:56 +0900 Subject: [PATCH 17/23] linux: Implement win.disable/enable/isEnabled() API --- atom/browser/native_window_views.cc | 13 ++++++++++-- atom/browser/native_window_views.h | 5 +++++ atom/browser/ui/x/event_disabler.cc | 27 ++++++++++++++++++++++++ atom/browser/ui/x/event_disabler.h | 32 +++++++++++++++++++++++++++++ filenames.gypi | 2 ++ 5 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 atom/browser/ui/x/event_disabler.cc create mode 100644 atom/browser/ui/x/event_disabler.h diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index f5c21bed74e..5cb005a92ca 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -37,6 +37,7 @@ #include "atom/browser/ui/views/global_menu_bar_x11.h" #include "atom/browser/ui/views/frameless_view.h" #include "atom/browser/ui/views/native_frame_view.h" +#include "atom/browser/ui/x/event_disabler.h" #include "atom/browser/ui/x/window_state_watcher.h" #include "atom/browser/ui/x/x_window_utils.h" #include "base/strings/string_util.h" @@ -385,6 +386,10 @@ void NativeWindowViews::Disable() { #if defined(OS_WIN) ::EnableWindow(GetAcceleratedWidget(), FALSE); #elif defined(USE_X11) + event_disabler_.reset(new EventDisabler); + views::DesktopWindowTreeHostX11* tree_host = + views::DesktopWindowTreeHostX11::GetHostForXID(GetAcceleratedWidget()); + tree_host->AddEventRewriter(event_disabler_.get()); #endif } @@ -392,14 +397,18 @@ void NativeWindowViews::Enable() { #if defined(OS_WIN) ::EnableWindow(GetAcceleratedWidget(), TRUE); #elif defined(USE_X11) + views::DesktopWindowTreeHostX11* tree_host = + views::DesktopWindowTreeHostX11::GetHostForXID(GetAcceleratedWidget()); + tree_host->RemoveEventRewriter(event_disabler_.get()); + event_disabler_.reset(); #endif } bool NativeWindowViews::IsEnabled() { #if defined(OS_WIN) return ::IsWindowEnabled(GetAcceleratedWidget()); -#else - return false; +#elif defined(USE_X11) + return !event_disabler_.get(); #endif } diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index b38572d9d2a..bdc2d36497b 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -32,6 +32,8 @@ class WindowStateWatcher; #if defined(OS_WIN) class AtomDesktopWindowTreeHostWin; +#elif defined(USE_X11) +class EventDisabler; #endif class NativeWindowViews : public NativeWindow, @@ -193,6 +195,9 @@ class NativeWindowViews : public NativeWindow, // Handles window state events. std::unique_ptr window_state_watcher_; + // To disable the mouse events. + std::unique_ptr event_disabler_; + // The "resizable" flag on Linux is implemented by setting size constraints, // we need to make sure size constraints are restored when window becomes // resizable again. diff --git a/atom/browser/ui/x/event_disabler.cc b/atom/browser/ui/x/event_disabler.cc new file mode 100644 index 00000000000..6d0e4cfeb04 --- /dev/null +++ b/atom/browser/ui/x/event_disabler.cc @@ -0,0 +1,27 @@ +// Copyright (c) 2016 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/ui/x/event_disabler.h" + +namespace atom { + +EventDisabler::EventDisabler() { +} + +EventDisabler::~EventDisabler() { +} + +ui::EventRewriteStatus EventDisabler::RewriteEvent( + const ui::Event& event, + std::unique_ptr* rewritten_event) { + return ui::EVENT_REWRITE_DISCARD; +} + +ui::EventRewriteStatus EventDisabler::NextDispatchEvent( + const ui::Event& last_event, + std::unique_ptr* new_event) { + return ui::EVENT_REWRITE_CONTINUE; +} + +} // namespace atom diff --git a/atom/browser/ui/x/event_disabler.h b/atom/browser/ui/x/event_disabler.h new file mode 100644 index 00000000000..9a6645bcdca --- /dev/null +++ b/atom/browser/ui/x/event_disabler.h @@ -0,0 +1,32 @@ +// Copyright (c) 2016 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_UI_X_EVENT_DISABLER_H_ +#define ATOM_BROWSER_UI_X_EVENT_DISABLER_H_ + +#include "base/macros.h" +#include "ui/events/event_rewriter.h" + +namespace atom { + +class EventDisabler : public ui::EventRewriter { + public: + EventDisabler(); + ~EventDisabler() override; + + // ui::EventRewriter: + ui::EventRewriteStatus RewriteEvent( + const ui::Event& event, + std::unique_ptr* rewritten_event) override; + ui::EventRewriteStatus NextDispatchEvent( + const ui::Event& last_event, + std::unique_ptr* new_event) override; + + private: + DISALLOW_COPY_AND_ASSIGN(EventDisabler); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_UI_X_EVENT_DISABLER_H_ diff --git a/filenames.gypi b/filenames.gypi index 87ff5dc9813..af7080821b4 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -286,6 +286,8 @@ 'atom/browser/ui/win/notify_icon.h', 'atom/browser/ui/win/taskbar_host.cc', 'atom/browser/ui/win/taskbar_host.h', + 'atom/browser/ui/x/event_disabler.cc', + 'atom/browser/ui/x/event_disabler.h', 'atom/browser/ui/x/window_state_watcher.cc', 'atom/browser/ui/x/window_state_watcher.h', 'atom/browser/ui/x/x_window_utils.cc', From 02acce8991e25ce1c725011791fe5020c258c4b0 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Sun, 19 Jun 2016 17:49:31 +0900 Subject: [PATCH 18/23] linux: Make modal dialog show more smoothly --- atom/browser/native_window_views.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 5cb005a92ca..0dcef4661d5 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -837,9 +837,10 @@ void NativeWindowViews::SetParentWindow(NativeWindow* parent) { void NativeWindowViews::SetModal(bool modal) { #if defined(USE_X11) + SetWindowType(GetAcceleratedWidget(), modal ? "dialog" : "normal"); + Show(); SetWMSpecState(GetAcceleratedWidget(), modal, GetAtom("_NET_WM_STATE_MODAL")); - SetWindowType(GetAcceleratedWidget(), modal ? "dialog" : "normal"); #endif } From 946d246aea3dc960aa74a41b7888279ae7a68c85 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 20 Jun 2016 11:06:48 +0900 Subject: [PATCH 19/23] docs: Parent and child windows --- docs/api/browser-window.md | 90 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 6aae7b0080a..dbeb847dfdd 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -59,6 +59,53 @@ win.loadURL('https://github.com') Note that even for apps that use `ready-to-show` event, it is still recommended to set `backgroundColor` to make app feel more native. +## Parent and child windows + +By using `parent` option, you can create child windows: + +```javascript +let top = new BrowserWindow() +let child = new BrowserWindow({parent: top}) +``` + +The `child` window will always show on top of the `top` window. + +### Modal windows + +A modal window is a child window that disables parent window, to turn a child +window into modal window, you need to call `win.setModal(true)`: + +```javascript +let child = new BrowserWindow({parent: top, show: false}) +child.loadURL('https://github.com') +child.setModal(true) +child.once('ready-to-show', () => { + child.show() +}) +``` + +### Sheets + +On macOS it is uncommon to use modal windows for tasks, a more native way is to +show window as sheet by calling `win.beginSheet(sheet)` API: + +```javascript +let child = new BrowserWindow({show: false}) +child.loadURL('https://github.com') +child.once('ready-to-show', () => { + top.beginSheet(child) +}) +``` + +### Platform notices + +* On macOS the child window will keep the relative position to parent window + when parent window moves, while on Windows and Linux child windows will not + move. +* On macOS the `setModal` has to be called before the child window shows. +* On Windows it is not supported to change parent window dynamically. +* On Linux the type of modal windows will be changed to `dialog`. + ## Class: BrowserWindow `BrowserWindow` is an @@ -116,6 +163,7 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. `true`. * `frame` Boolean - Specify `false` to create a [Frameless Window](frameless-window.md). Default is `true`. + * `parent` BrowserWindow - Parent window. * `acceptFirstMouse` Boolean - Whether the web view accepts a single mouse-down event that simultaneously activates the window. Default is `false`. @@ -532,6 +580,33 @@ Hides the window. Returns a boolean, whether the window is visible to the user. +### `win.setModal(modal)` + +* `modal` Boolean + +Sets current window as modal window. + +It can only be called for child windows. + +### `win.isModal()` + +Returns whether current window is a modal window. + +### `win.beginSheet(sheet)` _macOS_ + +* `sheet` BrowserWindow + +Shows `sheet` as a sheet for current window, when there is already a sheet +showing, this call will be queued. + +It can only be called for top-level windows. + +### `win.endSheet(sheet)` _macOS_ + +* `sheet` BrowserWindow + +Dismisses the `sheet`. + ### `win.maximize()` Maximizes the window. @@ -1017,3 +1092,18 @@ events. Changes whether the window can be focused. [blink-feature-string]: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/RuntimeEnabledFeatures.in + +### `win.setParentWindow(parent)` _Linux_ _macOS_ + +* `parent` BrowserWindow + +Sets `parent` as current window's parent window, passing `null` will turn +current window into a top-level window. + +### `win.getParentWindow()` + +Returns the parent window. + +### `win.getChildWindows()` + +Returns all child windows. From 1866dbe608ad6b11034517d23cde2bba1060a4db Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 20 Jun 2016 11:48:46 +0900 Subject: [PATCH 20/23] Move disable counter to NativeWindow from api::Window --- atom/browser/api/atom_api_window.cc | 11 +++-------- atom/browser/api/atom_api_window.h | 3 --- atom/browser/native_window.cc | 13 +++++++++++++ atom/browser/native_window.h | 8 ++++++-- atom/browser/native_window_mac.h | 3 +-- atom/browser/native_window_mac.mm | 11 +++-------- atom/browser/native_window_views.cc | 24 +++++++++--------------- atom/browser/native_window_views.h | 3 +-- 8 files changed, 36 insertions(+), 40 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index a3df8f9c08d..7068360ef91 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -77,8 +77,7 @@ v8::Local ToBuffer(v8::Isolate* isolate, void* val, int size) { Window::Window(v8::Isolate* isolate, const mate::Dictionary& options) - : disable_count_(0), - is_modal_(false) { + : is_modal_(false) { // Use options.webPreferences to create WebContents. mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate); options.Get(options::kWebPreferences, &web_preferences); @@ -325,15 +324,11 @@ bool Window::IsVisible() { } void Window::Disable() { - ++disable_count_; - if (disable_count_ == 1) - window_->Disable(); + window_->Disable(); } void Window::Enable() { - --disable_count_; - if (disable_count_ == 0) - window_->Enable(); + window_->Enable(); } bool Window::IsEnabled() { diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index 238e3ad7e1f..80785d7673f 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -209,9 +209,6 @@ class Window : public mate::TrackableObject, v8::Global parent_window_; KeyWeakMap child_windows_; - // How many times the Disable has been called. - int disable_count_; - // Is current window modal. bool is_modal_; diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 6db53df28ac..f58e026cf49 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -56,6 +56,7 @@ NativeWindow::NativeWindow( sheet_offset_x_(0.0), sheet_offset_y_(0.0), aspect_ratio_(0.0), + disable_count_(0), inspectable_web_contents_(inspectable_web_contents), weak_factory_(this) { options.Get(options::kFrame, &has_frame_); @@ -178,6 +179,18 @@ void NativeWindow::InitFromOptions(const mate::Dictionary& options) { Show(); } +void NativeWindow::Disable() { + ++disable_count_; + if (disable_count_ == 1) + SetEnabled(false); +} + +void NativeWindow::Enable() { + --disable_count_; + if (disable_count_ == 0) + SetEnabled(true); +} + void NativeWindow::SetSize(const gfx::Size& size, bool animate) { SetBounds(gfx::Rect(GetPosition(), size), animate); } diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index d20146819d4..96c10ac5464 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -98,8 +98,9 @@ class NativeWindow : public base::SupportsUserData, virtual void ShowInactive() = 0; virtual void Hide() = 0; virtual bool IsVisible() = 0; - virtual void Disable() = 0; - virtual void Enable() = 0; + virtual void Disable(); + virtual void Enable(); + virtual void SetEnabled(bool enable) = 0; // should not be used virtual bool IsEnabled() = 0; virtual void Maximize() = 0; virtual void Unmaximize() = 0; @@ -337,6 +338,9 @@ class NativeWindow : public base::SupportsUserData, double aspect_ratio_; gfx::Size aspect_ratio_extraSize_; + // How many times the Disable has been called. + int disable_count_; + // The page this window is viewing. brightray::InspectableWebContents* inspectable_web_contents_; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 3061663520a..2dcd29806d6 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -35,8 +35,7 @@ class NativeWindowMac : public NativeWindow { void ShowInactive() override; void Hide() override; bool IsVisible() override; - void Disable() override; - void Enable() override; + void SetEnabled(bool enable) override; bool IsEnabled() override; void Maximize() override; void Unmaximize() override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index f68d49929ab..b49f878eacd 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -673,14 +673,9 @@ bool NativeWindowMac::IsVisible() { return [window_ isVisible]; } -void NativeWindowMac::Disable() { - [window_ setDisableKeyOrMainWindow:YES]; - [window_ setDisableMouseEvents:YES]; -} - -void NativeWindowMac::Enable() { - [window_ setDisableKeyOrMainWindow:NO]; - [window_ setDisableMouseEvents:NO]; +void NativeWindowMac::SetEnabled(bool enable) { + [window_ setDisableKeyOrMainWindow:!enable]; + [window_ setDisableMouseEvents:!enable]; } bool NativeWindowMac::IsEnabled() { diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 0dcef4661d5..ab317f16d53 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -382,25 +382,19 @@ bool NativeWindowViews::IsVisible() { return window_->IsVisible(); } -void NativeWindowViews::Disable() { +void NativeWindowViews::SetEnabled(bool enable) { #if defined(OS_WIN) - ::EnableWindow(GetAcceleratedWidget(), FALSE); -#elif defined(USE_X11) - event_disabler_.reset(new EventDisabler); - views::DesktopWindowTreeHostX11* tree_host = - views::DesktopWindowTreeHostX11::GetHostForXID(GetAcceleratedWidget()); - tree_host->AddEventRewriter(event_disabler_.get()); -#endif -} - -void NativeWindowViews::Enable() { -#if defined(OS_WIN) - ::EnableWindow(GetAcceleratedWidget(), TRUE); + ::EnableWindow(GetAcceleratedWidget(), enable); #elif defined(USE_X11) views::DesktopWindowTreeHostX11* tree_host = views::DesktopWindowTreeHostX11::GetHostForXID(GetAcceleratedWidget()); - tree_host->RemoveEventRewriter(event_disabler_.get()); - event_disabler_.reset(); + if (enable) { + tree_host->RemoveEventRewriter(event_disabler_.get()); + event_disabler_.reset(); + } else { + event_disabler_.reset(new EventDisabler); + tree_host->AddEventRewriter(event_disabler_.get()); + } #endif } diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index bdc2d36497b..adc18a1dfde 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -57,8 +57,7 @@ class NativeWindowViews : public NativeWindow, void ShowInactive() override; void Hide() override; bool IsVisible() override; - void Disable() override; - void Enable() override; + void SetEnabled(bool enable) override; bool IsEnabled() override; void Maximize() override; void Unmaximize() override; From e33e4be257439488e9ed72f3dda654183f6e0898 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 20 Jun 2016 14:49:24 +0900 Subject: [PATCH 21/23] macOS: Use sheet window as modal window --- atom/browser/api/atom_api_window.cc | 56 +++++++---------------------- atom/browser/api/atom_api_window.h | 6 ---- atom/browser/native_window.cc | 14 +++++--- atom/browser/native_window.h | 19 ++++++---- atom/browser/native_window_mac.h | 6 ---- atom/browser/native_window_mac.mm | 45 +++++++++++++---------- atom/browser/native_window_views.cc | 3 ++ spec/api-browser-window-spec.js | 44 +++++++++-------------- 8 files changed, 81 insertions(+), 112 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 7068360ef91..bfa56dc5aa1 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -76,8 +76,7 @@ v8::Local ToBuffer(v8::Isolate* isolate, void* val, int size) { } // namespace -Window::Window(v8::Isolate* isolate, const mate::Dictionary& options) - : is_modal_(false) { +Window::Window(v8::Isolate* isolate, const mate::Dictionary& options) { // Use options.webPreferences to create WebContents. mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate); options.Get(options::kWebPreferences, &web_preferences); @@ -312,6 +311,10 @@ void Window::Show() { } void Window::ShowInactive() { + // This method doesn't make sense for modal window.. + if (IsModal()) + return; + window_->ShowInactive(); } @@ -696,6 +699,11 @@ void Window::SetAspectRatio(double aspect_ratio, mate::Arguments* args) { void Window::SetParentWindow(v8::Local value, mate::Arguments* args) { + if (IsModal()) { + args->ThrowError("Can not be called for modal window"); + return; + } + mate::Handle parent; if (value->IsNull()) { RemoveFromParentChildWindows(); @@ -721,43 +729,8 @@ std::vector> Window::GetChildWindows() const { return child_windows_.Values(isolate()); } -void Window::SetModal(bool modal, mate::Arguments* args) { - if (parent_window_.IsEmpty()) { - args->ThrowError("setModal can only be called for child window"); - return; - } - - mate::Handle parent; - if (!mate::ConvertFromV8(isolate(), GetParentWindow(), &parent)) { - args->ThrowError("Invalid parent window"); // should never happen - return; - } - - if (modal == is_modal_) - return; - - if (modal) - parent->Disable(); - else - parent->Enable(); - window_->SetModal(modal); - is_modal_ = modal; -} - bool Window::IsModal() const { - return is_modal_; -} - -void Window::BeginSheet(mate::Handle sheet, mate::Arguments* args) { - if (sheet->IsVisible()) { - args->ThrowError("Sheet window must not be visible"); - return; - } - window_->BeginSheet(sheet->window_.get()); -} - -void Window::EndSheet(mate::Handle sheet) { - window_->EndSheet(sheet->window_.get()); + return window_->is_modal(); } v8::Local Window::GetNativeWindowHandle() { @@ -794,10 +767,8 @@ void Window::RemoveFromParentChildWindows() { return; parent->child_windows_.Remove(ID()); - if (IsModal()) { + if (IsModal()) parent->Enable(); - is_modal_ = false; - } } // static @@ -828,10 +799,7 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("setParentWindow", &Window::SetParentWindow) .SetMethod("getParentWindow", &Window::GetParentWindow) .SetMethod("getChildWindows", &Window::GetChildWindows) - .SetMethod("setModal", &Window::SetModal) .SetMethod("isModal", &Window::IsModal) - .SetMethod("beginSheet", &Window::BeginSheet) - .SetMethod("endSheet", &Window::EndSheet) .SetMethod("getNativeWindowHandle", &Window::GetNativeWindowHandle) .SetMethod("getBounds", &Window::GetBounds) .SetMethod("setBounds", &Window::SetBounds) diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index 80785d7673f..cd8b6ddc42e 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -170,10 +170,7 @@ class Window : public mate::TrackableObject, void SetParentWindow(v8::Local value, mate::Arguments* args); v8::Local GetParentWindow() const; std::vector> GetChildWindows() const; - void SetModal(bool modal, mate::Arguments* args); bool IsModal() const; - void BeginSheet(mate::Handle sheet, mate::Arguments* args); - void EndSheet(mate::Handle sheet); v8::Local GetNativeWindowHandle(); #if defined(OS_WIN) @@ -209,9 +206,6 @@ class Window : public mate::TrackableObject, v8::Global parent_window_; KeyWeakMap child_windows_; - // Is current window modal. - bool is_modal_; - api::WebContents* api_web_contents_; std::unique_ptr window_; diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index f58e026cf49..42ba8d90237 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -46,7 +46,8 @@ namespace atom { NativeWindow::NativeWindow( brightray::InspectableWebContents* inspectable_web_contents, - const mate::Dictionary& options) + const mate::Dictionary& options, + NativeWindow* parent) : content::WebContentsObserver(inspectable_web_contents->GetWebContents()), has_frame_(true), transparent_(false), @@ -57,12 +58,17 @@ NativeWindow::NativeWindow( sheet_offset_y_(0.0), aspect_ratio_(0.0), disable_count_(0), + parent_(parent), + is_modal_(false), inspectable_web_contents_(inspectable_web_contents), weak_factory_(this) { options.Get(options::kFrame, &has_frame_); options.Get(options::kTransparent, &transparent_); options.Get(options::kEnableLargerThanScreen, &enable_larger_than_screen_); + if (parent) + options.Get("modal", &is_modal_); + // Tell the content module to initialize renderer widget with transparent // mode. ui::GpuSwitchingManager::SetTransparent(transparent_); @@ -305,10 +311,8 @@ bool NativeWindow::HasModalDialog() { return has_dialog_attached_; } -void NativeWindow::BeginSheet(NativeWindow* sheet) { -} - -void NativeWindow::EndSheet(NativeWindow* sheet) { +void NativeWindow::SetParentWindow(NativeWindow* parent) { + parent_ = parent; } void NativeWindow::FocusOnWebView() { diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index 96c10ac5464..ca1ffe2117e 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -100,7 +100,7 @@ class NativeWindow : public base::SupportsUserData, virtual bool IsVisible() = 0; virtual void Disable(); virtual void Enable(); - virtual void SetEnabled(bool enable) = 0; // should not be used + virtual void SetEnabled(bool enable) = 0; // internal API, should not be used virtual bool IsEnabled() = 0; virtual void Maximize() = 0; virtual void Unmaximize() = 0; @@ -163,10 +163,7 @@ class NativeWindow : public base::SupportsUserData, virtual void SetFocusable(bool focusable); virtual void SetMenu(ui::MenuModel* menu); virtual bool HasModalDialog(); - virtual void SetParentWindow(NativeWindow* parent) = 0; - virtual void BeginSheet(NativeWindow* sheet); - virtual void EndSheet(NativeWindow* sheet); - virtual void SetModal(bool modal) = 0; + virtual void SetParentWindow(NativeWindow* parent); virtual gfx::NativeWindow GetNativeWindow() = 0; virtual gfx::AcceleratedWidget GetAcceleratedWidget() = 0; @@ -264,9 +261,13 @@ class NativeWindow : public base::SupportsUserData, has_dialog_attached_ = has_dialog_attached; } + NativeWindow* parent() const { return parent_; } + bool is_modal() const { return is_modal_; } + protected: NativeWindow(brightray::InspectableWebContents* inspectable_web_contents, - const mate::Dictionary& options); + const mate::Dictionary& options, + NativeWindow* parent); // Convert draggable regions in raw format to SkRegion format. Caller is // responsible for deleting the returned SkRegion instance. @@ -341,6 +342,12 @@ class NativeWindow : public base::SupportsUserData, // How many times the Disable has been called. int disable_count_; + // The parent window, it is guaranteed to be valid during this window's life. + NativeWindow* parent_; + + // Is this a modal window. + bool is_modal_; + // The page this window is viewing. brightray::InspectableWebContents* inspectable_web_contents_; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 2dcd29806d6..732a75ee18a 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -82,9 +82,6 @@ class NativeWindowMac : public NativeWindow { void SetIgnoreMouseEvents(bool ignore) override; bool HasModalDialog() override; void SetParentWindow(NativeWindow* parent) override; - void BeginSheet(NativeWindow* sheet) override; - void EndSheet(NativeWindow* sheet) override; - void SetModal(bool modal) override; gfx::NativeWindow GetNativeWindow() override; gfx::AcceleratedWidget GetAcceleratedWidget() override; void SetProgressBar(double progress) override; @@ -151,9 +148,6 @@ class NativeWindowMac : public NativeWindow { // The "titleBarStyle" option. TitleBarStyle title_bar_style_; - // Whether to hide the native toolbar under fullscreen mode. - bool should_hide_native_toolbar_in_fullscreen_; - DISALLOW_COPY_AND_ASSIGN(NativeWindowMac); }; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index b49f878eacd..8a5308b6617 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -455,7 +455,7 @@ NativeWindowMac::NativeWindowMac( brightray::InspectableWebContents* web_contents, const mate::Dictionary& options, NativeWindow* parent) - : NativeWindow(web_contents, options), + : NativeWindow(web_contents, options, parent), is_kiosk_(false), attention_request_id_(0), title_bar_style_(NORMAL) { @@ -527,7 +527,8 @@ NativeWindowMac::NativeWindowMac( window_delegate_.reset([[AtomNSWindowDelegate alloc] initWithShell:this]); [window_ setDelegate:window_delegate_]; - if (parent) { + // Only use native parent window for non-modal windows. + if (parent && !is_modal()) { SetParentWindow(parent); } @@ -625,6 +626,12 @@ NativeWindowMac::~NativeWindowMac() { } void NativeWindowMac::Close() { + // When this is a sheet showing, performClose won't work. + if (is_modal() && parent() && IsVisible()) { + CloseImmediately(); + return; + } + if (!IsClosable()) { WindowList::WindowCloseCancelled(this); return; @@ -654,6 +661,12 @@ bool NativeWindowMac::IsFocused() { } void NativeWindowMac::Show() { + if (is_modal() && parent()) { + [parent()->GetNativeWindow() beginSheet:window_ + completionHandler:^(NSModalResponse) {}]; + return; + } + // 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]; @@ -666,6 +679,12 @@ void NativeWindowMac::ShowInactive() { } void NativeWindowMac::Hide() { + if (is_modal() && parent()) { + [window_ orderOut:nil]; + [parent()->GetNativeWindow() endSheet:window_]; + return; + } + [window_ orderOut:nil]; } @@ -679,7 +698,7 @@ void NativeWindowMac::SetEnabled(bool enable) { } bool NativeWindowMac::IsEnabled() { - return ![window_ disableMouseEvents]; + return [window_ attachedSheet] == nil; } void NativeWindowMac::Maximize() { @@ -951,6 +970,11 @@ bool NativeWindowMac::HasModalDialog() { } void NativeWindowMac::SetParentWindow(NativeWindow* parent) { + if (is_modal()) + return; + + NativeWindow::SetParentWindow(parent); + // Remove current parent window. if ([window_ parentWindow]) [[window_ parentWindow] removeChildWindow:window_]; @@ -960,21 +984,6 @@ void NativeWindowMac::SetParentWindow(NativeWindow* parent) { [parent->GetNativeWindow() addChildWindow:window_ ordered:NSWindowAbove]; } -void NativeWindowMac::BeginSheet(NativeWindow* sheet) { - [window_ beginSheet:sheet->GetNativeWindow() - completionHandler:^(NSModalResponse) { - }]; -} - -void NativeWindowMac::EndSheet(NativeWindow* sheet) { - sheet->Hide(); - [window_ endSheet:sheet->GetNativeWindow()]; - sheet->CloseImmediately(); -} - -void NativeWindowMac::SetModal(bool modal) { -} - gfx::NativeWindow NativeWindowMac::GetNativeWindow() { return window_; } diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index ab317f16d53..8d142101094 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -806,6 +806,8 @@ void NativeWindowViews::SetMenu(ui::MenuModel* menu_model) { } void NativeWindowViews::SetParentWindow(NativeWindow* parent) { + NativeWindow::SetParentWindow(parent); + #if defined(USE_X11) XDisplay* xdisplay = gfx::GetXDisplay(); XSetTransientForHint( @@ -830,6 +832,7 @@ void NativeWindowViews::SetParentWindow(NativeWindow* parent) { } void NativeWindowViews::SetModal(bool modal) { + NativeWindow::SetModal(modal); #if defined(USE_X11) SetWindowType(GetAcceleratedWidget(), modal ? "dialog" : "normal"); Show(); diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index e9813af5b75..7310cd6a904 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -902,10 +902,18 @@ describe('browser-window module', function () { }) }) - describe('win.setModal(modal)', function () { + describe('modal option', function () { + // The isEnabled API is not reliable on macOS. + if (process.platform === 'darwin') return + + beforeEach(function () { + if (c != null) c.destroy() + c = new BrowserWindow({show: false, parent: w, modal: true}) + }) + it('disables parent window', function () { assert.equal(w.isEnabled(), true) - c.setModal(true) + c.show() assert.equal(w.isEnabled(), false) }) @@ -914,38 +922,20 @@ describe('browser-window module', function () { assert.equal(w.isEnabled(), true) done() }) - c.setModal(true) + c.show() c.close() }) - it('enables parent window when setting not modal', function () { - assert.equal(w.isEnabled(), true) - c.setModal(true) - assert.equal(w.isEnabled(), false) - c.setModal(false) - assert.equal(w.isEnabled(), true) - }) - - it('enables parent window when removing parent', function () { - if (process.platform !== 'darwin') return - - assert.equal(w.isEnabled(), true) - c.setModal(true) - assert.equal(w.isEnabled(), false) - c.setParentWindow(null) - assert.equal(w.isEnabled(), true) - }) - it('disables parent window recursively', function () { - let c2 = new BrowserWindow({show: false, parent: w}) - c.setModal(true) - c2.setModal(true) + let c2 = new BrowserWindow({show: false, parent: w, modal: true}) + c.show() assert.equal(w.isEnabled(), false) - c.setModal(false) + c2.show() + assert.equal(w.isEnabled(), false) + c.destroy() assert.equal(w.isEnabled(), false) - c2.setModal(false) - assert.equal(w.isEnabled(), true) c2.destroy() + assert.equal(w.isEnabled(), true) }) }) }) From 5674e8d1148fe9cbc61594cfcd74295fafcc1ff8 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 20 Jun 2016 15:44:50 +0900 Subject: [PATCH 22/23] Only define enable/disable API for views --- atom/browser/api/atom_api_window.cc | 14 +---- atom/browser/api/atom_api_window.h | 2 - atom/browser/native_window.cc | 13 ----- atom/browser/native_window.h | 6 -- atom/browser/native_window_mac.h | 1 - atom/browser/native_window_mac.mm | 30 ---------- atom/browser/native_window_views.cc | 89 ++++++++++++++++++----------- atom/browser/native_window_views.h | 7 ++- 8 files changed, 64 insertions(+), 98 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index bfa56dc5aa1..cc3749bd014 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -326,14 +326,6 @@ bool Window::IsVisible() { return window_->IsVisible(); } -void Window::Disable() { - window_->Disable(); -} - -void Window::Enable() { - window_->Enable(); -} - bool Window::IsEnabled() { return window_->IsEnabled(); } @@ -767,8 +759,6 @@ void Window::RemoveFromParentChildWindows() { return; parent->child_windows_.Remove(ID()); - if (IsModal()) - parent->Enable(); } // static @@ -784,8 +774,6 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("showInactive", &Window::ShowInactive) .SetMethod("hide", &Window::Hide) .SetMethod("isVisible", &Window::IsVisible) - .SetMethod("enable", &Window::Enable) - .SetMethod("disable", &Window::Disable) .SetMethod("isEnabled", &Window::IsEnabled) .SetMethod("maximize", &Window::Maximize) .SetMethod("unmaximize", &Window::Unmaximize) @@ -796,7 +784,9 @@ void Window::BuildPrototype(v8::Isolate* isolate, .SetMethod("setFullScreen", &Window::SetFullScreen) .SetMethod("isFullScreen", &Window::IsFullscreen) .SetMethod("setAspectRatio", &Window::SetAspectRatio) +#if !defined(OS_WIN) .SetMethod("setParentWindow", &Window::SetParentWindow) +#endif .SetMethod("getParentWindow", &Window::GetParentWindow) .SetMethod("getChildWindows", &Window::GetChildWindows) .SetMethod("isModal", &Window::IsModal) diff --git a/atom/browser/api/atom_api_window.h b/atom/browser/api/atom_api_window.h index cd8b6ddc42e..21ecca4c729 100644 --- a/atom/browser/api/atom_api_window.h +++ b/atom/browser/api/atom_api_window.h @@ -99,8 +99,6 @@ class Window : public mate::TrackableObject, void ShowInactive(); void Hide(); bool IsVisible(); - void Disable(); - void Enable(); bool IsEnabled(); void Maximize(); void Unmaximize(); diff --git a/atom/browser/native_window.cc b/atom/browser/native_window.cc index 42ba8d90237..4eb72dfe08c 100644 --- a/atom/browser/native_window.cc +++ b/atom/browser/native_window.cc @@ -57,7 +57,6 @@ NativeWindow::NativeWindow( sheet_offset_x_(0.0), sheet_offset_y_(0.0), aspect_ratio_(0.0), - disable_count_(0), parent_(parent), is_modal_(false), inspectable_web_contents_(inspectable_web_contents), @@ -185,18 +184,6 @@ void NativeWindow::InitFromOptions(const mate::Dictionary& options) { Show(); } -void NativeWindow::Disable() { - ++disable_count_; - if (disable_count_ == 1) - SetEnabled(false); -} - -void NativeWindow::Enable() { - --disable_count_; - if (disable_count_ == 0) - SetEnabled(true); -} - void NativeWindow::SetSize(const gfx::Size& size, bool animate) { SetBounds(gfx::Rect(GetPosition(), size), animate); } diff --git a/atom/browser/native_window.h b/atom/browser/native_window.h index ca1ffe2117e..d3b6f8a2b0b 100644 --- a/atom/browser/native_window.h +++ b/atom/browser/native_window.h @@ -98,9 +98,6 @@ class NativeWindow : public base::SupportsUserData, virtual void ShowInactive() = 0; virtual void Hide() = 0; virtual bool IsVisible() = 0; - virtual void Disable(); - virtual void Enable(); - virtual void SetEnabled(bool enable) = 0; // internal API, should not be used virtual bool IsEnabled() = 0; virtual void Maximize() = 0; virtual void Unmaximize() = 0; @@ -339,9 +336,6 @@ class NativeWindow : public base::SupportsUserData, double aspect_ratio_; gfx::Size aspect_ratio_extraSize_; - // How many times the Disable has been called. - int disable_count_; - // The parent window, it is guaranteed to be valid during this window's life. NativeWindow* parent_; diff --git a/atom/browser/native_window_mac.h b/atom/browser/native_window_mac.h index 732a75ee18a..af99b3912e1 100644 --- a/atom/browser/native_window_mac.h +++ b/atom/browser/native_window_mac.h @@ -35,7 +35,6 @@ class NativeWindowMac : public NativeWindow { void ShowInactive() override; void Hide() override; bool IsVisible() override; - void SetEnabled(bool enable) override; bool IsEnabled() override; void Maximize() override; void Unmaximize() override; diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 8a5308b6617..5f997ee9b6b 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -279,7 +279,6 @@ bool ScopedDisableResize::disable_resize_ = false; @property BOOL acceptsFirstMouse; @property BOOL disableAutoHideCursor; @property BOOL disableKeyOrMainWindow; -@property BOOL disableMouseEvents; - (void)setShell:(atom::NativeWindowMac*)shell; - (void)setEnableLargerThanScreen:(bool)enable; @@ -349,29 +348,6 @@ bool ScopedDisableResize::disable_resize_ = false; return !self.disableKeyOrMainWindow; } -- (void)sendEvent:(NSEvent*)event { - // Drop all mouse events. - if (self.disableMouseEvents) { - switch([event type]) { - case NSLeftMouseUp: - case NSLeftMouseDown: - case NSRightMouseDown: - case NSRightMouseUp: - case NSOtherMouseUp: - case NSLeftMouseDragged: - case NSRightMouseDragged: - case NSOtherMouseDragged: - case NSMouseMoved: - case NSScrollWheel: - return; - default: - break; - } - } - - [super sendEvent:event]; -} - @end @interface ControlRegionView : NSView @@ -521,7 +497,6 @@ NativeWindowMac::NativeWindowMac( backing:NSBackingStoreBuffered defer:YES]); [window_ setShell:this]; - [window_ setDisableMouseEvents:NO]; [window_ setEnableLargerThanScreen:enable_larger_than_screen()]; window_delegate_.reset([[AtomNSWindowDelegate alloc] initWithShell:this]); @@ -692,11 +667,6 @@ bool NativeWindowMac::IsVisible() { return [window_ isVisible]; } -void NativeWindowMac::SetEnabled(bool enable) { - [window_ setDisableKeyOrMainWindow:!enable]; - [window_ setDisableMouseEvents:!enable]; -} - bool NativeWindowMac::IsEnabled() { return [window_ attachedSheet] == nil; } diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 8d142101094..1feb9618ac6 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -129,13 +129,14 @@ NativeWindowViews::NativeWindowViews( brightray::InspectableWebContents* web_contents, const mate::Dictionary& options, NativeWindow* parent) - : NativeWindow(web_contents, options), + : NativeWindow(web_contents, options, parent), window_(new views::Widget), web_view_(inspectable_web_contents()->GetView()->GetView()), menu_bar_autohide_(false), menu_bar_visible_(false), menu_bar_alt_pressed_(false), keyboard_event_handler_(new views::UnhandledKeyboardEventHandler), + disable_count_(0), use_content_size_(false), movable_(true), resizable_(true), @@ -242,16 +243,24 @@ NativeWindowViews::NativeWindowViews( state_atom_list.push_back(GetAtom("_NET_WM_STATE_FULLSCREEN")); } + std::string window_type; + options.Get(options::kType, &window_type); + + if (parent) { + SetParentWindow(parent); + // Force using dialog type for child window. + window_type = "dialog"; + // Modal window needs the _NET_WM_STATE_MODAL hint. + if (is_modal()) + state_atom_list.push_back(GetAtom("_NET_WM_STATE_MODAL")); + } + ui::SetAtomArrayProperty(GetAcceleratedWidget(), "_NET_WM_STATE", "ATOM", state_atom_list); // Set the _NET_WM_WINDOW_TYPE. - std::string window_type; - if (options.Get(options::kType, &window_type)) + if (!window_type.empty()) SetWindowType(GetAcceleratedWidget(), window_type); - - if (parent) - SetParentWindow(parent); #endif // Add web view. @@ -346,6 +355,9 @@ bool NativeWindowViews::IsFocused() { } void NativeWindowViews::Show() { + if (is_modal() && NativeWindow::parent()) + static_cast(NativeWindow::parent())->SetEnabled(false); + window_->native_widget_private()->ShowWithWindowState(GetRestoredState()); NotifyWindowShow(); @@ -368,6 +380,9 @@ void NativeWindowViews::ShowInactive() { } void NativeWindowViews::Hide() { + if (is_modal() && NativeWindow::parent()) + static_cast(NativeWindow::parent())->SetEnabled(true); + window_->Hide(); NotifyWindowHide(); @@ -382,22 +397,6 @@ bool NativeWindowViews::IsVisible() { return window_->IsVisible(); } -void NativeWindowViews::SetEnabled(bool enable) { -#if defined(OS_WIN) - ::EnableWindow(GetAcceleratedWidget(), enable); -#elif defined(USE_X11) - views::DesktopWindowTreeHostX11* tree_host = - views::DesktopWindowTreeHostX11::GetHostForXID(GetAcceleratedWidget()); - if (enable) { - tree_host->RemoveEventRewriter(event_disabler_.get()); - event_disabler_.reset(); - } else { - event_disabler_.reset(new EventDisabler); - tree_host->AddEventRewriter(event_disabler_.get()); - } -#endif -} - bool NativeWindowViews::IsEnabled() { #if defined(OS_WIN) return ::IsWindowEnabled(GetAcceleratedWidget()); @@ -831,16 +830,6 @@ void NativeWindowViews::SetParentWindow(NativeWindow* parent) { #endif } -void NativeWindowViews::SetModal(bool modal) { - NativeWindow::SetModal(modal); -#if defined(USE_X11) - SetWindowType(GetAcceleratedWidget(), modal ? "dialog" : "normal"); - Show(); - SetWMSpecState(GetAcceleratedWidget(), modal, - GetAtom("_NET_WM_STATE_MODAL")); -#endif -} - gfx::NativeWindow NativeWindowViews::GetNativeWindow() { return window_->GetNativeWindow(); } @@ -936,6 +925,33 @@ void NativeWindowViews::SetIcon(const gfx::ImageSkia& icon) { } #endif +void NativeWindowViews::SetEnabled(bool enable) { + // Handle multiple calls of SetEnabled correctly. + if (enable) { + --disable_count_; + if (disable_count_ != 0) + return; + } else { + ++disable_count_; + if (disable_count_ != 1) + return; + } + +#if defined(OS_WIN) + ::EnableWindow(GetAcceleratedWidget(), enable); +#elif defined(USE_X11) + views::DesktopWindowTreeHostX11* tree_host = + views::DesktopWindowTreeHostX11::GetHostForXID(GetAcceleratedWidget()); + if (enable) { + tree_host->RemoveEventRewriter(event_disabler_.get()); + event_disabler_.reset(); + } else { + event_disabler_.reset(new EventDisabler); + tree_host->AddEventRewriter(event_disabler_.get()); + } +#endif +} + void NativeWindowViews::OnWidgetActivationChanged( views::Widget* widget, bool active) { if (widget != window_.get()) @@ -969,6 +985,15 @@ void NativeWindowViews::OnWidgetBoundsChanged( } void NativeWindowViews::DeleteDelegate() { + if (is_modal() && NativeWindow::parent()) { + NativeWindowViews* parent = + static_cast(NativeWindow::parent()); + // Enable parent window after current window gets closed. + parent->SetEnabled(true); + // Focus on parent window. + parent->Focus(true); + } + NotifyWindowClosed(); } diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index adc18a1dfde..69d3d27a35d 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -57,7 +57,6 @@ class NativeWindowViews : public NativeWindow, void ShowInactive() override; void Hide() override; bool IsVisible() override; - void SetEnabled(bool enable) override; bool IsEnabled() override; void Maximize() override; void Unmaximize() override; @@ -100,7 +99,6 @@ class NativeWindowViews : public NativeWindow, void SetFocusable(bool focusable) override; void SetMenu(ui::MenuModel* menu_model) override; void SetParentWindow(NativeWindow* parent) override; - void SetModal(bool modal) override; gfx::NativeWindow GetNativeWindow() override; void SetOverlayIcon(const gfx::Image& overlay, const std::string& description) override; @@ -120,6 +118,8 @@ class NativeWindowViews : public NativeWindow, void SetIcon(const gfx::ImageSkia& icon); #endif + void SetEnabled(bool enable); + views::Widget* widget() const { return window_.get(); } #if defined(OS_WIN) @@ -230,6 +230,9 @@ class NativeWindowViews : public NativeWindow, // Map from accelerator to menu item's command id. accelerator_util::AcceleratorTable accelerator_table_; + // How many times the Disable has been called. + int disable_count_; + bool use_content_size_; bool movable_; bool resizable_; From f7a9b1ae04093136197c873a06077907dac5a3a5 Mon Sep 17 00:00:00 2001 From: Cheng Zhao Date: Mon, 20 Jun 2016 16:00:38 +0900 Subject: [PATCH 23/23] docs: modal window is greatly simplified --- docs/api/browser-window.md | 51 ++++++-------------------------------- 1 file changed, 8 insertions(+), 43 deletions(-) diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index dbeb847dfdd..e95570450b6 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -72,39 +72,25 @@ The `child` window will always show on top of the `top` window. ### Modal windows -A modal window is a child window that disables parent window, to turn a child -window into modal window, you need to call `win.setModal(true)`: +A modal window is a child window that disables parent window, to create a modal +window, you have to set both `parent` and `modal` options: ```javascript -let child = new BrowserWindow({parent: top, show: false}) +let child = new BrowserWindow({parent: top, modal: true, show: false}) child.loadURL('https://github.com') -child.setModal(true) child.once('ready-to-show', () => { child.show() }) ``` -### Sheets - -On macOS it is uncommon to use modal windows for tasks, a more native way is to -show window as sheet by calling `win.beginSheet(sheet)` API: - -```javascript -let child = new BrowserWindow({show: false}) -child.loadURL('https://github.com') -child.once('ready-to-show', () => { - top.beginSheet(child) -}) -``` - ### Platform notices -* On macOS the child window will keep the relative position to parent window +* On macOS the child windows will keep the relative position to parent window when parent window moves, while on Windows and Linux child windows will not move. -* On macOS the `setModal` has to be called before the child window shows. * On Windows it is not supported to change parent window dynamically. * On Linux the type of modal windows will be changed to `dialog`. +* On Linux many desktop environments do not support hiding a modal window. ## Class: BrowserWindow @@ -163,7 +149,9 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. `true`. * `frame` Boolean - Specify `false` to create a [Frameless Window](frameless-window.md). Default is `true`. - * `parent` BrowserWindow - Parent window. + * `parent` BrowserWindow - Specify parent window. Default is `null`. + * `modal` Boolean - Whether this is a modal window. This only works when the + window is a child window. Default is `false`. * `acceptFirstMouse` Boolean - Whether the web view accepts a single mouse-down event that simultaneously activates the window. Default is `false`. @@ -580,33 +568,10 @@ Hides the window. Returns a boolean, whether the window is visible to the user. -### `win.setModal(modal)` - -* `modal` Boolean - -Sets current window as modal window. - -It can only be called for child windows. - ### `win.isModal()` Returns whether current window is a modal window. -### `win.beginSheet(sheet)` _macOS_ - -* `sheet` BrowserWindow - -Shows `sheet` as a sheet for current window, when there is already a sheet -showing, this call will be queued. - -It can only be called for top-level windows. - -### `win.endSheet(sheet)` _macOS_ - -* `sheet` BrowserWindow - -Dismisses the `sheet`. - ### `win.maximize()` Maximizes the window.