Skip to content

Commit

Permalink
Fix focus issues under X11
Browse files Browse the repository at this point in the history 
 * Prevent focus stealing when mouse enter a web view.
 * Return focus to the toplevel window on
   BrowserHost->SendFocusEvent(false).

Signed-off-by: Jiří Janoušek <janousek.jiri@gmail.com>
  • Loading branch information
@jiri-janousek
jiri-janousek committed Dec 21, 2017
1 parent cd33baa commit b1d9e48
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 8 deletions.
5 changes: 4 additions & 1 deletion libcef/browser/native/browser_platform_delegate_native_linux.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,11 @@ views::Widget* CefBrowserPlatformDelegateNativeLinux::GetWindowWidget() const {
}

void CefBrowserPlatformDelegateNativeLinux::SendFocusEvent(bool setFocus) {
if (!setFocus)
if (!setFocus) {
if (window_x11_)
window_x11_->Unfocus();
return;
}

if (browser_->web_contents()) {
// Give logical focus to the RenderWidgetHostViewAura in the views
Expand Down
25 changes: 25 additions & 0 deletions libcef/browser/native/window_x11.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,31 @@ void CefWindowX11::Focus() {
}
}

void CefWindowX11::Unfocus() {
if (xwindow_ == None || !window_mapped_)
return;

::Window focused = None;
int revert_to = 0;
XGetInputFocus(xdisplay_, &focused, &revert_to);
if (!focused)
return;

::Window toplevel = FindToplevelParent(xdisplay_, xwindow_);
if (toplevel == xwindow_)
return;

::Window child = None;
if (browser_.get()) {
child = FindChild(xdisplay_, xwindow_);
}
if (focused == xwindow_ || focused == child) {
// Our window or child window still has keyboard focus. Return it back to
// the toplevel window so that GUI toolkits can receive keyboard events again.
XSetInputFocus(xdisplay_, toplevel, RevertToParent, CurrentTime);
}
}

void CefWindowX11::SetBounds(const gfx::Rect& bounds) {
if (xwindow_ == None)
return;
Expand Down
1 change: 1 addition & 0 deletions libcef/browser/native/window_x11.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class CefWindowX11 : public ui::PlatformEventDispatcher {
void Hide();

void Focus();
void Unfocus();

void SetBounds(const gfx::Rect& bounds);

Expand Down
34 changes: 27 additions & 7 deletions patch/patches/views_widget_180_1481_1565_1677_1749.patch
View file
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ index db66147f0e9c..2b9bdfa2ec53 100644
// a reference.
corewm::TooltipWin* tooltip_;
diff --git ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
index ebb251545434..14f1320470cd 100644
index ebb251545434..2cee9dd8a7ab 100644
--- ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
+++ ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc
@@ -152,6 +152,7 @@ DesktopWindowTreeHostX11::DesktopWindowTreeHostX11(
Expand Down Expand Up @@ -222,7 +222,18 @@ index ebb251545434..14f1320470cd 100644
return bounds_in_pixels_;
}

@@ -511,7 +515,8 @@ void DesktopWindowTreeHostX11::CloseNow() {
@@ -267,7 +271,9 @@ void DesktopWindowTreeHostX11::OnCrossingEvent(bool enter,
// window or the PointerRoot is focused) && |has_pointer_|. Therefore, we
// can just use |has_pointer_| in the assignment. The transitions for when
// the focus changes are handled in OnFocusEvent().
- has_pointer_focus_ = has_pointer_;
+ // On the other hand, if has_external_parent_ is true, we don't want to steal
+ // focus from the toolkits when mouse enters web view. It must click.
+ has_pointer_focus_ = has_pointer_ && !has_external_parent_;
}

AfterActivationStateChanged();
@@ -511,7 +517,8 @@ void DesktopWindowTreeHostX11::CloseNow() {
// Actually free our native resources.
if (ui::PlatformEventSource::GetInstance())
ui::PlatformEventSource::GetInstance()->RemovePlatformEventDispatcher(this);
Expand All @@ -232,7 +243,7 @@ index ebb251545434..14f1320470cd 100644
xwindow_ = None;

desktop_native_widget_aura_->OnHostClosed();
@@ -652,6 +657,8 @@ void DesktopWindowTreeHostX11::GetWindowPlacement(
@@ -652,6 +659,8 @@ void DesktopWindowTreeHostX11::GetWindowPlacement(
}

gfx::Rect DesktopWindowTreeHostX11::GetWindowBoundsInScreen() const {
Expand All @@ -241,7 +252,16 @@ index ebb251545434..14f1320470cd 100644
return ToDIPRect(bounds_in_pixels_);
}

@@ -1247,6 +1254,8 @@ void DesktopWindowTreeHostX11::SetBoundsInPixels(
@@ -728,7 +737,7 @@ void DesktopWindowTreeHostX11::SetShape(
}

void DesktopWindowTreeHostX11::Activate() {
- if (!IsVisible() || !activatable_)
+ if (!IsVisible() || !activatable_ || has_external_parent_)
return;

BeforeActivationStateChanged();
@@ -1247,6 +1256,8 @@ void DesktopWindowTreeHostX11::SetBoundsInPixels(
}

gfx::Point DesktopWindowTreeHostX11::GetLocationOnScreenInPixels() const {
Expand All @@ -250,15 +270,15 @@ index ebb251545434..14f1320470cd 100644
return bounds_in_pixels_.origin();
}

@@ -1339,7 +1348,6 @@ void DesktopWindowTreeHostX11::InitX11Window(
@@ -1339,7 +1350,6 @@ void DesktopWindowTreeHostX11::InitX11Window(
::Atom window_type;
switch (params.type) {
case Widget::InitParams::TYPE_MENU:
- swa.override_redirect = True;
window_type = gfx::GetAtom("_NET_WM_WINDOW_TYPE_MENU");
break;
case Widget::InitParams::TYPE_TOOLTIP:
@@ -1395,9 +1403,15 @@ void DesktopWindowTreeHostX11::InitX11Window(
@@ -1395,9 +1405,15 @@ void DesktopWindowTreeHostX11::InitX11Window(
attribute_mask |= CWBorderPixel;
swa.border_pixel = 0;

Expand All @@ -275,7 +295,7 @@ index ebb251545434..14f1320470cd 100644
bounds_in_pixels_.y(), bounds_in_pixels_.width(),
bounds_in_pixels_.height(),
0, // border width
@@ -2012,6 +2026,10 @@ uint32_t DesktopWindowTreeHostX11::DispatchEvent(
@@ -2012,6 +2028,10 @@ uint32_t DesktopWindowTreeHostX11::DispatchEvent(
}
break;
}
Expand Down

3 comments on commit b1d9e48

@cztomczak
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fenryxo Have you seen https://bitbucket.org/chromiumembedded/cef/issues/2026/multiple-major-keyboard-focus-issues-on ? Is this fix related?

@jiri-janousek
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @cztomczak. My fix addresses the focus issues I encountered with CEF embedded in a GTK+ 3 app. I haven't tested any of the scenarios in the mentioned ticket, but I think this patch could help if you add a few BrowserHost->SendFocusEvent(false) calls in your code.

public class WebView : Gtk.Widget {
    [...]
    public override void grab_focus() {
        if (browser != null) {
            Cef.assert_browser_ui_thread();
            browser.get_host().send_focus_event(1);
        }
        base.grab_focus();
    }
    
    public override bool focus_in_event(Gdk.EventFocus event) {
        base.focus_in_event(event);
        if (browser != null) {
            Cef.assert_browser_ui_thread();
            browser.get_host().send_focus_event(1);
        }
        return false;
    }
	
    public override bool focus_out_event(Gdk.EventFocus event) {
        base.focus_out_event(event);
        if (browser != null) {
            Cef.assert_browser_ui_thread();
            browser.get_host().send_focus_event(0);
        }
	return false;
    }
}

@cztomczak
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fenryxo Thanks!

Please sign in to comment.