Commit f7e31c9d authored by Jacobo Aragunde Pérez's avatar Jacobo Aragunde Pérez Committed by Commit Bot

Set fallback focus to the accounts and sync dialog.

Set the first container as initial focus, so focus manager will search
the first focusable item in case no other view is explicitly focused
(e.g. with Ctrl+Shift+M, focus is set to the profile list for quick
profile change). Setting focus inside the dialog is important for
accessibility: when focus changes to a view inside the dialog, screen
readers will announce the dialog to give context to the focus change.

We don't need to override GetAccessibleWindowRole() any more because
the implementation in DialogDelegateView will set the 'dialog' role for
bubbles which have an initially focused view.

Tests are altered to make up for the default focus change. Before,
default focus was set outside the dialog and you had to advance once
to reach the first element. Now, it's already placed on the first
element.

Bug: 1098304
Change-Id: Ie0db18193cb224497d311c8ae864ec4e532572b6
AX-relnotes: set focus into, and read accounts and sync dialog when open
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2475056Reviewed-by: default avatarRamin Halavati <rhalavati@chromium.org>
Commit-Queue: Jacobo Aragunde Pérez <jaragunde@igalia.com>
Cr-Commit-Position: refs/heads/master@{#819287}
parent 9f6cd10b
...@@ -941,6 +941,14 @@ void ProfileMenuViewBase::Reset() { ...@@ -941,6 +941,14 @@ void ProfileMenuViewBase::Reset() {
kMenuWidth); kMenuWidth);
layout->StartRow(1.0f, 0); layout->StartRow(1.0f, 0);
layout->AddView(std::move(scroll_view)); layout->AddView(std::move(scroll_view));
// Set the first container as initial focus, so focus manager will search the
// first focusable item in case no other view is explicitly focused (e.g. with
// Ctrl+Shift+M, focus is set to the profile list for quick profile change).
// Setting focus inside the dialog is important for accessibility: when focus
// changes to a view inside the dialog, screen readers will announce the
// dialog to give context to the focus change.
SetInitiallyFocusedView(heading_container_);
} }
void ProfileMenuViewBase::FocusButtonOnKeyboardOpen() { void ProfileMenuViewBase::FocusButtonOnKeyboardOpen() {
...@@ -967,14 +975,6 @@ void ProfileMenuViewBase::OnThemeChanged() { ...@@ -967,14 +975,6 @@ void ProfileMenuViewBase::OnThemeChanged() {
UpdateSyncInfoContainerBackground(); UpdateSyncInfoContainerBackground();
} }
ax::mojom::Role ProfileMenuViewBase::GetAccessibleWindowRole() {
// Return |ax::mojom::Role::kDialog| which will make screen readers announce
// the following in the listed order:
// the title of the dialog, labels (if any), the focused View within the
// dialog (if any)
return ax::mojom::Role::kDialog;
}
bool ProfileMenuViewBase::HandleContextMenu( bool ProfileMenuViewBase::HandleContextMenu(
content::RenderFrameHost* render_frame_host, content::RenderFrameHost* render_frame_host,
const content::ContextMenuParams& params) { const content::ContextMenuParams& params) {
......
...@@ -180,7 +180,6 @@ class ProfileMenuViewBase : public content::WebContentsDelegate, ...@@ -180,7 +180,6 @@ class ProfileMenuViewBase : public content::WebContentsDelegate,
void Init() final; void Init() final;
void WindowClosing() override; void WindowClosing() override;
void OnThemeChanged() override; void OnThemeChanged() override;
ax::mojom::Role GetAccessibleWindowRole() override;
// content::WebContentsDelegate: // content::WebContentsDelegate:
bool HandleContextMenu(content::RenderFrameHost* render_frame_host, bool HandleContextMenu(content::RenderFrameHost* render_frame_host,
......
...@@ -535,7 +535,7 @@ class ProfileMenuClickTest : public ProfileMenuClickTestBase, ...@@ -535,7 +535,7 @@ class ProfileMenuClickTest : public ProfileMenuClickTestBase,
// about the histogram recorded. // about the histogram recorded.
ASSERT_TRUE(profile_menu_view()); ASSERT_TRUE(profile_menu_view());
profile_menu_view()->set_perform_menu_actions_for_testing(false); profile_menu_view()->set_perform_menu_actions_for_testing(false);
AdvanceFocus(/*count=*/GetParam() + 1); AdvanceFocus(/*count=*/GetParam());
ASSERT_TRUE(GetFocusedItem()); ASSERT_TRUE(GetFocusedItem());
Click(GetFocusedItem()); Click(GetFocusedItem());
LOG(INFO) << "Clicked item at index " << GetParam(); LOG(INFO) << "Clicked item at index " << GetParam();
......
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