From 64e4b86462d48bbbda61891756b8d14e068ab19a Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 25 Jun 2019 16:51:10 -0700 Subject: [PATCH 1/8] wip: wish i could hack the main window focus away --- shell/browser/native_window_mac.mm | 2 ++ 1 file changed, 2 insertions(+) diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index f96e8ddccd6a5..882c557da3516 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -538,6 +538,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 25096e3895f3bcb4a706ba2edd2dca99ff8ce3d5 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Tue, 25 Jun 2019 17:08:02 -0700 Subject: [PATCH 2/8] main -> key --- shell/browser/ui/cocoa/atom_ns_window_delegate.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shell/browser/ui/cocoa/atom_ns_window_delegate.mm b/shell/browser/ui/cocoa/atom_ns_window_delegate.mm index f685377843105..371bfd2f13347 100644 --- a/shell/browser/ui/cocoa/atom_ns_window_delegate.mm +++ b/shell/browser/ui/cocoa/atom_ns_window_delegate.mm @@ -88,11 +88,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 7af58026b1c0f2957d77aa5b488e57da1fff0cae Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 26 Jun 2019 09:06:50 -0700 Subject: [PATCH 3/8] remove useless code --- shell/browser/native_window_mac.mm | 2 -- 1 file changed, 2 deletions(-) diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index 882c557da3516..f96e8ddccd6a5 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -538,8 +538,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 6d7610b6658b7ba7e42412e1da607baba5849f0a Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 26 Jun 2019 10:28:18 -0700 Subject: [PATCH 4/8] 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 e310090dc7809..1af5c8fd4060e 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1637,6 +1637,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 5d3bc09835ce3e430f69885326691abe09f339b1 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 26 Jun 2019 11:29:36 -0700 Subject: [PATCH 5/8] 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 1af5c8fd4060e..97448abc88dab 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1639,7 +1639,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, @@ -1654,20 +1655,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 9dc2fbaec09d10a58cb35a4126b0b4562073e9b3 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Wed, 26 Jun 2019 14:08:53 -0700 Subject: [PATCH 6/8] 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 97448abc88dab..daced7900b30b 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1639,7 +1639,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, @@ -1651,27 +1650,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 84767b9def1b096330615813e9ebabe08395a2db Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 27 Jun 2019 14:56:43 -0700 Subject: [PATCH 7/8] =?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 daced7900b30b..fb69ec389fb79 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1652,13 +1652,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 19497b419fd1efe5df087a1da8a6c18829113963 Mon Sep 17 00:00:00 2001 From: Erick Zhao Date: Thu, 27 Jun 2019 15:24:49 -0700 Subject: [PATCH 8/8] =?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 fb69ec389fb79..8dd37d158b3ab 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1639,7 +1639,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, @@ -1652,23 +1652,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()