From 21d714ab6e09eff6a6c103387efa32f54f68b1d1 Mon Sep 17 00:00:00 2001 From: Marshall Greenblatt Date: Fri, 5 Aug 2022 14:58:31 -0400 Subject: [PATCH] Fix issues with browser focus assignment (fixes issue #3306, fixes issue #3116, see issue #3040) DesktopWindowTreeHostWin ("Chrome_WidgetWin_0") focus needs to be set synchronously in response to the parent window WM_SETFOCUS message and before the associated call to WebContents::Focus. See updated comments in CefBrowserPlatformDelegateNativeWin::SetFocus. --- libcef/browser/browser_host_base.cc | 11 +++--- libcef/browser/browser_host_base.h | 2 -- .../browser_platform_delegate_native_win.cc | 34 +++++++++++++------ 3 files changed, 28 insertions(+), 19 deletions(-) diff --git a/libcef/browser/browser_host_base.cc b/libcef/browser/browser_host_base.cc index c2bad10db..6c7e8df08 100644 --- a/libcef/browser/browser_host_base.cc +++ b/libcef/browser/browser_host_base.cc @@ -233,13 +233,12 @@ bool CefBrowserHostBase::HasView() { } void CefBrowserHostBase::SetFocus(bool focus) { - // Always execute asynchronously to work around issue #3040. - CEF_POST_TASK(CEF_UIT, base::BindOnce(&CefBrowserHostBase::SetFocusInternal, - this, focus)); -} + if (!CEF_CURRENTLY_ON_UIT()) { + CEF_POST_TASK(CEF_UIT, + base::BindOnce(&CefBrowserHostBase::SetFocus, this, focus)); + return; + } -void CefBrowserHostBase::SetFocusInternal(bool focus) { - CEF_REQUIRE_UIT(); if (focus) OnSetFocus(FOCUS_SOURCE_SYSTEM); else if (platform_delegate_) diff --git a/libcef/browser/browser_host_base.h b/libcef/browser/browser_host_base.h index 58778891f..3a9d29fac 100644 --- a/libcef/browser/browser_host_base.h +++ b/libcef/browser/browser_host_base.h @@ -337,8 +337,6 @@ class CefBrowserHostBase : public CefBrowserHost, // Called from LoadMainFrameURL to perform the actual navigation. virtual bool Navigate(const content::OpenURLParams& params); - void SetFocusInternal(bool focus); - // Create the CefFileDialogManager if it doesn't already exist. bool EnsureFileDialogManager(); diff --git a/libcef/browser/native/browser_platform_delegate_native_win.cc b/libcef/browser/native/browser_platform_delegate_native_win.cc index ccd5315ce..599f530d7 100644 --- a/libcef/browser/native/browser_platform_delegate_native_win.cc +++ b/libcef/browser/native/browser_platform_delegate_native_win.cc @@ -350,15 +350,12 @@ void CefBrowserPlatformDelegateNativeWin::SetFocus(bool setFocus) { if (!setFocus) return; - if (web_contents_) { - // Give logical focus to the RenderWidgetHostViewAura in the views - // hierarchy. This does not change the native keyboard focus. - web_contents_->Focus(); - } - if (window_widget_) { - // Give native focus to the DesktopWindowTreeHostWin associated with the - // root window. + // 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 DesktopWindowTreeHostWin HandleNativeFocus/HandleNativeBlur methods // are called in response to WM_SETFOCUS/WM_KILLFOCUS respectively. The @@ -369,9 +366,17 @@ void CefBrowserPlatformDelegateNativeWin::SetFocus(bool setFocus) { // 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). + // then: + // (a) InputMethodBase::GetTextInputClient will return NULL and + // InputMethodWin::OnChar will fail to send character events to the + // renderer (see issue #1700); and + // (b) InputMethodWinBase::IsWindowFocused will return false due to + // ::GetFocus() returning the currently focused HWND (e.g. + // "CefBrowserWindow") instead of the expected "Chrome_WidgetWin_0" HWND, + // 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. // // This differs from activation in Chrome which is handled via // HWNDMessageHandler::PostProcessActivateMessage (Widget::Show indirectly @@ -387,6 +392,13 @@ void CefBrowserPlatformDelegateNativeWin::SetFocus(bool setFocus) { // discovered. ::SetFocus(HWNDForWidget(window_widget_)); } + + 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. + web_contents_->Focus(); + } } void CefBrowserPlatformDelegateNativeWin::NotifyMoveOrResizeStarted() {