From 7cb0324750a28dcf5d501dbe6f8f10fb1564fde9 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 541c90fff4235..97bc8115bc016 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -546,6 +546,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() beginSheet:window_ completionHandler:^(NSModalResponse){ From 7ba99ab55873b1a133c8d5852d05eea414167fde 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 b96316a3ee753..8b1d83aef8cd5 100644 --- a/atom/browser/ui/cocoa/atom_ns_window_delegate.mm +++ b/atom/browser/ui/cocoa/atom_ns_window_delegate.mm @@ -83,11 +83,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 ed23868cf21e1c681848e30d5ff70219e2b8df6e 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 97bc8115bc016..541c90fff4235 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -546,8 +546,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() beginSheet:window_ completionHandler:^(NSModalResponse){ From 40450838e74e3fee4173b3fcfa7baf755abb3929 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 540a197b4eb6e..62ca0fde88abe 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2330,6 +2330,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 f4dfb5b427784cdbc4b79f927c3d08edc23d5158 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 62ca0fde88abe..5490ba1d1a201 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2332,7 +2332,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, @@ -2347,20 +2348,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 f429443c6f703bbd404c057635c40355fcc86646 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 5490ba1d1a201..77f05690cc69b 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2332,7 +2332,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, @@ -2344,27 +2343,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 fbc8ad27742b825ef1b6bb2af99c374b547e8d4c 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 77f05690cc69b..5b2f5a3f66fee 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2345,13 +2345,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 43ef5663f8caf51c7b954cf3323d10a6c11cb6cf 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 5b2f5a3f66fee..2068730ac2517 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2332,7 +2332,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, @@ -2345,23 +2345,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 c567d29097a6a3bbc99a74346b8704dcff9dffb2 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Mon, 1 Jul 2019 15:13:01 -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 2068730ac2517..d309ad0253753 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -2332,7 +2332,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,