Commit 414b62b0 authored by Maria Villarreal's avatar Maria Villarreal Committed by Commit Bot

Set initial focus on session restore

Issue:
- When On startup setting is set to "Continue where you left off",
the initial focus is not set after launching the browser.
- Users have to press Tab to focus the window.
- This affects users who rely on screen readers since the window
information is not announced on launch (on UIA mode).

More context:
- The line calling SetInitialFocus was previously in the code but got
removed after a startup focus refactor:
https://chromium-review.googlesource.com/c/chromium/src/+/1313728
- The refactor added the call to RestoreFocus on
BrowserView::OnWidgetActivationChanged if the property
restore_focus_on_activation_ is set to True. This property gets set
during BrowserView::Show, but only in the case of session restore,
Show gets called after OnWidgetActivationChanged so RestoreFocus is
never called.
- The inverted call stack on session restore is because AddRestoredTab
on browser_tabrestore calls Activate before showing the window.

Fix:
- Similar to what we currently have for MacOS, only call Activate
in AddRestoredTab if it is not |from_session_restore|. This way Show
will be called before Activate and thus focus will be set as
expected.
- This change is only made on Windows due to a couple of reasons
1) crbug.com/1102685 is only present on this platform.
2) Making this change on Linux would re-introduce crbug.com/1019048.

Bug: 1102685
Change-Id: I6e62ebad7693eb5d908770364ba7c8d755ccaa70
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2284382Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Maria Villarreal <mavill@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#791950}
parent 9641b626
......@@ -70,6 +70,7 @@
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/bindings_policy.h"
#include "content/public/test/browser_test.h"
......@@ -913,6 +914,11 @@ IN_PROC_BROWSER_TEST_F(SessionRestoreTest, Basic) {
ASSERT_EQ(1u, active_browser_list_->size());
ASSERT_EQ(url2_,
new_browser->tab_strip_model()->GetActiveWebContents()->GetURL());
// Ensure window has initial focus on launch.
EXPECT_TRUE(new_browser->tab_strip_model()
->GetActiveWebContents()
->GetRenderWidgetHostView()
->HasFocus());
GoBack(new_browser);
ASSERT_EQ(url1_,
new_browser->tab_strip_model()->GetActiveWebContents()->GetURL());
......
......@@ -180,11 +180,12 @@ WebContents* AddRestoredTab(
raw_web_contents->WasHidden();
} else {
const bool should_activate =
#if defined(OS_MACOSX)
#if defined(OS_WIN) || defined(OS_MACOSX)
// Activating a window on another space causes the system to switch to
// that space. Since the session restore process shows and activates
// windows itself, activating windows here should be safe to skip.
// Cautiously apply only to macOS, for now (https://crbug.com/1019048).
// Cautiously apply only to Windows and MacOS, for now
// (https://crbug.com/1019048).
!from_session_restore;
#else
true;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment