From be79417a032602502b4ccb294b2655ea9c8b0c40 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 13 Feb 2017 15:21:55 -0800 Subject: [PATCH 1/7] Include CanFocus on Windows and map to state --- atom/browser/native_window_views.cc | 11 +++---- atom/browser/native_window_views.h | 2 ++ atom/browser/native_window_views_win.cc | 4 +++ .../ui/win/atom_desktop_native_widget_aura.cc | 20 +++++++++++++ .../ui/win/atom_desktop_native_widget_aura.h | 29 +++++++++++++++++++ filenames.gypi | 2 ++ 6 files changed, 63 insertions(+), 5 deletions(-) create mode 100644 atom/browser/ui/win/atom_desktop_native_widget_aura.cc create mode 100644 atom/browser/ui/win/atom_desktop_native_widget_aura.h diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index 771caf583d..a883a66cbb 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -48,6 +48,7 @@ #include "ui/views/window/native_frame_view.h" #elif defined(OS_WIN) #include "atom/browser/ui/views/win_frame_view.h" +#include "atom/browser/ui/win/atom_desktop_native_widget_aura.h" #include "atom/browser/ui/win/atom_desktop_window_tree_host_win.h" #include "skia/ext/skia_utils_win.h" #include "ui/base/win/shell.h" @@ -148,7 +149,8 @@ NativeWindowViews::NativeWindowViews( resizable_(true), maximizable_(true), minimizable_(true), - fullscreenable_(true) { + fullscreenable_(true), + focusable_(true) { options.Get(options::kTitle, &title_); options.Get(options::kAutoHideMenuBar, &menu_bar_autohide_); @@ -196,16 +198,14 @@ NativeWindowViews::NativeWindowViews( if (transparent() && !has_frame()) params.shadow_type = views::Widget::InitParams::SHADOW_TYPE_NONE; - bool focusable; - if (options.Get(options::kFocusable, &focusable) && !focusable) + if (options.Get(options::kFocusable, &focusable_) && !focusable_) params.activatable = views::Widget::InitParams::ACTIVATABLE_NO; #if defined(OS_WIN) if (parent) params.parent = parent->GetNativeWindow(); - params.native_widget = - new views::DesktopNativeWidgetAura(window_.get()); + params.native_widget = new AtomDeskopNativeWidgetAura(window_.get(), this); atom_desktop_window_tree_host_win_ = new AtomDesktopWindowTreeHostWin( this, window_.get(), @@ -806,6 +806,7 @@ void NativeWindowViews::SetContentProtection(bool enable) { } void NativeWindowViews::SetFocusable(bool focusable) { + focusable_ = focusable; #if defined(OS_WIN) LONG ex_style = ::GetWindowLong(GetAcceleratedWidget(), GWL_EXSTYLE); if (focusable) diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index a7f02fb272..032c78f1d1 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -130,6 +130,7 @@ class NativeWindowViews : public NativeWindow, #if defined(OS_WIN) TaskbarHost& taskbar_host() { return taskbar_host_; } + bool CanFocus(); #endif private: @@ -262,6 +263,7 @@ class NativeWindowViews : public NativeWindow, bool maximizable_; bool minimizable_; bool fullscreenable_; + bool focusable_; std::string title_; gfx::Size widget_size_; diff --git a/atom/browser/native_window_views_win.cc b/atom/browser/native_window_views_win.cc index 1b523e90b8..ea6fc8004a 100644 --- a/atom/browser/native_window_views_win.cc +++ b/atom/browser/native_window_views_win.cc @@ -202,4 +202,8 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) { } } +bool NativeWindowViews::CanFocus() { + return focusable_ && IsVisible(); +} + } // namespace atom diff --git a/atom/browser/ui/win/atom_desktop_native_widget_aura.cc b/atom/browser/ui/win/atom_desktop_native_widget_aura.cc new file mode 100644 index 0000000000..812e2636bf --- /dev/null +++ b/atom/browser/ui/win/atom_desktop_native_widget_aura.cc @@ -0,0 +1,20 @@ +// Copyright (c) 2017 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/win/atom_desktop_native_widget_aura.h" + +namespace atom { + +AtomDeskopNativeWidgetAura::AtomDeskopNativeWidgetAura( + views::internal::NativeWidgetDelegate* delegate, + NativeWindowViews* window) + : views::DesktopNativeWidgetAura(delegate), + window_(window) { +} + +bool AtomDeskopNativeWidgetAura::CanFocus() { + return window_->CanFocus(); +} + +} // namespace atom diff --git a/atom/browser/ui/win/atom_desktop_native_widget_aura.h b/atom/browser/ui/win/atom_desktop_native_widget_aura.h new file mode 100644 index 0000000000..7048a47c19 --- /dev/null +++ b/atom/browser/ui/win/atom_desktop_native_widget_aura.h @@ -0,0 +1,29 @@ +// Copyright (c) 2017 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_WIN_ATOM_DESKTOP_NATIVE_WIDGET_AURA_H_ +#define ATOM_BROWSER_UI_WIN_ATOM_DESKTOP_NATIVE_WIDGET_AURA_H_ + +#include "atom/browser/native_window_views.h" +#include "ui/views/widget/desktop_aura/desktop_native_widget_aura.h" + +namespace atom { + +class AtomDeskopNativeWidgetAura : public views::DesktopNativeWidgetAura { + public: + AtomDeskopNativeWidgetAura(views::internal::NativeWidgetDelegate* delegate, + NativeWindowViews* window); + + // aura::WindowDelegate + bool CanFocus() override; + + private: + NativeWindowViews* window_; + + DISALLOW_COPY_AND_ASSIGN(AtomDeskopNativeWidgetAura); +}; + +} // namespace atom + +#endif // ATOM_BROWSER_UI_WIN_ATOM_DESKTOP_NATIVE_WIDGET_AURA_H_ diff --git a/filenames.gypi b/filenames.gypi index d3dc7ed22c..00297becef 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -317,6 +317,8 @@ 'atom/browser/ui/views/submenu_button.h', 'atom/browser/ui/views/win_frame_view.cc', 'atom/browser/ui/views/win_frame_view.h', + 'atom/browser/ui/win/atom_desktop_native_widget_aura.cc', + 'atom/browser/ui/win/atom_desktop_native_widget_aura.h', 'atom/browser/ui/win/atom_desktop_window_tree_host_win.cc', 'atom/browser/ui/win/atom_desktop_window_tree_host_win.h', 'atom/browser/ui/win/jump_list.cc', From 35908ac398bb11f9644dbd28d445a9874843a79a Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 13 Feb 2017 18:11:37 -0800 Subject: [PATCH 2/7] Add webContents.focus() spec --- spec/api-web-contents-spec.js | 12 +++++++++++ spec/fixtures/pages/focus-web-contents.html | 24 +++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 spec/fixtures/pages/focus-web-contents.html diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index f80e04ac7c..07faa93e61 100644 --- a/spec/api-web-contents-spec.js +++ b/spec/api-web-contents-spec.js @@ -308,4 +308,16 @@ describe('webContents module', function () { } }) }) + + describe('focus()', function () { + it('focuses the parent window', function (done) { + ipcMain.once('answer', (event, visible, focused) => { + assert.equal(visible, true) + assert.equal(focused, true) + done() + }) + w.show() + w.loadURL('file://' + path.join(__dirname, 'fixtures', 'pages', 'focus-web-contents.html')) + }) + }) }) diff --git a/spec/fixtures/pages/focus-web-contents.html b/spec/fixtures/pages/focus-web-contents.html new file mode 100644 index 0000000000..8411439a80 --- /dev/null +++ b/spec/fixtures/pages/focus-web-contents.html @@ -0,0 +1,24 @@ + + + + + + + + + + + From 86007fe61dcb980f602670f99f3ebbe88d5c06e7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 13 Feb 2017 18:22:57 -0800 Subject: [PATCH 3/7] Update blur parent window spec --- spec/api-web-contents-spec.js | 16 +++++++++------- spec/fixtures/pages/focus-web-contents.html | 16 ++++++++-------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index 07faa93e61..0faaafbcda 100644 --- a/spec/api-web-contents-spec.js +++ b/spec/api-web-contents-spec.js @@ -310,14 +310,16 @@ describe('webContents module', function () { }) describe('focus()', function () { - it('focuses the parent window', function (done) { - ipcMain.once('answer', (event, visible, focused) => { - assert.equal(visible, true) - assert.equal(focused, true) - done() + describe('when the web contents is hidden', function () { + it('does not blur the focused window', function (done) { + ipcMain.once('answer', (event, parentFocused, childFocused) => { + assert.equal(parentFocused, true) + assert.equal(childFocused, false) + done() + }) + w.show() + w.loadURL('file://' + path.join(__dirname, 'fixtures', 'pages', 'focus-web-contents.html')) }) - w.show() - w.loadURL('file://' + path.join(__dirname, 'fixtures', 'pages', 'focus-web-contents.html')) }) }) }) diff --git a/spec/fixtures/pages/focus-web-contents.html b/spec/fixtures/pages/focus-web-contents.html index 8411439a80..83675bc1d3 100644 --- a/spec/fixtures/pages/focus-web-contents.html +++ b/spec/fixtures/pages/focus-web-contents.html @@ -6,16 +6,16 @@ From bda8af8dd3037e126dddd590de6b8ff087573637 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Feb 2017 12:52:19 -0800 Subject: [PATCH 4/7] Just use visible state in CanFocus delegate --- atom/browser/native_window_views.cc | 7 +++---- atom/browser/native_window_views.h | 2 -- atom/browser/native_window_views_win.cc | 4 ---- atom/browser/ui/win/atom_desktop_native_widget_aura.cc | 2 +- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index a883a66cbb..c2c6212153 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -149,8 +149,7 @@ NativeWindowViews::NativeWindowViews( resizable_(true), maximizable_(true), minimizable_(true), - fullscreenable_(true), - focusable_(true) { + fullscreenable_(true) { options.Get(options::kTitle, &title_); options.Get(options::kAutoHideMenuBar, &menu_bar_autohide_); @@ -198,7 +197,8 @@ NativeWindowViews::NativeWindowViews( if (transparent() && !has_frame()) params.shadow_type = views::Widget::InitParams::SHADOW_TYPE_NONE; - if (options.Get(options::kFocusable, &focusable_) && !focusable_) + bool focusable; + if (options.Get(options::kFocusable, &focusable) && !focusable) params.activatable = views::Widget::InitParams::ACTIVATABLE_NO; #if defined(OS_WIN) @@ -806,7 +806,6 @@ void NativeWindowViews::SetContentProtection(bool enable) { } void NativeWindowViews::SetFocusable(bool focusable) { - focusable_ = focusable; #if defined(OS_WIN) LONG ex_style = ::GetWindowLong(GetAcceleratedWidget(), GWL_EXSTYLE); if (focusable) diff --git a/atom/browser/native_window_views.h b/atom/browser/native_window_views.h index 032c78f1d1..a7f02fb272 100644 --- a/atom/browser/native_window_views.h +++ b/atom/browser/native_window_views.h @@ -130,7 +130,6 @@ class NativeWindowViews : public NativeWindow, #if defined(OS_WIN) TaskbarHost& taskbar_host() { return taskbar_host_; } - bool CanFocus(); #endif private: @@ -263,7 +262,6 @@ class NativeWindowViews : public NativeWindow, bool maximizable_; bool minimizable_; bool fullscreenable_; - bool focusable_; std::string title_; gfx::Size widget_size_; diff --git a/atom/browser/native_window_views_win.cc b/atom/browser/native_window_views_win.cc index ea6fc8004a..1b523e90b8 100644 --- a/atom/browser/native_window_views_win.cc +++ b/atom/browser/native_window_views_win.cc @@ -202,8 +202,4 @@ void NativeWindowViews::HandleSizeEvent(WPARAM w_param, LPARAM l_param) { } } -bool NativeWindowViews::CanFocus() { - return focusable_ && IsVisible(); -} - } // namespace atom diff --git a/atom/browser/ui/win/atom_desktop_native_widget_aura.cc b/atom/browser/ui/win/atom_desktop_native_widget_aura.cc index 812e2636bf..4e2cc60bff 100644 --- a/atom/browser/ui/win/atom_desktop_native_widget_aura.cc +++ b/atom/browser/ui/win/atom_desktop_native_widget_aura.cc @@ -14,7 +14,7 @@ AtomDeskopNativeWidgetAura::AtomDeskopNativeWidgetAura( } bool AtomDeskopNativeWidgetAura::CanFocus() { - return window_->CanFocus(); + return window_->IsVisible(); } } // namespace atom From db79f4f450c86a358e8963fc8f9c028936599b47 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Feb 2017 13:09:24 -0800 Subject: [PATCH 5/7] Implement Activate instead of CanFocus --- atom/browser/native_window_views.cc | 2 +- atom/browser/ui/win/atom_desktop_native_widget_aura.cc | 7 ++++--- atom/browser/ui/win/atom_desktop_native_widget_aura.h | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index c2c6212153..ad85cb0169 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -205,7 +205,7 @@ NativeWindowViews::NativeWindowViews( if (parent) params.parent = parent->GetNativeWindow(); - params.native_widget = new AtomDeskopNativeWidgetAura(window_.get(), this); + params.native_widget = new AtomDesktopNativeWidgetAura(window_.get(), this); atom_desktop_window_tree_host_win_ = new AtomDesktopWindowTreeHostWin( this, window_.get(), diff --git a/atom/browser/ui/win/atom_desktop_native_widget_aura.cc b/atom/browser/ui/win/atom_desktop_native_widget_aura.cc index 4e2cc60bff..2986fdf8ae 100644 --- a/atom/browser/ui/win/atom_desktop_native_widget_aura.cc +++ b/atom/browser/ui/win/atom_desktop_native_widget_aura.cc @@ -6,15 +6,16 @@ namespace atom { -AtomDeskopNativeWidgetAura::AtomDeskopNativeWidgetAura( +AtomDesktopNativeWidgetAura::AtomDesktopNativeWidgetAura( views::internal::NativeWidgetDelegate* delegate, NativeWindowViews* window) : views::DesktopNativeWidgetAura(delegate), window_(window) { } -bool AtomDeskopNativeWidgetAura::CanFocus() { - return window_->IsVisible(); +void AtomDesktopNativeWidgetAura::Activate() { + if (window_->IsVisible()) + views::DesktopNativeWidgetAura::Activate(); } } // namespace atom diff --git a/atom/browser/ui/win/atom_desktop_native_widget_aura.h b/atom/browser/ui/win/atom_desktop_native_widget_aura.h index 7048a47c19..a41d48d73f 100644 --- a/atom/browser/ui/win/atom_desktop_native_widget_aura.h +++ b/atom/browser/ui/win/atom_desktop_native_widget_aura.h @@ -10,18 +10,18 @@ namespace atom { -class AtomDeskopNativeWidgetAura : public views::DesktopNativeWidgetAura { +class AtomDesktopNativeWidgetAura : public views::DesktopNativeWidgetAura { public: - AtomDeskopNativeWidgetAura(views::internal::NativeWidgetDelegate* delegate, + AtomDesktopNativeWidgetAura(views::internal::NativeWidgetDelegate* delegate, NativeWindowViews* window); - // aura::WindowDelegate - bool CanFocus() override; + // internal::NativeWidgetPrivate: + void Activate() override; private: NativeWindowViews* window_; - DISALLOW_COPY_AND_ASSIGN(AtomDeskopNativeWidgetAura); + DISALLOW_COPY_AND_ASSIGN(AtomDesktopNativeWidgetAura); }; } // namespace atom From 52801c4a41b6852503fe1802733ac016a080034d Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Feb 2017 13:14:35 -0800 Subject: [PATCH 6/7] Use internal IsVisible method --- atom/browser/native_window_views.cc | 2 +- .../browser/ui/win/atom_desktop_native_widget_aura.cc | 11 ++++++----- atom/browser/ui/win/atom_desktop_native_widget_aura.h | 5 +---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/atom/browser/native_window_views.cc b/atom/browser/native_window_views.cc index ad85cb0169..b0722cbc4a 100644 --- a/atom/browser/native_window_views.cc +++ b/atom/browser/native_window_views.cc @@ -205,7 +205,7 @@ NativeWindowViews::NativeWindowViews( if (parent) params.parent = parent->GetNativeWindow(); - params.native_widget = new AtomDesktopNativeWidgetAura(window_.get(), this); + params.native_widget = new AtomDesktopNativeWidgetAura(window_.get()); atom_desktop_window_tree_host_win_ = new AtomDesktopWindowTreeHostWin( this, window_.get(), diff --git a/atom/browser/ui/win/atom_desktop_native_widget_aura.cc b/atom/browser/ui/win/atom_desktop_native_widget_aura.cc index 2986fdf8ae..e0cd68608a 100644 --- a/atom/browser/ui/win/atom_desktop_native_widget_aura.cc +++ b/atom/browser/ui/win/atom_desktop_native_widget_aura.cc @@ -7,14 +7,15 @@ namespace atom { AtomDesktopNativeWidgetAura::AtomDesktopNativeWidgetAura( - views::internal::NativeWidgetDelegate* delegate, - NativeWindowViews* window) - : views::DesktopNativeWidgetAura(delegate), - window_(window) { + views::internal::NativeWidgetDelegate* delegate) + : views::DesktopNativeWidgetAura(delegate) { } void AtomDesktopNativeWidgetAura::Activate() { - if (window_->IsVisible()) + // Activate can cause the focused window to be blurred so only + // call when the window being activated is visible. This prevents + // hidden windows from blurring the focused window when created. + if (IsVisible()) views::DesktopNativeWidgetAura::Activate(); } diff --git a/atom/browser/ui/win/atom_desktop_native_widget_aura.h b/atom/browser/ui/win/atom_desktop_native_widget_aura.h index a41d48d73f..30da757ec1 100644 --- a/atom/browser/ui/win/atom_desktop_native_widget_aura.h +++ b/atom/browser/ui/win/atom_desktop_native_widget_aura.h @@ -12,15 +12,12 @@ namespace atom { class AtomDesktopNativeWidgetAura : public views::DesktopNativeWidgetAura { public: - AtomDesktopNativeWidgetAura(views::internal::NativeWidgetDelegate* delegate, - NativeWindowViews* window); + AtomDesktopNativeWidgetAura(views::internal::NativeWidgetDelegate* delegate); // internal::NativeWidgetPrivate: void Activate() override; private: - NativeWindowViews* window_; - DISALLOW_COPY_AND_ASSIGN(AtomDesktopNativeWidgetAura); }; From 70849de8c4c23b191f467c28cb43d36f62691bd2 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Tue, 14 Feb 2017 13:18:06 -0800 Subject: [PATCH 7/7] Mark constructor as explicit --- atom/browser/ui/win/atom_desktop_native_widget_aura.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/atom/browser/ui/win/atom_desktop_native_widget_aura.h b/atom/browser/ui/win/atom_desktop_native_widget_aura.h index 30da757ec1..b5a6c0933d 100644 --- a/atom/browser/ui/win/atom_desktop_native_widget_aura.h +++ b/atom/browser/ui/win/atom_desktop_native_widget_aura.h @@ -12,7 +12,8 @@ namespace atom { class AtomDesktopNativeWidgetAura : public views::DesktopNativeWidgetAura { public: - AtomDesktopNativeWidgetAura(views::internal::NativeWidgetDelegate* delegate); + explicit AtomDesktopNativeWidgetAura( + views::internal::NativeWidgetDelegate* delegate); // internal::NativeWidgetPrivate: void Activate() override;