Fix multiple major keyboard focus issues on linux/X11(fix issue #2026) by JCYang · Pull Request #4120 · chromiumembedded/cef

JCYang · 2026-03-11T06:24:38Z

Mouse crossing into Cef browser view will take the keyboard focus, credits partitally goes to original patches by Jiří Janoušek here tiliado/cef@f9a1cbe
In X11 world, CefFocusHandler can not properly report OnGotFocus(), this hurts for X11 backends when other native widgets need to compete with focus. Not properly fix this can result in multiple input carets blinking in different widgets. Fix it by monitor via WebContentsObserver::DidGetUserInteraction()

…iumembedded#2026) 1. Mouse crossing into Cef browser view will take the keyboard focus, credits partitally goes to original patches by Jiří Janoušek here tiliado/cef@f9a1cbe 2. In X11 world, CefFocusHandler can not properly report OnGotFocus(), this hurts for X11 backends when other native widgets need to compete with focus. Not properly fix this can result in multiple input carets blinking in different widgets. Fix it by monitor via WebContentsObserver::DidGetUserInteraction()

LeeGDavis · 2026-05-28T17:22:13Z

We’re using a variation of this backported to 7778 to address focus issues for our Linux users. Without it, focus handling breaks. It this being considered for adoption?

magreenblatt

Patch file (views_widget.patch) concerns:

~600 lines of unrelated style changes (brace additions, namespace comments, return-type reformatting) in render_widget_host_view_base.cc, render_widget_host_view_event_handler.cc, window_tree_host_platform.cc. These increase rebase burden and obscure the functional fix. Should be dropped from this PR.
HasExternalParent/SetHasExternalParent duplicated at the wrong layer. The patch adds these to PlatformWindowDelegate and WindowTreeHostPlatform, but CEF already tracks this concept on RenderWidgetHostViewBase and CefBrowserPlatformDelegate. Consider using the existing mechanism instead of adding new virtuals to Chromium base classes.
Menu window parenting is a separate behavioral change. The existing patch unconditionally parents menus to x_root_window_. This PR conditionally parents to parent_widget and sets HasExternalParent(true). This changes popup positioning for all external-parent scenarios and deserves its own discussion.

magreenblatt · 2026-05-28T17:45:43Z

    SkColor background_color)
-    : CefBrowserPlatformDelegateNativeAura(window_info, background_color) {}
+    : CefBrowserPlatformDelegateNativeAura(window_info, background_color) {
+  connection_ = x11::Connection::Get();


connection_ = x11::Connection::Get() runs unconditionally, outside any #if BUILDFLAG(SUPPORTS_OZONE_X11) guard. On a Wayland-only runtime this will crash or return null.

magreenblatt · 2026-05-28T17:45:43Z

+    return;
+  }
+
+  if (not IsWindowVisible(connection_, host_x11_win)) {


Use ! instead of not — not is a valid C++ alternative token but unconventional in Chromium/CEF. Same on the other occurrence in chromeRuntimeBrowserUnfocus().

magreenblatt · 2026-05-28T17:45:43Z

+    return;
+  }
+
+  const auto focused = connection_->GetInputFocus().Sync().reply->focus;


Crash risk: if Sync() fails, reply is null and this dereferences it. Needs a guard like the GetWindowAttributes calls elsewhere in this PR. Same issue at the other GetInputFocus().Sync().reply->focus in chromeRuntimeBrowserUnfocus().

magreenblatt · 2026-05-28T17:45:43Z

