From 47dab09c5e36a6e293c98776013659da7d45ec83 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Wed, 10 Aug 2022 13:52:39 -0400 Subject: [PATCH] Fix browser focus assignment on mouse click (fixes issue #3306) DesktopWindowTreeHostWin ("Chrome_WidgetWin_0") focus needs to be set before the associated call to WebContents::Focus. In the case of mouse click events, this means ::SetFocus needs to be called explicitly. See updated comments in CefBrowserPlatformDelegateNativeWin::SetFocus. --- .../browser_platform_delegate_native_win.cc | 30 +++++++++++++------ patch/patches/views_widget.patch | 19 ++++++++++-- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/libcef/browser/native/browser_platform_delegate_native_win.cc b/libcef/browser/native/browser_platform_delegate_native_win.cc index 599f530d7..26fc360bd 100644 --- a/libcef/browser/native/browser_platform_delegate_native_win.cc +++ b/libcef/browser/native/browser_platform_delegate_native_win.cc @@ -354,17 +354,26 @@ void CefBrowserPlatformDelegateNativeWin::SetFocus(bool setFocus) { // Give native focus to the DesktopWindowTreeHostWin ("Chrome_WidgetWin_0") // associated with the root window. The currently focused HWND may be // "CefBrowserWindow" if we're called in response to our WndProc receiving - // the WM_SETFOCUS event, or some other HWND if the client calls - // CefBrowserHost::SetFocus(true) directly. + // the WM_SETFOCUS event (possibly due to "CefBrowserWindow" recieving the + // top-level WM_ACTIVATE event), or some other HWND if the client calls + // CefBrowserHost::SetFocus(true) directly. DesktopWindowTreeHostWin may + // also receive focus/blur and mouse click events from the OS directly, in + // which case this method will not be called but the below discussion still + // applies. // - // The DesktopWindowTreeHostWin HandleNativeFocus/HandleNativeBlur methods + // 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: + // DesktopWindowTreeHostWin::HandleMouseEvent method is called if the user + // clicks on the WebContents. These methods have all been patched to call + // HandleActivationChanged (indirectly via ::SetFocus in the case of mouse + // clicks). HandleActivationChanged will then trigger 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). + // expected (see issue #1677) and also triggers an initial call to + // WebContents::Focus which gives logical focus to the + // RenderWidgetHostViewAura in the views hierarchy (see issue #3306). // 2. Update focus state of the ui::InputMethod. If this does not occur // then: // (a) InputMethodBase::GetTextInputClient will return NULL and @@ -376,7 +385,8 @@ void CefBrowserPlatformDelegateNativeWin::SetFocus(bool setFocus) { // causing TSF not to handle IME events (see issue #3306). For this same // reason, ::SetFocus needs to be called before WebContents::Focus which // sends the InputMethod OnWillChangeFocusedClient notification that then - // calls IsWindowFocused. + // calls IsWindowFocused (e.g. WebContents::Focus is intentionally called + // multiple times). // // This differs from activation in Chrome which is handled via // HWNDMessageHandler::PostProcessActivateMessage (Widget::Show indirectly @@ -395,8 +405,10 @@ void CefBrowserPlatformDelegateNativeWin::SetFocus(bool setFocus) { if (web_contents_) { // Give logical focus to the RenderWidgetHostViewAura in the views - // hierarchy. This does not change the native keyboard focus. See above - // comments about InputMethod notifications. + // hierarchy. This does not change the native keyboard focus. When + // |window_widget_| exists this additional Focus() call is necessary to + // correctly assign focus/input state after native focus resulting from + // window activation (see the InputMethod discussion above). web_contents_->Focus(); } } diff --git a/patch/patches/views_widget.patch b/patch/patches/views_widget.patch index 612ec0344..45294316b 100644 --- a/patch/patches/views_widget.patch +++ b/patch/patches/views_widget.patch @@ -258,7 +258,7 @@ index ff670d8099968..f39fd6972e304 100644 // Calculate initial bounds. diff --git ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc -index d7260950ca1ba..67664a29e3510 100644 +index d7260950ca1ba..f18a52e61446c 100644 --- ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc +++ ui/views/widget/desktop_aura/desktop_window_tree_host_win.cc @@ -181,8 +181,12 @@ void DesktopWindowTreeHostWin::Init(const Widget::InitParams& params) { @@ -280,19 +280,32 @@ index d7260950ca1ba..67664a29e3510 100644 void DesktopWindowTreeHostWin::HandleNativeFocus(HWND last_focused_window) { - // TODO(beng): inform the native_widget_delegate_. -+ // See comments in CefBrowserHostImpl::PlatformSetFocus. ++ // See comments in CefBrowserPlatformDelegateNativeWin::SetFocus. + if (has_external_parent_ && CanActivate()) + HandleActivationChanged(true); } void DesktopWindowTreeHostWin::HandleNativeBlur(HWND focused_window) { - // TODO(beng): inform the native_widget_delegate_. -+ // See comments in CefBrowserHostImpl::PlatformSetFocus. ++ // See comments in CefBrowserPlatformDelegateNativeWin::SetFocus. + if (has_external_parent_ && CanActivate()) + HandleActivationChanged(false); } bool DesktopWindowTreeHostWin::HandleMouseEvent(ui::MouseEvent* event) { +@@ -1001,6 +1009,12 @@ bool DesktopWindowTreeHostWin::HandleMouseEvent(ui::MouseEvent* event) { + if (ui::PlatformEventSource::ShouldIgnoreNativePlatformEvents()) + return true; + ++ // See comments in CefBrowserPlatformDelegateNativeWin::SetFocus. ++ if (has_external_parent_ && CanActivate() && event->IsAnyButton() && ++ ::GetFocus() != GetHWND()) { ++ ::SetFocus(GetHWND()); ++ } ++ + SendEventToSink(event); + return event->handled(); + } diff --git ui/views/widget/desktop_aura/desktop_window_tree_host_win.h ui/views/widget/desktop_aura/desktop_window_tree_host_win.h index 0aae49ec83b88..ab61925742ed7 100644 --- ui/views/widget/desktop_aura/desktop_window_tree_host_win.h