Commit d2bf7df7 authored by Anatoliy Potapchuk's avatar Anatoliy Potapchuk Committed by Commit Bot

Revert "Set fallback focus to the accounts and sync dialog."

This reverts commit f7e31c9d.

Reason for revert: this seems to cause breakages in mac builders
https://ci.chromium.org/p/chromium/builders/ci/Mac10.14%20Tests/9647

Original change's description:
> 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/+/2475056
> Reviewed-by: Ramin Halavati <rhalavati@chromium.org>
> Commit-Queue: Jacobo Aragunde Pérez <jaragunde@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#819287}

TBR=pbos@chromium.org,jkrcal@chromium.org,rhalavati@chromium.org,jaragunde@igalia.com

Change-Id: I8ad7448148905434910bd1178594e6542935ed85
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1098304
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2489882Reviewed-by: default avatarAnatoliy Potapchuk <apotapchuk@chromium.org>
Commit-Queue: Anatoliy Potapchuk <apotapchuk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#819320}
parent 37effe75
...@@ -941,14 +941,6 @@ void ProfileMenuViewBase::Reset() { ...@@ -941,14 +941,6 @@ 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() {
...@@ -975,6 +967,14 @@ void ProfileMenuViewBase::OnThemeChanged() { ...@@ -975,6 +967,14 @@ 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,6 +180,7 @@ class ProfileMenuViewBase : public content::WebContentsDelegate, ...@@ -180,6 +180,7 @@ 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,
......
...@@ -536,7 +536,7 @@ class ProfileMenuClickTest : public ProfileMenuClickTestBase, ...@@ -536,7 +536,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()); AdvanceFocus(/*count=*/GetParam() + 1);
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