+  return false;
+}
+
+void CefBrowserPlatformDelegateNativeLinux::chromeRuntimeBrowserFocus() {


This and chromeRuntimeBrowserUnfocus each make multiple synchronous X11 round-trips (GetInputFocus, GetWindowAttributes, QueryTree) on every focus/unfocus. Worth documenting the perf tradeoff or considering async alternatives.

magreenblatt · 2026-05-28T17:45:44Z

+
 void CefBrowserPlatformDelegateNativeLinux::SetFocus(bool setFocus) {
  if (!setFocus) {
+#if BUILDFLAG(SUPPORTS_OZONE_X11)


The return on the next context line is now inside this #if block. On non-X11 builds, SetFocus(false) falls through to the focus-setting code — breaking the original behavior. Move return after #endif.

magreenblatt · 2026-05-28T17:45:44Z

 }

+#if BUILDFLAG(SUPPORTS_OZONE_X11)
+// does this help wayland? I don't know, I'm just targeting X11. If anyone


Comments like "does this help wayland? I don't know" and "should(should we?) further investigate" read as dev notes. Rewrite as concise technical comments or remove — these belong in the PR description, not shipped code.

magreenblatt · 2026-05-28T17:45:44Z

+        // one exception you think inappropriate
+        wc->Focus();
+      }
+      if (client().get()) {


Use the CEF idiomatic pattern: if (auto c = client()) { if (auto handler = c->GetFocusHandler()) { ... } } instead of .get() null checks.

magreenblatt · 2026-05-28T17:45:44Z

  void WebContentsDestroyed() override;
-
+#if BUILDFLAG(SUPPORTS_OZONE_X11)
+  void DidGetUserInteraction(const blink::WebInputEvent& event) override;


The #if guard makes the class signature vary by build config. Consider always declaring the override with a no-op body on non-X11 to keep the interface stable.

magreenblatt · 2026-05-28T17:45:44Z

 #if BUILDFLAG(SUPPORTS_OZONE_X11)
  raw_ptr<CefWindowX11> window_x11_ = nullptr;
+  // for chrome_runtime only
+  void chromeRuntimeBrowserFocus();


Method names should use PascalCase per Chromium/CEF convention: ChromeRuntimeBrowserFocus, ChromeRuntimeBrowserUnfocus, EnableKeyboardEventsForWindow.

magreenblatt · 2026-05-28T17:45:44Z

+  void chromeRuntimeBrowserUnfocus();
+  void enableKeyboardEventsForWindow(x11::Window target_win, bool enable) const;
+  raw_ptr<x11::Connection> connection_ = nullptr;
+  x11::Window previously_focused_ = x11::Window::None;


previously_focused_ is set but never cleared. If the X11 window is destroyed, the stale ID could be reused by X for a different window, causing chromeRuntimeBrowserUnfocus() to restore focus to the wrong target.

LeeGDavis · 2026-05-28T18:03:37Z

Patch file (views_widget.patch) concerns:

~600 lines of unrelated style changes (brace additions, namespace comments, return-type reformatting) in render_widget_host_view_base.cc, render_widget_host_view_event_handler.cc, window_tree_host_platform.cc. These increase rebase burden and obscure the functional fix. Should be dropped from this PR.

HasExternalParent/SetHasExternalParent duplicated at the wrong layer. The patch adds these to PlatformWindowDelegate and WindowTreeHostPlatform, but CEF already tracks this concept on RenderWidgetHostViewBase and CefBrowserPlatformDelegate. Consider using the existing mechanism instead of adding new virtuals to Chromium base classes.

Menu window parenting is a separate behavioral change. The existing patch unconditionally parents menus to x_root_window_. This PR conditionally parents to parent_widget and sets HasExternalParent(true). This changes popup positioning for all external-parent scenarios and deserves its own discussion.

We noticed the style changes too, but we've also inherited a number of the other things you would like altered. If I find some time I'll try provide some updates to this if @JCYang doesn't update. Currently using LeeGDavis@819559b#diff-941690a3e08dd6925162a020800696183a259ad20a450a077ea7f91c704948c2

JCYang mentioned this pull request Mar 11, 2026

Multiple major keyboard focus issues on Linux (branches 2883, 2840, 2785) #2026

Open

magreenblatt reviewed May 28, 2026

View reviewed changes

Uh oh!

Conversation

JCYang commented Mar 11, 2026

Uh oh!

LeeGDavis commented May 28, 2026

Uh oh!

magreenblatt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LeeGDavis commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants