Skip to content

Commit

Permalink
views: Fix Chrome style browser RequestFocus behavior (fixes #3819)
Browse files Browse the repository at this point in the history
Fix implementation of CefBrowserView::RequestFocus for Chrome style
browsers. Match Alloy style behavior of requesting browser focus
(calling OnSetFocus) after initial navigation. Add CefView::HasFocus
and CefWindow::GetFocusedView that can be used in combination with
CefWindow::IsActive to determine global keyboard focus.

Update sample applications for the new behavior.

In cefclient:
- Browser receives initial focus via ViewsWindow::RequestBrowserFocus.
- When running with `--show-overlay-browser` (see #3790):
  - Give initial focus to the overlay browser.
  - Change the overlay popout shortcut to CTRL+SHIFT+O to avoid
    assigning focus to the menu in the main window.
  - Switching from overlay in the main window to popout browser
    window will give focus to the popout browser.
  - Switching from popout browser to overlay will leave current focus
    unchanged (e.g. in the overlay browser, or somewhere else). User
    gesture to activate the main window may be required on Mac/Linux.
- When running with `--no-active` don't give initial focus to either
  browser.

In cefsimple:
- Browser receives initial focus via default handling.
  • Loading branch information
magreenblatt committed Nov 6, 2024
1 parent b070564 commit b660522
Show file tree
Hide file tree
Showing 53 changed files with 652 additions and 91 deletions.
15 changes: 12 additions & 3 deletions include/capi/views/cef_view_capi.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
// by hand. See the translator.README.txt file in the tools directory for
// more information.
//
// $hash=08f13de764f30261616372dfffb7f97c57957f73$
// $hash=4c3a47faa34c20cee64fcaa37123860319995809$
//

#ifndef CEF_INCLUDE_CAPI_VIEWS_CEF_VIEW_CAPI_H_
Expand Down Expand Up @@ -332,8 +332,17 @@ typedef struct _cef_view_t {
int(CEF_CALLBACK* is_accessibility_focusable)(struct _cef_view_t* self);

///
/// Request keyboard focus. If this View is focusable it will become the
/// focused View.
/// Returns true (1) if this View has focus in the context of the containing
/// Window. Check both this function and cef_window_t::IsActive to determine
/// global keyboard focus.
///
int(CEF_CALLBACK* has_focus)(struct _cef_view_t* self);

///
/// Request focus for this View in the context of the containing Window. If
/// this View is focusable it will become the focused View. Any focus changes
/// while a Window is not active may be applied after that Window next becomes
/// active.
///
void(CEF_CALLBACK* request_focus)(struct _cef_view_t* self);

Expand Down
12 changes: 11 additions & 1 deletion include/capi/views/cef_window_capi.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
// by hand. See the translator.README.txt file in the tools directory for
// more information.
//
// $hash=dd32b58ec9aca0e04a1d30ccf94a741995fcc094$
// $hash=9de8ff77a40adc1e3edd7e2648f6c395c21a36bd$
//

#ifndef CEF_INCLUDE_CAPI_VIEWS_CEF_WINDOW_CAPI_H_
Expand All @@ -52,6 +52,7 @@ extern "C" {
#endif

struct _cef_browser_view_t;
struct _cef_view_t;

///
/// A Window is a top-level Window/widget in the Views hierarchy. By default it
Expand Down Expand Up @@ -177,6 +178,15 @@ typedef struct _cef_window_t {
///
int(CEF_CALLBACK* is_fullscreen)(struct _cef_window_t* self);

///
/// Returns the View that currently has focus in this Window, or nullptr if no
/// View currently has focus. A Window may have a focused View even if it is
/// not currently active. Any focus changes while a Window is not active may
/// be applied after that Window next becomes active.
///
struct _cef_view_t*(CEF_CALLBACK* get_focused_view)(
struct _cef_window_t* self);

///
/// Set the Window title.
///
Expand Down
8 changes: 4 additions & 4 deletions include/cef_api_hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,13 @@
// way that may cause binary incompatibility with other builds. The universal
// hash value will change if any platform is affected whereas the platform hash
// values will change only if that particular platform is affected.
#define CEF_API_HASH_UNIVERSAL "548bb305d04b05c0ef29eb2eed88c400e7905ab1"
#define CEF_API_HASH_UNIVERSAL "38565e673fbcfcd9e4494914bcea03609b41ec30"
#if defined(OS_WIN)
#define CEF_API_HASH_PLATFORM "b73640088f5d35912fcba870607031d8d0a0c33e"
#define CEF_API_HASH_PLATFORM "df2092177211214092ab77559596adbc37edf68d"
#elif defined(OS_MAC)
#define CEF_API_HASH_PLATFORM "01224eb3f2e4bfa18defb3e6d40c93f65231b189"
#define CEF_API_HASH_PLATFORM "aaa5bde96ceffff3de2c6fab11142c9264f44a39"
#elif defined(OS_LINUX)
#define CEF_API_HASH_PLATFORM "46107dd79de8c5aab11757980c8951979cc7c266"
#define CEF_API_HASH_PLATFORM "40de77e9ae3e071eda1f3bed7e900aedcdb354e5"
#endif

#ifdef __cplusplus
Expand Down
14 changes: 12 additions & 2 deletions include/views/cef_view.h
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,18 @@ class CefView : public CefBaseRefCounted {
virtual bool IsAccessibilityFocusable() = 0;

///
/// Request keyboard focus. If this View is focusable it will become the
/// focused View.
/// Returns true if this View has focus in the context of the containing
/// Window. Check both this method and CefWindow::IsActive to determine global
/// keyboard focus.
///
/*--cef()--*/
virtual bool HasFocus() = 0;

///
/// Request focus for this View in the context of the containing Window. If
/// this View is focusable it will become the focused View. Any focus changes
/// while a Window is not active may be applied after that Window next becomes
/// active.
///
/*--cef()--*/
virtual void RequestFocus() = 0;
Expand Down
10 changes: 10 additions & 0 deletions include/views/cef_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
#include "include/views/cef_window_delegate.h"

class CefBrowserView;
class CefView;

///
/// A Window is a top-level Window/widget in the Views hierarchy. By default it
Expand Down Expand Up @@ -190,6 +191,15 @@ class CefWindow : public CefPanel {
/*--cef()--*/
virtual bool IsFullscreen() = 0;

///
/// Returns the View that currently has focus in this Window, or nullptr if no
/// View currently has focus. A Window may have a focused View even if it is
/// not currently active. Any focus changes while a Window is not active may
/// be applied after that Window next becomes active.
///
/*--cef()--*/
virtual CefRefPtr<CefView> GetFocusedView() = 0;

///
/// Set the Window title.
///
Expand Down
8 changes: 7 additions & 1 deletion libcef/browser/chrome/chrome_browser_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ CefRefPtr<ChromeBrowserHostImpl> ChromeBrowserHostImpl::Create(
auto browser = CreateBrowser(params, std::nullopt);

GURL url = url_util::MakeGURL(params.url, /*fixup=*/true);
if (url.is_empty()) {
const bool url_is_empty = url.is_empty();
if (url_is_empty) {
// Chrome will navigate to kChromeUINewTabURL by default. We want to keep
// the current CEF behavior of not navigating at all. Use a special URL that
// will be recognized in HandleNonNavigationAboutURL.
Expand All @@ -50,6 +51,11 @@ CefRefPtr<ChromeBrowserHostImpl> ChromeBrowserHostImpl::Create(
ChromeBrowserHostImpl::GetBrowserForContents(web_contents);
CHECK(browser_host);

if (!url_is_empty) {
// Match Alloy-style behavior of requesting focus after initial navigation.
browser_host->OnSetFocus(FOCUS_SOURCE_NAVIGATION);
}

return browser_host;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,12 @@ CefBrowserPlatformDelegateChromeViews::GetBrowserView() const {
return browser_view_.get();
}

void CefBrowserPlatformDelegateChromeViews::SetFocus(bool setFocus) {
if (setFocus && browser_view_) {
browser_view_->RequestFocusSync();
}
}

bool CefBrowserPlatformDelegateChromeViews::IsViewsHosted() const {
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class CefBrowserPlatformDelegateChromeViews
views::Widget* GetWindowWidget() const override;
CefRefPtr<CefBrowserView> GetBrowserView() const override;
void SetBrowserView(CefRefPtr<CefBrowserView> browser_view) override;
void SetFocus(bool setFocus) override;
bool IsViewsHosted() const override;

CefBrowserViewImpl* browser_view() const { return browser_view_.get(); }
Expand Down
11 changes: 2 additions & 9 deletions libcef/browser/views/browser_platform_delegate_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,15 +154,8 @@ void CefBrowserPlatformDelegateViews::SendTouchEvent(
}

void CefBrowserPlatformDelegateViews::SetFocus(bool setFocus) {
// Will activate the Widget and result in a call to WebContents::Focus().
if (setFocus && browser_view_->root_view()) {
if (auto widget = GetWindowWidget()) {
// Don't activate a minimized Widget, or it will be shown.
if (widget->IsMinimized()) {
return;
}
}
browser_view_->root_view()->RequestFocus();
if (setFocus && browser_view_) {
browser_view_->RequestFocusSync();
}
}

Expand Down
23 changes: 16 additions & 7 deletions libcef/browser/views/browser_view_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,20 @@ void CefBrowserViewImpl::BrowserDestroyed(CefBrowserHostBase* browser) {
DCHECK(!cef_widget_);
}

void CefBrowserViewImpl::RequestFocusSync() {
// With Chrome style the root_view() type (ChromeBrowserView) does not accept
// focus, so always give focus to the WebView directly.
if (web_view()) {
if (auto widget = web_view()->GetWidget(); widget->IsMinimized()) {
// Don't activate a minimized Widget, or it will be shown.
return;
}

// Activate the Widget and indirectly call WebContents::Focus().
web_view()->RequestFocus();
}
}

bool CefBrowserViewImpl::HandleKeyboardEvent(
const input::NativeWebKeyboardEvent& event) {
if (!root_view()) {
Expand Down Expand Up @@ -256,9 +270,8 @@ cef_runtime_style_t CefBrowserViewImpl::GetRuntimeStyle() {
void CefBrowserViewImpl::RequestFocus() {
CEF_REQUIRE_VALID_RETURN_VOID();
// Always execute asynchronously to work around issue #3040.
CEF_POST_TASK(CEF_UIT,
base::BindOnce(&CefBrowserViewImpl::RequestFocusInternal,
weak_ptr_factory_.GetWeakPtr()));
CEF_POST_TASK(CEF_UIT, base::BindOnce(&CefBrowserViewImpl::RequestFocusSync,
weak_ptr_factory_.GetWeakPtr()));
}

void CefBrowserViewImpl::SetBackgroundColor(cef_color_t color) {
Expand Down Expand Up @@ -481,10 +494,6 @@ bool CefBrowserViewImpl::HandleAccelerator(
return false;
}

void CefBrowserViewImpl::RequestFocusInternal() {
ParentClass::RequestFocus();
}

void CefBrowserViewImpl::DisassociateFromWidget() {
if (!cef_widget_) {
return;
Expand Down
4 changes: 2 additions & 2 deletions libcef/browser/views/browser_view_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class CefBrowserViewImpl
void BrowserCreated(CefBrowserHostBase* browser,
base::RepeatingClosure on_bounds_changed);
void BrowserDestroyed(CefBrowserHostBase* browser);
void RequestFocusSync();

// Called to handle accelerators when the event is unhandled by the web
// content and the browser client.
Expand Down Expand Up @@ -91,6 +92,7 @@ class CefBrowserViewImpl

// Return the WebView representation of this object.
views::WebView* web_view() const;
views::View* content_view() const override { return web_view(); }

// Return the CEF specialization of BrowserView.
ChromeBrowserView* chrome_browser_view() const;
Expand Down Expand Up @@ -133,8 +135,6 @@ class CefBrowserViewImpl
bool HandleAccelerator(const input::NativeWebKeyboardEvent& event,
views::FocusManager* focus_manager);

void RequestFocusInternal();

void DisassociateFromWidget();

// True if the browser is Alloy style, otherwise Chrome style.
Expand Down
17 changes: 12 additions & 5 deletions libcef/browser/views/view_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ CEF_VIEW_IMPL_T class CefViewImpl : public CefViewAdapter, public CefViewClass {
void SetFocusable(bool focusable) override;
bool IsFocusable() override;
bool IsAccessibilityFocusable() override;
bool HasFocus() override;
void RequestFocus() override;
void SetBackgroundColor(cef_color_t color) override;
cef_color_t GetBackgroundColor() override;
Expand Down Expand Up @@ -656,23 +657,29 @@ CEF_VIEW_IMPL_T bool CEF_VIEW_IMPL_D::IsEnabled() {

CEF_VIEW_IMPL_T void CEF_VIEW_IMPL_D::SetFocusable(bool focusable) {
CEF_REQUIRE_VALID_RETURN_VOID();
root_view()->SetFocusBehavior(focusable ? views::View::FocusBehavior::ALWAYS
: views::View::FocusBehavior::NEVER);
content_view()->SetFocusBehavior(focusable
? views::View::FocusBehavior::ALWAYS
: views::View::FocusBehavior::NEVER);
}

CEF_VIEW_IMPL_T bool CEF_VIEW_IMPL_D::IsFocusable() {
CEF_REQUIRE_VALID_RETURN(false);
return root_view()->IsFocusable();
return content_view()->IsFocusable();
}

CEF_VIEW_IMPL_T bool CEF_VIEW_IMPL_D::IsAccessibilityFocusable() {
CEF_REQUIRE_VALID_RETURN(false);
return root_view()->GetViewAccessibility().IsAccessibilityFocusable();
return content_view()->GetViewAccessibility().IsAccessibilityFocusable();
}

CEF_VIEW_IMPL_T bool CEF_VIEW_IMPL_D::HasFocus() {
CEF_REQUIRE_VALID_RETURN(false);
return content_view()->HasFocus();
}

CEF_VIEW_IMPL_T void CEF_VIEW_IMPL_D::RequestFocus() {
CEF_REQUIRE_VALID_RETURN_VOID();
root_view()->RequestFocus();
content_view()->RequestFocus();
}

CEF_VIEW_IMPL_T void CEF_VIEW_IMPL_D::SetBackgroundColor(cef_color_t color) {
Expand Down
10 changes: 10 additions & 0 deletions libcef/browser/views/window_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,16 @@ bool CefWindowImpl::IsFullscreen() {
return false;
}

CefRefPtr<CefView> CefWindowImpl::GetFocusedView() {
CEF_REQUIRE_VALID_RETURN(nullptr);
if (widget_ && widget_->GetFocusManager()) {
if (auto* focused_view = widget_->GetFocusManager()->GetFocusedView()) {
return view_util::GetFor(focused_view, /*find_known_parent=*/true);
}
}
return nullptr;
}

void CefWindowImpl::SetTitle(const CefString& title) {
CEF_REQUIRE_VALID_RETURN_VOID();
if (root_view()) {
Expand Down
1 change: 1 addition & 0 deletions libcef/browser/views/window_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class CefWindowImpl
bool IsMaximized() override;
bool IsMinimized() override;
bool IsFullscreen() override;
CefRefPtr<CefView> GetFocusedView() override;
void SetTitle(const CefString& title) override;
CefString GetTitle() override;
void SetWindowIcon(CefRefPtr<CefImage> image) override;
Expand Down
22 changes: 21 additions & 1 deletion libcef_dll/cpptoc/views/browser_view_cpptoc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// implementations. See the translator.README.txt file in the tools directory
// for more information.
//
// $hash=34539b590718fa83d794a6cfcb34876da8a03d26$
// $hash=f6c0c1dc3de70dab819ba42db591025c48667379$
//

#include "libcef_dll/cpptoc/views/browser_view_cpptoc.h"
Expand Down Expand Up @@ -964,6 +964,25 @@ browser_view_is_accessibility_focusable(struct _cef_view_t* self) {
return _retval;
}

int CEF_CALLBACK browser_view_has_focus(struct _cef_view_t* self) {
shutdown_checker::AssertNotShutdown();

// AUTO-GENERATED CONTENT - DELETE THIS COMMENT BEFORE MODIFYING

DCHECK(self);
if (!self) {
return 0;
}

// Execute
bool _retval =
CefBrowserViewCppToC::Get(reinterpret_cast<cef_browser_view_t*>(self))
->HasFocus();

// Return type: bool
return _retval;
}

void CEF_CALLBACK browser_view_request_focus(struct _cef_view_t* self) {
shutdown_checker::AssertNotShutdown();

Expand Down Expand Up @@ -1299,6 +1318,7 @@ CefBrowserViewCppToC::CefBrowserViewCppToC() {
GetStruct()->base.is_focusable = browser_view_is_focusable;
GetStruct()->base.is_accessibility_focusable =
browser_view_is_accessibility_focusable;
GetStruct()->base.has_focus = browser_view_has_focus;
GetStruct()->base.request_focus = browser_view_request_focus;
GetStruct()->base.set_background_color = browser_view_set_background_color;
GetStruct()->base.get_background_color = browser_view_get_background_color;
Expand Down
Loading

0 comments on commit b660522

Please sign in to comment.