-
Notifications
You must be signed in to change notification settings - Fork 450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Windows: IME not positioned correctly with embedded windowed browser #3306
Comments
How does it behave with |
|
See also issue #3313. |
Agreed, issue #3313 is a different problem (with OSR, the IME code is implemented in cefclient). |
Testing with current master (M105), I am able to reproduce this issue every time I type in the text field with Chinese or Japanese (instructions from issue #3313). Similar to issue #3313, it starts in the bottom-right corner on the 1st character but then jumps to the top-left corner on 2nd+ character. When misbehaving the composition mode also behaves differently. It shows the characters in a window specific to the IME editor instead of showing them in the search field. The search field is only populated after accepting the composition. This reproduces with both cefclient and cefsimple. |
With logging in RootWindowWin and DesktopWindowTreeHostWin, the IME messages change substantially between “correct” and “incorrect” placement cases. In the “correct” case it looks like some other code or window is handling the composition-related IME events ( With correct IME window placement (native parent):
With correct IME window placement (
With incorrect IME window placement:
|
In the “correct” case, InputMethodWinTSF::OnDidChangeFocusedClient sets |focused| as the focused client and is followed by a call to InputMethodWinTSF::OnFocus. In the “incorrect” case, OnDidChangeFocusedClient is still called followed by OnFocus, but OnDidChangeFocusedClient does not set |focused| as the focused client. Looks like this is due to InputMethodWinBase::IsWindowFocused returning false. However, fixing the above IsWindowFocused behavior does not resolve the issue. Adding more logging and testing by entering a character, then changing windows (loose focus), then returning to the cefclient window (gain focus), then entering another character: With “correct” behavior (
With “incorrect” behavior”:
|
Looks like the |
Original comment by WONHEE JEONG (Bitbucket: WONHEE JEONG). When I create a browser with CefBowerHost::CreateBrowser() as child, I have the same issue in Korean Input mode too but in english mode is not. I tried the following method to bypass this issue.
I don't know why the issue was resolved this way. But it was temporarily resolved. |
One difference between Views a non-Views on re-focus is that is this code is failing in TSFEventRouter::Delegate::SetManager.
|
Thanks for that detail. I think the problem here is that the wrong HWND is getting focused (at least from TSF’s perspective). That would also explain why |
First problem: The WM_SETFOCUS event in CefBrowserPlatformDelegateNativeWin::WndProc is calling CefBrowserHostBase::SetFocus asynchronously due to issue #3040. Second problem (after fixing the 1st problem): CefBrowserPlatformDelegateNativeWin::SetFocus (which calls ::SetFocus with the Chrome_WidgetWin_0 handle) is being called after InputMethodWinTSF::OnWillChangeFocusedClient:
The InputMethodWinTSF::OnWillChangeFocusedClient call appears to result from the
However, we’ll still need to revisit issue #3040. |
Fix issues with browser focus assignment (fixes issue 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. → <<cset 21d714ab6e09 (bb)>> |
|
Fix issues with browser focus assignment (fixes issue #3306, fixes issue #3166, see issue #3040) DesktopWindowTreeHostWin ("Chrome_WidgetWin_0") focus needs to be set → <<cset 08f37697afd7 (bb)>> |
Fix issues with browser focus assignment (fixes issue #3306, fixes issue #3166, see issue #3040) DesktopWindowTreeHostWin ("Chrome_WidgetWin_0") focus needs to be set → <<cset 2587cf23c511 (bb)>> |
Original comment by Skogkatt (Bitbucket: Skogkatt, GitHub: Skogkatt). I’ve tested official build cef_binary_104.4.18+g2587cf2+chromium-104.0.5112.81_windows32_client.tar.bz2 and cef_binary_105.0.3+g08f3769+chromium-105.0.5195.8_windows32_beta_client.tar.bz2, the problem still exists. And I back port change set to 4515 CEF92 version, the problem still exists too, may be need some more digging. I’ve debug this issue these days too, and I found this problem was probably introduced in this commit, year 2020: SHA-1: 71768ea6c32deba08d16c2c77bc9ddce5f33387b (bb)
Before this commit, it is Chromium 79.0.3945.1, and cefclient works just fine most times ( 78 some times not correct but I dunno why). |
What Windows version and IME language are you testing? Are you adding any command-line flags? Are your reproduction steps exactly the same as above? |
Original comment by Skogkatt (Bitbucket: Skogkatt, GitHub: Skogkatt). @wonhee JEONG 's modification works for me. @marshall Greenblatt : What Windows version and IME language are you testing? My Windows version is Windows 10 Pro workstation 21H2 With Microsoft Pinyin. Are you adding any command-line flags? 104: run without command line flags has the bug. run with 105: run without command line flags has the bug. run with Are your reproduction steps exactly the same as above? Yes, I follow my steps above:
|
Thanks, I can reproduce the incorrect placement issue when switching focus in cefclient between the URL text field and the browser search field. Placement is correct when switching focus between different windows (was also incorrect previously). |
In this case CefBrowserPlatformDelegateNativeWin::SetFocus and TSFBridgeImpl::SetFocusedClient are not being called when clicking into the search field. Additionally, you need to click twice before the search field gets input focus (which is also incorrect behavior).
The call to This is the call stack for the OnWebContentsFocused call:
|
It looks like the first click simply dismisses the IME composition window, and the 2nd click actually focuses the browser view. |
OnWebContentsFocused is being called before DesktopWindowTreeHostWin::HandleNativeFocus when clicking in the search field. This is problematic because GetFocus() won’t be returning the correct/expected value at this point.
Related call stack:
This isn’t a problem in Chrome because the WM_ACTIVATE message, which is sent to the root window, will have already assigned keyboard focus via DefaultWndProc handling (docs). In CEF, where DesktopWindowTreeHostWin is not the root window, we’ve patched DesktopWindowTreeHostWin::HandleNativeFocus to call HandleActivationChanged, and that works in the switching windows case. However, with this click behavior, the HandleNativeFocus call is arriving too late (after WebContents::Focus). |
IME logging changes, for future reference:
|
It would be good to also test the behavior with touch events, as that may require a similar change. |
Fix browser focus assignment on mouse click (fixes issue #3306) DesktopWindowTreeHostWin ("Chrome_WidgetWin_0") focus needs to be set before → <<cset 8908465546bf (bb)>> |
|
Fix browser focus assignment on mouse click (fixes issue #3306) DesktopWindowTreeHostWin ("Chrome_WidgetWin_0") focus needs to be set before → <<cset c9b8f03731c6 (bb)>> |
Fix browser focus assignment on mouse click (fixes issue #3306) DesktopWindowTreeHostWin ("Chrome_WidgetWin_0") focus needs to be set before → <<cset 47dab09c5e36 (bb)>> |
…d#3306, fixes issue chromiumembedded#3166, see issue chromiumembedded#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.
…dded#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.
…d#3306, fixes issue chromiumembedded#3116, see issue chromiumembedded#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.
…dded#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.
…d#3306, fixes issue chromiumembedded#3166, see issue chromiumembedded#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.
…dded#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.
Original report by Skogkatt (Bitbucket: Skogkatt, GitHub: Skogkatt).
I know there was already a issue (
#1610) with the same name and was fixed. Here is the new one.When using the Microsoft Pinyin on Windows, the IME window is not positioned next to the input field that has focus.
First configure for Chinese IME Microsoft Pinyin on windows 10 21H2:
Reproducing the problem:
I’ve tested CEF78,92,100, under Windows 10,11 on five machines, all of them have this issue.
The text was updated successfully, but these errors were encountered: