Skip to content

Commit

Permalink
Focus event to indicate context if UI focused after tab switch
Browse files Browse the repository at this point in the history
When switching to a tab that has the Omnibox or other UI focused,
screen readers do not read the title of the document being navigated to.

For the case where a tab is switched, and the UI is about to be focused,
fire an extra focus event on the widget's root view so that the document
title is read before the UI focus.

TBR: tsepez@chromium.org
Bug: 791757
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I19a91eaac1c24dfd04ab3458a41aaa0c512a8739
Reviewed-on: https://chromium-review.googlesource.com/938778
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542131}
  • Loading branch information
aleventhal authored and Commit Bot committed Mar 9, 2018
1 parent a863f15 commit 3dfddc0
Show file tree
Hide file tree
Showing 14 changed files with 144 additions and 37 deletions.
26 changes: 22 additions & 4 deletions chrome/browser/ui/views/frame/browser_view.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
#include "chrome/browser/ui/views/omnibox/omnibox_view_views.h"
#include "chrome/browser/ui/views/profiles/profile_indicator_icon.h"
#include "chrome/browser/ui/views/status_bubble_views.h"
#include "chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.h"
#include "chrome/browser/ui/views/tabs/browser_tab_strip_controller.h"
#include "chrome/browser/ui/views/tabs/tab.h"
#include "chrome/browser/ui/views/tabs/tab_strip.h"
Expand Down Expand Up @@ -747,19 +748,23 @@ void BrowserView::OnActiveTabChanged(content::WebContents* old_contents,
bool change_tab_contents =
contents_web_view_->web_contents() != new_contents;

bool will_restore_focus = !browser_->tab_strip_model()->closing_all() &&
GetWidget()->IsActive() && GetWidget()->IsVisible();

// Update various elements that are interested in knowing the current
// WebContents.

// When we toggle the NTP floating bookmarks bar and/or the info bar,
// we don't want any WebContents to be attached, so that we
// avoid an unnecessary resize and re-layout of a WebContents.
if (change_tab_contents) {
if (GetWidget() &&
(contents_web_view_->HasFocus() || devtools_web_view_->HasFocus())) {
if (will_restore_focus) {
// Manually clear focus before setting focus behavior so that the focus
// is not temporarily advanced to an arbitrary place in the UI via
// SetFocusBehavior(FocusBehavior::NEVER), confusing screen readers.
// The saved focus for new_contents is restored after it is attached.
// In addition, this ensures that the next RestoreFocus() will be
// read out to screen readers, even if focus doesn't actually change.
GetWidget()->GetFocusManager()->ClearFocus();
}
contents_web_view_->SetWebContents(nullptr);
Expand All @@ -785,6 +790,20 @@ void BrowserView::OnActiveTabChanged(content::WebContents* old_contents,
UpdateDevToolsForContents(new_contents, !change_tab_contents);

if (change_tab_contents) {
// When the location bar or other UI focus will be restored, first focus the
// root view so that screen readers announce the current page title. The
// kFocusContext event will delay the subsequent focus event so that screen
// readers register them as distinct events.
if (will_restore_focus) {
ChromeWebContentsViewFocusHelper* focus_helper =
ChromeWebContentsViewFocusHelper::FromWebContents(new_contents);
if (focus_helper &&
focus_helper->GetStoredFocus() != contents_web_view_) {
GetWidget()->GetRootView()->NotifyAccessibilityEvent(
ax::mojom::Event::kFocusContext, true);
}
}

web_contents_close_handler_->ActiveTabChanged();
contents_web_view_->SetWebContents(new_contents);
SadTabHelper* sad_tab_helper = SadTabHelper::FromWebContents(new_contents);
Expand All @@ -796,8 +815,7 @@ void BrowserView::OnActiveTabChanged(content::WebContents* old_contents,
UpdateDevToolsForContents(new_contents, true);
}

if (!browser_->tab_strip_model()->closing_all() && GetWidget()->IsActive() &&
GetWidget()->IsVisible()) {
if (will_restore_focus) {
// We only restore focus if our window is visible, to avoid invoking blur
// handlers when we are eventually shown.
new_contents->RestoreFocus();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@
#include "content/public/browser/web_contents.h"
#include "ui/views/widget/widget.h"


ChromeWebContentsViewDelegateViews::ChromeWebContentsViewDelegateViews(
content::WebContents* web_contents)
: ContextMenuDelegate(web_contents),
focus_helper_(
std::make_unique<ChromeWebContentsViewFocusHelper>(web_contents)),
web_contents_(web_contents) {}
: ContextMenuDelegate(web_contents), web_contents_(web_contents) {
ChromeWebContentsViewFocusHelper::CreateForWebContents(web_contents);
}

ChromeWebContentsViewDelegateViews::~ChromeWebContentsViewDelegateViews() =
default;
Expand All @@ -43,24 +41,32 @@ content::WebDragDestDelegate*
return bookmark_handler_.get();
}

ChromeWebContentsViewFocusHelper*
ChromeWebContentsViewDelegateViews::GetFocusHelper() const {
ChromeWebContentsViewFocusHelper* helper =
ChromeWebContentsViewFocusHelper::FromWebContents(web_contents_);
DCHECK(helper);
return helper;
}

bool ChromeWebContentsViewDelegateViews::Focus() {
return focus_helper_->Focus();
return GetFocusHelper()->Focus();
}

bool ChromeWebContentsViewDelegateViews::TakeFocus(bool reverse) {
return focus_helper_->TakeFocus(reverse);
return GetFocusHelper()->TakeFocus(reverse);
}

void ChromeWebContentsViewDelegateViews::StoreFocus() {
focus_helper_->StoreFocus();
GetFocusHelper()->StoreFocus();
}

bool ChromeWebContentsViewDelegateViews::RestoreFocus() {
return focus_helper_->RestoreFocus();
return GetFocusHelper()->RestoreFocus();
}

void ChromeWebContentsViewDelegateViews::ResetStoredFocus() {
focus_helper_->ResetStoredFocus();
GetFocusHelper()->ResetStoredFocus();
}

std::unique_ptr<RenderViewContextMenuBase>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#include "components/renderer_context_menu/context_menu_delegate.h"
#include "content/public/browser/web_contents_view_delegate.h"

class ChromeWebContentsViewFocusHelper;
class RenderViewContextMenuBase;
class ChromeWebContentsViewFocusHelper;

namespace content {
class WebContents;
Expand Down Expand Up @@ -49,9 +49,6 @@ class ChromeWebContentsViewDelegateViews
void ShowMenu(std::unique_ptr<RenderViewContextMenuBase> menu) override;

private:
// Used to handle focus management.
std::unique_ptr<ChromeWebContentsViewFocusHelper> focus_helper_;

// The context menu is reset every time we show it, but we keep a pointer to
// between uses so that it won't go out of scope before we're done with it.
std::unique_ptr<RenderViewContextMenuBase> context_menu_;
Expand All @@ -61,6 +58,8 @@ class ChromeWebContentsViewDelegateViews

content::WebContents* web_contents_;

ChromeWebContentsViewFocusHelper* GetFocusHelper() const;

DISALLOW_COPY_AND_ASSIGN(ChromeWebContentsViewDelegateViews);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,22 @@
#include "ui/views/focus/focus_manager.h"
#include "ui/views/widget/widget.h"

DEFINE_WEB_CONTENTS_USER_DATA_KEY(ChromeWebContentsViewFocusHelper);

// static
void ChromeWebContentsViewFocusHelper::CreateForWebContents(
content::WebContents* web_contents) {
if (!ChromeWebContentsViewFocusHelper::FromWebContents(web_contents)) {
web_contents->SetUserData(
ChromeWebContentsViewFocusHelper::UserDataKey(),
base::WrapUnique(new ChromeWebContentsViewFocusHelper(web_contents)));
}
}

ChromeWebContentsViewFocusHelper::ChromeWebContentsViewFocusHelper(
content::WebContents* web_contents)
: web_contents_(web_contents) {}

ChromeWebContentsViewFocusHelper::~ChromeWebContentsViewFocusHelper() {}

bool ChromeWebContentsViewFocusHelper::Focus() {
SadTabHelper* sad_tab_helper = SadTabHelper::FromWebContents(web_contents_);
if (sad_tab_helper) {
Expand Down Expand Up @@ -52,12 +62,10 @@ void ChromeWebContentsViewFocusHelper::StoreFocus() {
}

bool ChromeWebContentsViewFocusHelper::RestoreFocus() {
views::View* last_focused_view = last_focused_view_tracker_.view();
views::View* view_to_focus = GetStoredFocus();
last_focused_view_tracker_.Clear();
if (last_focused_view &&
last_focused_view->IsFocusable() &&
GetFocusManager()->ContainsView(last_focused_view)) {
last_focused_view->RequestFocus();
if (view_to_focus) {
view_to_focus->RequestFocus();
return true;
}
return false;
Expand All @@ -67,6 +75,15 @@ void ChromeWebContentsViewFocusHelper::ResetStoredFocus() {
last_focused_view_tracker_.Clear();
}

views::View* ChromeWebContentsViewFocusHelper::GetStoredFocus() {
views::View* last_focused_view = last_focused_view_tracker_.view();
if (last_focused_view && last_focused_view->IsFocusable() &&
GetFocusManager()->ContainsView(last_focused_view)) {
return last_focused_view;
}
return nullptr;
}

gfx::NativeView ChromeWebContentsViewFocusHelper::GetActiveNativeView() {
return web_contents_->GetFullscreenRenderWidgetHostView() ?
web_contents_->GetFullscreenRenderWidgetHostView()->GetNativeView() :
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#ifndef CHROME_BROWSER_UI_VIEWS_TAB_CONTENTS_CHROME_WEB_CONTENTS_VIEW_FOCUS_HELPER_H_
#define CHROME_BROWSER_UI_VIEWS_TAB_CONTENTS_CHROME_WEB_CONTENTS_VIEW_FOCUS_HELPER_H_

#include "base/supports_user_data.h"
#include "content/public/browser/web_contents_user_data.h"
#include "ui/gfx/native_widget_types.h"
#include "ui/views/view_tracker.h"

Expand All @@ -15,21 +17,29 @@ class WebContents;
namespace views {
class FocusManager;
class Widget;
class View;
}

// A chrome specific helper class that handles focus management.
class ChromeWebContentsViewFocusHelper {
class ChromeWebContentsViewFocusHelper
: public content::WebContentsUserData<ChromeWebContentsViewFocusHelper> {
public:
explicit ChromeWebContentsViewFocusHelper(content::WebContents* web_contents);
~ChromeWebContentsViewFocusHelper();
// Creates a ChromeWebContentsViewFocusHelper for the given
// WebContents. If a ChromeWebContentsViewFocusHelper is already
// associated with the WebContents, this method is a no-op.
static void CreateForWebContents(content::WebContents* web_contents);

void StoreFocus();
bool RestoreFocus();
void ResetStoredFocus();
bool Focus();
bool TakeFocus(bool reverse);
// Returns the View that will be focused if RestoreFocus() is called.
views::View* GetStoredFocus();

private:
explicit ChromeWebContentsViewFocusHelper(content::WebContents* web_contents);
friend class content::WebContentsUserData<ChromeWebContentsViewFocusHelper>;
gfx::NativeView GetActiveNativeView();
views::Widget* GetTopLevelWidget();
views::FocusManager* GetFocusManager();
Expand Down
1 change: 1 addition & 0 deletions chrome/common/extensions/api/automation.idl
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
documentSelectionChanged,
expandedChanged,
focus,
focusContext,
imageFrameUpdated,
hide,
hitTestResult,
Expand Down
2 changes: 2 additions & 0 deletions chrome/renderer/extensions/automation_ax_tree_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ api::automation::EventType ToAutomationEvent(ax::mojom::Event event_type) {
case ax::mojom::Event::kExpandedChanged:
return api::automation::EVENT_TYPE_EXPANDEDCHANGED;
case ax::mojom::Event::kFocus:
case ax::mojom::Event::kFocusContext:
return api::automation::EVENT_TYPE_FOCUS;
case ax::mojom::Event::kHide:
return api::automation::EVENT_TYPE_HIDE;
Expand Down Expand Up @@ -317,6 +318,7 @@ bool AutomationAXTreeWrapper::IsEventTypeHandledByAXEventGenerator(
case api::automation::EVENT_TYPE_NONE:
case api::automation::EVENT_TYPE_AUTOCORRECTIONOCCURED:
case api::automation::EVENT_TYPE_CLICKED:
case api::automation::EVENT_TYPE_FOCUSCONTEXT:
case api::automation::EVENT_TYPE_HITTESTRESULT:
case api::automation::EVENT_TYPE_HOVER:
case api::automation::EVENT_TYPE_MEDIASTARTEDPLAYING:
Expand Down
1 change: 1 addition & 0 deletions third_party/closure_compiler/externs/automation.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ chrome.automation.EventType = {
DOCUMENT_SELECTION_CHANGED: 'documentSelectionChanged',
EXPANDED_CHANGED: 'expandedChanged',
FOCUS: 'focus',
FOCUS_CONTEXT: 'focusContext',
IMAGE_FRAME_UPDATED: 'imageFrameUpdated',
HIDE: 'hide',
HIT_TEST_RESULT: 'hitTestResult',
Expand Down
2 changes: 2 additions & 0 deletions ui/accessibility/ax_enum_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ const char* ToString(ax::mojom::Event event) {
return "expandedChanged";
case ax::mojom::Event::kFocus:
return "focus";
case ax::mojom::Event::kFocusContext:
return "focusContext";
case ax::mojom::Event::kHide:
return "hide";
case ax::mojom::Event::kHitTestResult:
Expand Down
1 change: 1 addition & 0 deletions ui/accessibility/ax_enums.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ enum Event {
kDocumentSelectionChanged,
kExpandedChanged, // Web
kFocus,
kFocusContext, // Contextual focus event that must delay the next focus event
kHide, // Remove: http://crbug.com/392502
kHitTestResult,
kHover,
Expand Down
1 change: 1 addition & 0 deletions ui/accessibility/platform/ax_platform_node_auralinux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1113,6 +1113,7 @@ void AXPlatformNodeAuraLinux::NotifyAccessibilityEvent(
ax::mojom::Event event_type) {
switch (event_type) {
case ax::mojom::Event::kFocus:
case ax::mojom::Event::kFocusContext:
OnFocused();
break;
default:
Expand Down
2 changes: 2 additions & 0 deletions ui/accessibility/platform/ax_platform_node_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ EventMap BuildEventMap() {
const EventMap::value_type events[] = {
{ax::mojom::Event::kFocus,
NSAccessibilityFocusedUIElementChangedNotification},
{ax::mojom::Event::kFocusContext,
NSAccessibilityFocusedUIElementChangedNotification},
{ax::mojom::Event::kTextChanged, NSAccessibilityTitleChangedNotification},
{ax::mojom::Event::kValueChanged,
NSAccessibilityValueChangedNotification},
Expand Down
1 change: 1 addition & 0 deletions ui/accessibility/platform/ax_platform_node_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3564,6 +3564,7 @@ int AXPlatformNodeWin::MSAAEvent(ax::mojom::Event event) {
case ax::mojom::Event::kExpandedChanged:
return EVENT_OBJECT_STATECHANGE;
case ax::mojom::Event::kFocus:
case ax::mojom::Event::kFocusContext:
return EVENT_OBJECT_FOCUS;
case ax::mojom::Event::kMenuStart:
return EVENT_SYSTEM_MENUSTART;
Expand Down
Loading

0 comments on commit 3dfddc0

Please sign in to comment.