From 4a8546efd70700eed1e1f921e85293abf90d2691 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Thu, 27 Aug 2015 17:55:48 -0400 Subject: [PATCH] Windows: Fix focus/activation handling and keyboard input (issue #1700) --- libcef/browser/browser_host_impl_win.cc | 32 ++++++++++++++++++++--- libcef/browser/window_delegate_view.cc | 11 ++++++++ patch/patch.cfg | 10 ++----- patch/patches/aura_window_1677.patch | 32 ----------------------- patch/patches/views_widget_180_1677.patch | 17 +++++++----- 5 files changed, 52 insertions(+), 50 deletions(-) delete mode 100644 patch/patches/aura_window_1677.patch diff --git a/libcef/browser/browser_host_impl_win.cc b/libcef/browser/browser_host_impl_win.cc index d94deb502..c41d620d9 100644 --- a/libcef/browser/browser_host_impl_win.cc +++ b/libcef/browser/browser_host_impl_win.cc @@ -864,10 +864,34 @@ void CefBrowserHostImpl::PlatformSetFocus(bool focus) { } if (window_widget_) { - // Give native focus to the DesktopNativeWidgetAura for the root window. - // Needs to be done via the HWND so that keyboard focus is assigned - // correctly. DesktopNativeWidgetAura will update focus state on the - // aura::Window when WM_SETFOCUS and WM_KILLFOCUS are received. + // Give native focus to the DesktopWindowTreeHostWin associated with the + // root window. + // + // The DesktopWindowTreeHostWin HandleNativeFocus/HandleNativeBlur methods + // are called in response to WM_SETFOCUS/WM_KILLFOCUS respectively. The + // implementation has been patched to call HandleActivationChanged which + // results in the following behaviors: + // 1. Update focus/activation state of the aura::Window indirectly via + // wm::FocusController. This allows focus-related behaviors (e.g. focus + // rings, flashing caret, onFocus/onBlur JS events, etc.) to work as + // expected (see issue #1677). + // 2. Update focus state of the ui::InputMethod. If this does not occur + // then InputMethodBase::GetTextInputClient will return NULL and + // InputMethodWin::OnChar will fail to sent character events to the + // renderer (see issue #1700). + // + // This differs from activation in Chrome which is handled via + // HWNDMessageHandler::PostProcessActivateMessage (Widget::Show indirectly + // calls HWNDMessageHandler::Activate which calls ::SetForegroundWindow + // resulting in a WM_ACTIVATE message being sent to the window). The Chrome + // code path doesn't work for CEF because IsTopLevelWindow in + // hwnd_message_handler.cc will return false and consequently + // HWNDMessageHandler::PostProcessActivateMessage will not be called. + // + // Activation events are usually reserved for the top-level window so + // triggering activation based on focus events may be incorrect in some + // circumstances. Revisit this implementation if additional problems are + // discovered. ::SetFocus(HWNDForWidget(window_widget_)); } } diff --git a/libcef/browser/window_delegate_view.cc b/libcef/browser/window_delegate_view.cc index 491705285..68608f16b 100644 --- a/libcef/browser/window_delegate_view.cc +++ b/libcef/browser/window_delegate_view.cc @@ -41,6 +41,13 @@ void CefWindowDelegateView::Init( params.opacity = views::Widget::InitParams::OPAQUE_WINDOW; // Tell Aura not to draw the window frame on resize. params.remove_standard_frame = true; +#if defined(OS_WIN) + // Cause WidgetDelegate::CanActivate to return true. See comments in + // CefBrowserHostImpl::PlatformSetFocus. + // TODO(cef): Do we need similar logic on Linux for proper activation/focus + // handling? + params.activatable = views::Widget::InitParams::ACTIVATABLE_YES; +#endif // Results in a call to InitContent(). widget->Init(params); @@ -49,6 +56,10 @@ void CefWindowDelegateView::Init( DCHECK_EQ(widget, GetWidget()); // |widget| must be top-level for focus handling to work correctly. DCHECK(widget->is_top_level()); +#if defined(OS_WIN) + // |widget| must be activatable for focus handling to work correctly. + DCHECK(widget->widget_delegate()->CanActivate()); +#endif } void CefWindowDelegateView::InitContent() { diff --git a/patch/patch.cfg b/patch/patch.cfg index c114eb2a8..2dd4be043 100644 --- a/patch/patch.cfg +++ b/patch/patch.cfg @@ -53,8 +53,9 @@ patches = [ { # Allow specification of a parent window handle for Widget creation. # https://code.google.com/p/chromiumembedded/issues/detail?id=180 - # Fix focus/blur handling on Windows. + # Fix focus/activation handling and keyboard input on Windows. # https://bitbucket.org/chromiumembedded/cef/issues/1677 + # https://bitbucket.org/chromiumembedded/cef/issues/1700 'name': 'views_widget_180_1677', 'path': '../ui/views/widget/', }, @@ -182,13 +183,6 @@ patches = [ 'name': 'hwnd_message_handler_1481', 'path': '../ui/views/win/', }, - { - # Fix focus/blur handling on Windows. - # https://bitbucket.org/chromiumembedded/cef/issues/1677 - # https://codereview.chromium.org/1135063002 (reverted) - 'name': 'aura_window_1677', - 'path': '../ui/aura/', - }, { # Fix rendering of the PDF extension with OSR when the device scale factor # is not 1. diff --git a/patch/patches/aura_window_1677.patch b/patch/patches/aura_window_1677.patch deleted file mode 100644 index dde074dd7..000000000 --- a/patch/patches/aura_window_1677.patch +++ /dev/null @@ -1,32 +0,0 @@ -diff --git window.cc window.cc -index 6cde8c2..bdd5093 100644 ---- window.cc -+++ window.cc -@@ -522,6 +522,12 @@ void Window::Focus() { - client->FocusWindow(this); - } - -+void Window::Blur() { -+ client::FocusClient* client = client::GetFocusClient(this); -+ DCHECK(client); -+ client->FocusWindow(NULL); -+} -+ - bool Window::HasFocus() const { - client::FocusClient* client = client::GetFocusClient(this); - return client && client->GetFocusedWindow() == this; -diff --git window.h window.h -index 5586044..53bfd2e 100644 ---- window.h -+++ window.h -@@ -262,8 +262,9 @@ class AURA_EXPORT Window : public ui::LayerDelegate, - // that has a delegate set). The toplevel window may be |this|. - Window* GetToplevelWindow(); - -- // Claims focus. -+ // Claims or relinquishes the claim to focus. - void Focus(); -+ void Blur(); - - // Returns true if the Window is currently the focused window. - bool HasFocus() const; diff --git a/patch/patches/views_widget_180_1677.patch b/patch/patches/views_widget_180_1677.patch index a39a4fed4..22fa0be77 100644 --- a/patch/patches/views_widget_180_1677.patch +++ b/patch/patches/views_widget_180_1677.patch @@ -12,7 +12,7 @@ index a8e088c..838b6a0 100644 return host ? host->GetAcceleratedWidget() : NULL; } diff --git desktop_aura/desktop_window_tree_host_win.cc desktop_aura/desktop_window_tree_host_win.cc -index 5bee7fd..53e6c22 100644 +index 5bee7fd..3d89853 100644 --- desktop_aura/desktop_window_tree_host_win.cc +++ desktop_aura/desktop_window_tree_host_win.cc @@ -131,7 +131,9 @@ void DesktopWindowTreeHostWin::Init(aura::Window* content_window, @@ -26,16 +26,21 @@ index 5bee7fd..53e6c22 100644 parent_hwnd = params.parent->GetHost()->GetAcceleratedWidget(); message_handler_->set_remove_standard_frame(params.remove_standard_frame); -@@ -793,10 +795,12 @@ void DesktopWindowTreeHostWin::HandleFrameChanged() { +@@ -792,11 +794,15 @@ void DesktopWindowTreeHostWin::HandleFrameChanged() { + } void DesktopWindowTreeHostWin::HandleNativeFocus(HWND last_focused_window) { - // TODO(beng): inform the native_widget_delegate_. -+ GetWidget()->GetNativeWindow()->Focus(); +- // TODO(beng): inform the native_widget_delegate_. ++ // See comments in CefBrowserHostImpl::PlatformSetFocus. ++ if (CanActivate()) ++ HandleActivationChanged(true); } void DesktopWindowTreeHostWin::HandleNativeBlur(HWND focused_window) { - // TODO(beng): inform the native_widget_delegate_. -+ GetWidget()->GetNativeWindow()->Blur(); +- // TODO(beng): inform the native_widget_delegate_. ++ // See comments in CefBrowserHostImpl::PlatformSetFocus. ++ if (CanActivate()) ++ HandleActivationChanged(false); } bool DesktopWindowTreeHostWin::HandleMouseEvent(const ui::MouseEvent& event) {