From de09fa7d209759ab5fed34fddb4c52ada85006cd Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 25 Jun 2019 16:51:10 -0700 Subject: [PATCH 1/9] wip: wish i could hack the main window focus away --- atom/browser/native_window_mac.mm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 1bd932f25f6ce..38d47aff170ca 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -539,6 +539,8 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { void NativeWindowMac::Show() { if (is_modal() && parent()) { + LOG(INFO) << "[LOG] Showing my modal"; + // parent()->SetFocusable(false); if ([window_ sheetParent] == nil) [parent()->GetNativeWindow().GetNativeNSWindow() beginSheet:window_ From b4d841246cb0cd133cafdcb5fb09c81f2fa08b6e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 25 Jun 2019 17:08:02 -0700 Subject: [PATCH 2/9] main -> key --- atom/browser/ui/cocoa/atom_ns_window_delegate.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm index 7e311991a9b7f..2e7fe738b024f 100644 --- a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm +++ b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm @@ -84,11 +84,11 @@ - (NSRect)windowWillUseStandardFrame:(NSWindow*)window return frame; } -- (void)windowDidBecomeMain:(NSNotification*)notification { +- (void)windowDidBecomeKey:(NSNotification*)notification { shell_->NotifyWindowFocus(); } -- (void)windowDidResignMain:(NSNotification*)notification { +- (void)windowDidResignKey:(NSNotification*)notification { shell_->NotifyWindowBlur(); } From e10f479b9e1f65ce1fa4e0ae2b36af79e2368c1e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 26 Jun 2019 09:06:50 -0700 Subject: [PATCH 3/9] remove useless code --- atom/browser/native_window_mac.mm | 2 -- 1 file changed, 2 deletions(-) diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 38d47aff170ca..1bd932f25f6ce 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -539,8 +539,6 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { void NativeWindowMac::Show() { if (is_modal() && parent()) { - LOG(INFO) << "[LOG] Showing my modal"; - // parent()->SetFocusable(false); if ([window_ sheetParent] == nil) [parent()->GetNativeWindow().GetNativeNSWindow() beginSheet:window_ From e1eac86e9b4e208159b60cc779f289ee6ede0575 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 26 Jun 2019 10:28:18 -0700 Subject: [PATCH 4/9] test: Add focus event spec --- spec/api-browser-window-spec.js | 38 +++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 3224209ec8fd6..dbd3055ea4b91 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2480,6 +2480,44 @@ describe('BrowserWindow module', () => { }) }) + describe('focus event', () => { + it('should not emit if focusing on a main window with a modal open', (done) => { + let hasMainFocusAfterModalFocus = false + const child = new BrowserWindow({ + parent: w, + modal: true, + show: false + }) + + child.once('ready-to-show', () => { + child.show() + }) + + // after child gains focus, attempt to focus on + // main window, and set flag if this was successful + child.once('focus', () => { + w.once('focus', () => { + hasMainFocusAfterModalFocus = true + }) + w.focus() + // close child to end the spec + child.close() + }) + + // clean up and assert that main window never + // regained focus + child.once('closed', () => { + w.removeAllListeners() + w.close() + expect(hasMainFocusAfterModalFocus).to.equal(false) + done() + }) + + child.loadURL(server.url) + w.show() + }) + }) + describe('sheet-begin event', () => { let sheet = null From d587a8d6a7b5d0918fbc9a994019e28e1bb96b85 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 26 Jun 2019 11:29:36 -0700 Subject: [PATCH 5/9] test: more robust spec --- spec/api-browser-window-spec.js | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index dbd3055ea4b91..0e0843d6166aa 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2482,7 +2482,8 @@ describe('BrowserWindow module', () => { describe('focus event', () => { it('should not emit if focusing on a main window with a modal open', (done) => { - let hasMainFocusAfterModalFocus = false + let hasMainFocus = false + let childWindowClosed = false const child = new BrowserWindow({ parent: w, modal: true, @@ -2497,20 +2498,21 @@ describe('BrowserWindow module', () => { // main window, and set flag if this was successful child.once('focus', () => { w.once('focus', () => { - hasMainFocusAfterModalFocus = true + hasMainFocus = true + expect(childWindowClosed).to.equal(true) + done() }) w.focus() // close child to end the spec child.close() }) - // clean up and assert that main window never - // regained focus + // clean up + // assert that main window didn't regain focus + // assert that main window child.once('closed', () => { - w.removeAllListeners() - w.close() - expect(hasMainFocusAfterModalFocus).to.equal(false) - done() + childWindowClosed = true + expect(hasMainFocus).to.equal(false) }) child.loadURL(server.url) From cba49856a8109ed21286db6b92dfed0d2cf38120 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 26 Jun 2019 14:08:53 -0700 Subject: [PATCH 6/9] test: simplify test --- spec/api-browser-window-spec.js | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 0e0843d6166aa..51f053a3a89e6 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2482,7 +2482,6 @@ describe('BrowserWindow module', () => { describe('focus event', () => { it('should not emit if focusing on a main window with a modal open', (done) => { - let hasMainFocus = false let childWindowClosed = false const child = new BrowserWindow({ parent: w, @@ -2494,27 +2493,20 @@ describe('BrowserWindow module', () => { child.show() }) - // after child gains focus, attempt to focus on - // main window, and set flag if this was successful - child.once('focus', () => { + child.on('show', () => { w.once('focus', () => { - hasMainFocus = true - expect(childWindowClosed).to.equal(true) + expect(childWindowClosed).to.equal(true, 'Main window should only regain focus once the modal is closed') done() }) - w.focus() - // close child to end the spec + w.focus() // this should not trigger the above listener child.close() }) - // clean up - // assert that main window didn't regain focus - // assert that main window - child.once('closed', () => { + child.on('closed', () => { childWindowClosed = true - expect(hasMainFocus).to.equal(false) }) + // act child.loadURL(server.url) w.show() }) From f677e209dba923405227b42ac43b2fd62b978017 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 27 Jun 2019 14:56:43 -0700 Subject: [PATCH 7/9] =?UTF-8?q?duplicate=20non-firing=20macOS=20event=20li?= =?UTF-8?q?stener=20=F0=9F=98=B3?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/api-browser-window-spec.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 51f053a3a89e6..d2d97a752fbba 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2495,13 +2495,19 @@ describe('BrowserWindow module', () => { child.on('show', () => { w.once('focus', () => { - expect(childWindowClosed).to.equal(true, 'Main window should only regain focus once the modal is closed') + expect(childWindowClosed).to.equal(true, + 'Main window should only regain focus once the modal is closed') done() }) w.focus() // this should not trigger the above listener child.close() }) + // trying stuff... + child.on('close', () => { + childWindowClosed = true + }) + child.on('closed', () => { childWindowClosed = true }) From 66d7d464905d96df552250da03d1d3c78984235e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 27 Jun 2019 15:24:49 -0700 Subject: [PATCH 8/9] =?UTF-8?q?destroy=20=F0=9F=9A=A8?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- spec/api-browser-window-spec.js | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index d2d97a752fbba..d767ae789b61a 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2482,7 +2482,7 @@ describe('BrowserWindow module', () => { describe('focus event', () => { it('should not emit if focusing on a main window with a modal open', (done) => { - let childWindowClosed = false + const childWindowClosed = false const child = new BrowserWindow({ parent: w, modal: true, @@ -2495,23 +2495,13 @@ describe('BrowserWindow module', () => { child.on('show', () => { w.once('focus', () => { - expect(childWindowClosed).to.equal(true, - 'Main window should only regain focus once the modal is closed') + expect(child.isDestroyed()).to.equal(true) done() }) w.focus() // this should not trigger the above listener child.close() }) - // trying stuff... - child.on('close', () => { - childWindowClosed = true - }) - - child.on('closed', () => { - childWindowClosed = true - }) - // act child.loadURL(server.url) w.show() From fb41fa2ac510e34ddae4a91a49a46879fdd9770e Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 1 Jul 2019 15:23:52 -0700 Subject: [PATCH 9/9] chore: remove unused variable --- spec/api-browser-window-spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index d767ae789b61a..e5d93747af493 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2482,7 +2482,6 @@ describe('BrowserWindow module', () => { describe('focus event', () => { it('should not emit if focusing on a main window with a modal open', (done) => { - const childWindowClosed = false const child = new BrowserWindow({ parent: w, modal: true,