From 3dfddc05c6fea9d96313e62a8093d84823bec87c Mon Sep 17 00:00:00 2001 From: Aaron Leventhal Date: Fri, 9 Mar 2018 16:10:08 +0000 Subject: [PATCH] Focus event to indicate context if UI focused after tab switch 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 Reviewed-by: Scott Violet Reviewed-by: Dominic Mazzoni Cr-Commit-Position: refs/heads/master@{#542131} --- chrome/browser/ui/views/frame/browser_view.cc | 26 ++++++-- ...chrome_web_contents_view_delegate_views.cc | 26 +++++--- .../chrome_web_contents_view_delegate_views.h | 7 +- .../chrome_web_contents_view_focus_helper.cc | 31 +++++++-- .../chrome_web_contents_view_focus_helper.h | 16 ++++- chrome/common/extensions/api/automation.idl | 1 + .../extensions/automation_ax_tree_wrapper.cc | 2 + .../closure_compiler/externs/automation.js | 1 + ui/accessibility/ax_enum_util.cc | 2 + ui/accessibility/ax_enums.mojom | 1 + .../platform/ax_platform_node_auralinux.cc | 1 + .../platform/ax_platform_node_mac.mm | 2 + .../platform/ax_platform_node_win.cc | 1 + .../native_view_accessibility_base.cc | 64 ++++++++++++++++--- 14 files changed, 144 insertions(+), 37 deletions(-) diff --git a/chrome/browser/ui/views/frame/browser_view.cc b/chrome/browser/ui/views/frame/browser_view.cc index 1cc7f254761d61..6ed0f792010dff 100644 --- a/chrome/browser/ui/views/frame/browser_view.cc +++ b/chrome/browser/ui/views/frame/browser_view.cc @@ -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" @@ -747,6 +748,9 @@ 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. @@ -754,12 +758,13 @@ void BrowserView::OnActiveTabChanged(content::WebContents* old_contents, // 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); @@ -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); @@ -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(); diff --git a/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.cc b/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.cc index 3ee30942dcd28c..9dc22d125004d6 100644 --- a/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.cc +++ b/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.cc @@ -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(web_contents)), - web_contents_(web_contents) {} + : ContextMenuDelegate(web_contents), web_contents_(web_contents) { + ChromeWebContentsViewFocusHelper::CreateForWebContents(web_contents); +} ChromeWebContentsViewDelegateViews::~ChromeWebContentsViewDelegateViews() = default; @@ -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 diff --git a/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.h b/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.h index 684850c6c9ac0d..196b84b639fb84 100644 --- a/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.h +++ b/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_delegate_views.h @@ -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; @@ -49,9 +49,6 @@ class ChromeWebContentsViewDelegateViews void ShowMenu(std::unique_ptr menu) override; private: - // Used to handle focus management. - std::unique_ptr 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 context_menu_; @@ -61,6 +58,8 @@ class ChromeWebContentsViewDelegateViews content::WebContents* web_contents_; + ChromeWebContentsViewFocusHelper* GetFocusHelper() const; + DISALLOW_COPY_AND_ASSIGN(ChromeWebContentsViewDelegateViews); }; diff --git a/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.cc b/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.cc index ece83aae91058d..9b4689d9259900 100644 --- a/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.cc +++ b/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.cc @@ -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) { @@ -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; @@ -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() : diff --git a/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.h b/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.h index 1d05c7e36ff834..684494b55771ac 100644 --- a/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.h +++ b/chrome/browser/ui/views/tab_contents/chrome_web_contents_view_focus_helper.h @@ -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" @@ -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 { 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; gfx::NativeView GetActiveNativeView(); views::Widget* GetTopLevelWidget(); views::FocusManager* GetFocusManager(); diff --git a/chrome/common/extensions/api/automation.idl b/chrome/common/extensions/api/automation.idl index 97fd6e9c2e98a6..93531c02026c3e 100644 --- a/chrome/common/extensions/api/automation.idl +++ b/chrome/common/extensions/api/automation.idl @@ -24,6 +24,7 @@ documentSelectionChanged, expandedChanged, focus, + focusContext, imageFrameUpdated, hide, hitTestResult, diff --git a/chrome/renderer/extensions/automation_ax_tree_wrapper.cc b/chrome/renderer/extensions/automation_ax_tree_wrapper.cc index 9fc7eba29c98cc..2137e4ad550030 100644 --- a/chrome/renderer/extensions/automation_ax_tree_wrapper.cc +++ b/chrome/renderer/extensions/automation_ax_tree_wrapper.cc @@ -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; @@ -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: diff --git a/third_party/closure_compiler/externs/automation.js b/third_party/closure_compiler/externs/automation.js index d7cb8660ef6cf7..23f0d529e79ce1 100644 --- a/third_party/closure_compiler/externs/automation.js +++ b/third_party/closure_compiler/externs/automation.js @@ -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', diff --git a/ui/accessibility/ax_enum_util.cc b/ui/accessibility/ax_enum_util.cc index 416206988040dc..b9d4c9e55d7bb6 100644 --- a/ui/accessibility/ax_enum_util.cc +++ b/ui/accessibility/ax_enum_util.cc @@ -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: diff --git a/ui/accessibility/ax_enums.mojom b/ui/accessibility/ax_enums.mojom index 0d84da6436fa88..fa08af479f5e62 100644 --- a/ui/accessibility/ax_enums.mojom +++ b/ui/accessibility/ax_enums.mojom @@ -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, diff --git a/ui/accessibility/platform/ax_platform_node_auralinux.cc b/ui/accessibility/platform/ax_platform_node_auralinux.cc index b502a6786c6ef8..ed48d127c31f3f 100644 --- a/ui/accessibility/platform/ax_platform_node_auralinux.cc +++ b/ui/accessibility/platform/ax_platform_node_auralinux.cc @@ -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: diff --git a/ui/accessibility/platform/ax_platform_node_mac.mm b/ui/accessibility/platform/ax_platform_node_mac.mm index 6a439f162f80af..81677fcf07ce43 100644 --- a/ui/accessibility/platform/ax_platform_node_mac.mm +++ b/ui/accessibility/platform/ax_platform_node_mac.mm @@ -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}, diff --git a/ui/accessibility/platform/ax_platform_node_win.cc b/ui/accessibility/platform/ax_platform_node_win.cc index b3c5d352ff82e7..1173f486f56d85 100644 --- a/ui/accessibility/platform/ax_platform_node_win.cc +++ b/ui/accessibility/platform/ax_platform_node_win.cc @@ -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; diff --git a/ui/views/accessibility/native_view_accessibility_base.cc b/ui/views/accessibility/native_view_accessibility_base.cc index 255cf4704f8883..62455b9afc9ef0 100644 --- a/ui/views/accessibility/native_view_accessibility_base.cc +++ b/ui/views/accessibility/native_view_accessibility_base.cc @@ -7,7 +7,9 @@ #include "ui/views/accessibility/native_view_accessibility_base.h" +#include "base/lazy_instance.h" #include "base/memory/ptr_util.h" +#include "base/threading/thread_task_runner_handle.h" #include "ui/accessibility/platform/ax_platform_node.h" #include "ui/events/event_utils.h" #include "ui/gfx/native_widget_types.h" @@ -22,6 +24,17 @@ namespace { base::LazyInstance>::Leaky g_unique_id_to_ax_platform_node = LAZY_INSTANCE_INITIALIZER; +// Information required to fire a delayed accessibility event. +struct QueuedEvent { + ax::mojom::Event type; + int32_t node_id; +}; + +base::LazyInstance>::Leaky g_event_queue = + LAZY_INSTANCE_INITIALIZER; + +bool g_is_queueing_events = false; + bool IsAccessibilityFocusableWhenEnabled(View* view) { return view->focus_behavior() != View::FocusBehavior::NEVER && view->IsDrawn(); @@ -88,9 +101,49 @@ gfx::NativeViewAccessible NativeViewAccessibilityBase::GetNativeObject() { return ax_node_->GetNativeViewAccessible(); } +ui::AXPlatformNode* PlatformNodeFromNodeID(int32_t id) { + // Note: For Views, node IDs and unique IDs are the same - but that isn't + // necessarily true for all AXPlatformNodes. + auto it = g_unique_id_to_ax_platform_node.Get().find(id); + + if (it == g_unique_id_to_ax_platform_node.Get().end()) + return nullptr; + + return it->second; +} + +void FireEvent(QueuedEvent event) { + ui::AXPlatformNode* node = PlatformNodeFromNodeID(event.node_id); + if (node) + node->NotifyAccessibilityEvent(event.type); +} + +void FlushQueue() { + DCHECK(g_is_queueing_events); + for (QueuedEvent event : g_event_queue.Get()) + FireEvent(event); + g_is_queueing_events = false; + g_event_queue.Get().clear(); +} + void NativeViewAccessibilityBase::NotifyAccessibilityEvent( ax::mojom::Event event_type) { + if (g_is_queueing_events) { + g_event_queue.Get().push_back({event_type, GetUniqueId().Get()}); + return; + } + ax_node_->NotifyAccessibilityEvent(event_type); + + // A focus context event is intended to send a focus event and a delay + // before the next focus event. It makes sense to delay the entire next + // synchronous batch of next events so that ordering remains the same. + if (event_type == ax::mojom::Event::kFocusContext) { + // Begin queueing subsequent events and flush queue asynchronously. + g_is_queueing_events = true; + base::OnceCallback cb = base::BindOnce(&FlushQueue); + base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, std::move(cb)); + } } // ui::AXPlatformNodeDelegate @@ -250,7 +303,7 @@ gfx::NativeViewAccessible NativeViewAccessibilityBase::GetFocus() { View* focused_view = focus_manager ? focus_manager->GetFocusedView() : nullptr; if (fake_focus_view_id_) { - ui::AXPlatformNode* ax_node = GetFromNodeID(fake_focus_view_id_); + ui::AXPlatformNode* ax_node = PlatformNodeFromNodeID(fake_focus_view_id_); if (ax_node) return ax_node->GetNativeViewAccessible(); } @@ -258,14 +311,7 @@ gfx::NativeViewAccessible NativeViewAccessibilityBase::GetFocus() { } ui::AXPlatformNode* NativeViewAccessibilityBase::GetFromNodeID(int32_t id) { - // Note: For Views, node IDs and unique IDs are the same - but that isn't - // necessarily true for all AXPlatformNodes. - auto it = g_unique_id_to_ax_platform_node.Get().find(id); - - if (it == g_unique_id_to_ax_platform_node.Get().end()) - return nullptr; - - return it->second; + return PlatformNodeFromNodeID(id); } int NativeViewAccessibilityBase::GetIndexInParent() const {