Improve app list search box accessibility

Increases the delay for announcing search query changes to give
chromevox more time to announce query changes, and to give search
results more time to stabilize.

Makes sure the selection moves to the search box text field when the
user interacts with the search box (e.g. when they change the query).
This should ensure that the chromevox announces the textfield changes to
the user. Note that this should be done only if the selection has moved
to a search result (otherwise ChromeVox would announce the full
extfield value on each text update). To achieve this,
SearchResultPageView has to notify the SearchBoxView that it changed the
current selection - the cl adds a11y_selection_on_search_result_ member
to the SearchBoxView that is set by SearchResultPageView, and reset by
SearchBoxView when the selection moves back to the textfield.

Removes calls to notify a11y selection changes from search result views.
SearchResultPageView already handles result selection changes and sends
the selection updates, but it also updates the search box state.

Moves the a11y selection to the close button when it's focused.

Updates the search box accessible name when search box is active not to
reference using keys to navigate apps (as there are no apps to navigate
when the search box is active).

BUG=1111579

Change-Id: Ib11bcc9122e7545c7a37ad4ea2a6005459ee6176
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2476903
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#817970}

ash/app_list/views/privacy_info_view.cc

@@ -161,10 +161,8 @@ void PrivacyInfoView::OnKeyEvent(ui::KeyEvent* event) {
 void PrivacyInfoView::SelectInitialResultAction(bool reverse_tab_order) {
   if (!reverse_tab_order) {
     selected_action_ = Action::kTextLink;
-    text_view_->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
   } else {
     selected_action_ = Action::kCloseButton;
-    close_button_->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
   }
 
   // Update visual indicators for focus.
@@ -179,12 +177,10 @@ bool PrivacyInfoView::SelectNextResultAction(bool reverse_tab_order) {
   if (!reverse_tab_order && selected_action_ == Action::kTextLink) {
     // Move selection forward from the text view to the close button.
     selected_action_ = Action::kCloseButton;
-    close_button_->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
     action_changed = true;
   } else if (reverse_tab_order && selected_action_ == Action::kCloseButton) {
     // Move selection backward from the close button to the text view.
     selected_action_ = Action::kTextLink;
-    text_view_->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
     action_changed = true;
   } else {
     selected_action_ = Action::kNone;
@@ -196,17 +192,14 @@ bool PrivacyInfoView::SelectNextResultAction(bool reverse_tab_order) {
   return action_changed;
 }
 
-void PrivacyInfoView::NotifyA11yResultSelected() {
+views::View* PrivacyInfoView::GetSelectedView() {
   switch (selected_action_) {
     case Action::kTextLink:
-      text_view_->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
-      break;
+      return text_view_;
     case Action::kCloseButton:
-      close_button_->NotifyAccessibilityEvent(ax::mojom::Event::kSelection,
-                                              true);
-      break;
+      return close_button_;
     case Action::kNone:
-      break;
+      return this;
   }
 }
 

ash/app_list/views/privacy_info_view.h

@@ -38,7 +38,7 @@ class PrivacyInfoView : public SearchResultBaseView {
   // SearchResultBaseView:
   void SelectInitialResultAction(bool reverse_tab_order) override;
   bool SelectNextResultAction(bool reverse_tab_order) override;
-  void NotifyA11yResultSelected() override;
+  views::View* GetSelectedView() override;
 
   virtual void LinkClicked() = 0;
   virtual void CloseButtonPressed() = 0;

ash/app_list/views/search_box_view.cc

@@ -280,6 +280,17 @@ void SearchBoxView::RecordSearchBoxActivationHistogram(
   }
 }
 
+void SearchBoxView::OnSearchBoxActiveChanged(bool active) {
+  if (active) {
+    search_box()->SetAccessibleName(base::string16());
+  } else {
+    search_box()->SetAccessibleName(l10n_util::GetStringUTF16(
+        is_tablet_mode_
+            ? IDS_APP_LIST_SEARCH_BOX_ACCESSIBILITY_NAME_TABLET
+            : IDS_APP_LIST_SEARCH_BOX_ACCESSIBILITY_NAME_CLAMSHELL));
+  }
+}
+
 void SearchBoxView::OnKeyEvent(ui::KeyEvent* event) {
   app_list_view_->RedirectKeyEventToSearchBox(event);
 
@@ -492,6 +503,13 @@ void SearchBoxView::ClearAutocompleteText() {
   ResetHighlightRange();
 }
 
+void SearchBoxView::OnBeforeUserAction(views::Textfield* sender) {
+  if (a11y_selection_on_search_result_) {
+    a11y_selection_on_search_result_ = false;
+    search_box()->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
+  }
+}
+
 void SearchBoxView::ContentsChanged(views::Textfield* sender,
                                     const base::string16& new_contents) {
   if (IsTrimmedQueryEmpty(current_query_) && !IsSearchBoxTrimmedQueryEmpty()) {
@@ -565,6 +583,7 @@ void SearchBoxView::ClearSearchAndDeactivateSearchBox() {
   contents_view_->search_results_page_view()
       ->result_selection_controller()
       ->ClearSelection();
+  a11y_selection_on_search_result_ = false;
   ClearSearch();
   SetSearchBoxActive(false, ui::ET_UNKNOWN);
 }
@@ -691,6 +710,9 @@ bool SearchBoxView::HandleKeyEvent(views::Textfield* sender,
 
       DCHECK(close_button()->GetVisible());
       close_button()->RequestFocus();
+      close_button()->NotifyAccessibilityEvent(ax::mojom::Event::kSelection,
+                                               true);
+      a11y_selection_on_search_result_ = false;
       break;
     case ResultSelectionController::MoveResult::kResultChanged:
       UpdateSearchBoxTextForSelectedResult(

ash/app_list/views/search_box_view.h

@@ -60,6 +60,7 @@ class APP_LIST_EXPORT SearchBoxView : public SearchBoxViewBase,
   void SetupCloseButton() override;
   void SetupBackButton() override;
   void RecordSearchBoxActivationHistogram(ui::EventType event_type) override;
+  void OnSearchBoxActiveChanged(bool active) override;
 
   // Overridden from views::View:
   void OnKeyEvent(ui::KeyEvent* event) override;
@@ -110,6 +111,10 @@ class APP_LIST_EXPORT SearchBoxView : public SearchBoxViewBase,
   }
   ContentsView* contents_view() { return contents_view_; }
 
+  void set_a11y_selection_on_search_result(bool value) {
+    a11y_selection_on_search_result_ = value;
+  }
+
   void set_highlight_range_for_test(const gfx::Range& range) {
     highlight_range_ = range;
   }
@@ -133,6 +138,7 @@ class APP_LIST_EXPORT SearchBoxView : public SearchBoxViewBase,
   void SetAutocompleteText(const base::string16& autocomplete_text);
 
   // Overridden from views::TextfieldController:
+  void OnBeforeUserAction(views::Textfield* sender) override;
   void ContentsChanged(views::Textfield* sender,
                        const base::string16& new_contents) override;
   bool HandleKeyEvent(views::Textfield* sender,
@@ -175,10 +181,13 @@ class APP_LIST_EXPORT SearchBoxView : public SearchBoxViewBase,
   // True if app list search autocomplete is enabled.
   const bool is_app_list_search_autocomplete_enabled_;
 
-
   // Whether tablet mode is active.
   bool is_tablet_mode_ = false;
 
+  // Set by SearchResultPageView when the accessibility selection moves to a
+  // search result view.
+  bool a11y_selection_on_search_result_ = false;
+
   base::WeakPtrFactory<SearchBoxView> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(SearchBoxView);

ash/app_list/views/search_result_actions_view.cc

@@ -254,16 +254,16 @@ bool SearchResultActionsView::SelectNextAction(bool reverse_tab_order) {
   return true;
 }
 
-void SearchResultActionsView::NotifyA11yResultSelected() {
+views::View* SearchResultActionsView::GetSelectedView() {
   DCHECK(HasSelectedAction());
 
   int selected_action = GetSelectedAction();
   for (views::View* child : children()) {
-    if (static_cast<views::Button*>(child)->tag() == selected_action) {
-      child->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
-      return;
-    }
+    if (static_cast<views::Button*>(child)->tag() == selected_action)
+      return child;
   }
+
+  return nullptr;
 }
 
 void SearchResultActionsView::ClearSelectedAction() {

ash/app_list/views/search_result_actions_view.h

@@ -56,8 +56,8 @@ class APP_LIST_EXPORT SearchResultActionsView : public views::View,
   // getting cleared).
   bool SelectNextAction(bool reverse_tab_order);
 
-  // Sends kSelection a11y notification for the selected action button.
-  void NotifyA11yResultSelected();
+  // Returns the selected action button.
+  views::View* GetSelectedView();
 
   // Clears selected action state.
   void ClearSelectedAction();

ash/app_list/views/search_result_base_view.cc

@@ -62,12 +62,10 @@ bool SearchResultBaseView::SelectNextResultAction(bool reverse_tab_order) {
   return true;
 }
 
-void SearchResultBaseView::NotifyA11yResultSelected() {
-  if (actions_view_ && actions_view_->HasSelectedAction()) {
-    actions_view_->NotifyA11yResultSelected();
-    return;
-  }
-  NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
+views::View* SearchResultBaseView::GetSelectedView() {
+  if (actions_view_ && actions_view_->HasSelectedAction())
+    return actions_view_->GetSelectedView();
+  return this;
 }
 
 void SearchResultBaseView::SetResult(SearchResult* result) {
@@ -112,10 +110,8 @@ void SearchResultBaseView::ClearResult() {
 }
 
 void SearchResultBaseView::SelectInitialResultAction(bool reverse_tab_order) {
-  if (actions_view_ && actions_view_->SelectInitialAction(reverse_tab_order))
-    return;
-
-  NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
+  if (actions_view_)
+    actions_view_->SelectInitialAction(reverse_tab_order);
 }
 
 void SearchResultBaseView::ClearSelectedResultAction() {

ash/app_list/views/search_result_base_view.h

@@ -45,11 +45,10 @@ class APP_LIST_EXPORT SearchResultBaseView : public views::Button,
   // Returns whether the selected result action was changed.
   virtual bool SelectNextResultAction(bool reverse_tab_order);
 
-  // If the search result is currently selected, sends the appropriate
-  // kSelection view accessibility event. For example, if a result action is
-  // selected, the notification will be sent for the selected action button
-  // view.
-  virtual void NotifyA11yResultSelected();
+  // Returns the view that is currently selected - for example, if the result
+  // supports action views and an action view is currently selected, this
+  // should return the action view, otherwise it should return `this`.
+  virtual views::View* GetSelectedView();
 
   SearchResult* result() const { return result_; }
   void SetResult(SearchResult* result);

ash/app_list/views/search_result_list_view.cc

@@ -249,9 +249,6 @@ void SearchResultListView::SearchResultActionActivated(SearchResultView* view,
       view_delegate_->InvokeSearchResultAction(view->result()->id(),
                                                action_index, event_flags);
     } else if (action == OmniBoxZeroStateAction::kAppendSuggestion) {
-      // Make sure ChromeVox will focus on the search box.
-      main_view_->search_box_view()->search_box()->NotifyAccessibilityEvent(
-          ax::mojom::Event::kSelection, true);
       main_view_->search_box_view()->UpdateQuery(view->result()->title());
     }
   }

ash/app_list/views/search_result_page_view.cc

@@ -75,7 +75,7 @@ constexpr int kSearchBoxSearchResultShadowElevation = 12;
 // The amount of time by which notifications to accessibility framework about
 // result page changes are delayed.
 constexpr base::TimeDelta kNotifyA11yDelay =
-    base::TimeDelta::FromMilliseconds(500);
+    base::TimeDelta::FromMilliseconds(1500);
 
 // A container view that ensures the card background and the shadow are painted
 // in the correct order.
@@ -341,6 +341,7 @@ void SearchResultPageView::SetIgnoreResultChangesForA11y(bool ignore) {
   ignore_result_changes_for_a11y_ = ignore;
 
   GetViewAccessibility().OverrideIsLeaf(ignore);
+  GetViewAccessibility().OverrideIsIgnored(ignore);
   NotifyAccessibilityEvent(ax::mojom::Event::kTreeChanged, true);
 }
 
@@ -370,8 +371,20 @@ void SearchResultPageView::NotifySelectedResultChanged() {
     return;
   }
 
+  SearchBoxView* search_box = AppListPage::contents_view()->GetSearchBoxView();
+  // Ignore result selection change if the focus moved away from the search boc
+  // textfield, for example to the close button.
+  if (!search_box->search_box()->HasFocus())
+    return;
+
+  views::View* selected_view =
+      result_selection_controller_->selected_result()->GetSelectedView();
+  if (!selected_view)
+    return;
+
+  selected_view->NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
   NotifyAccessibilityEvent(ax::mojom::Event::kSelectedChildrenChanged, true);
-  result_selection_controller_->selected_result()->NotifyA11yResultSelected();
+  search_box->set_a11y_selection_on_search_result(true);
 }
 
 void SearchResultPageView::OnSearchResultContainerResultsChanging() {

ash/app_list/views/search_result_view.cc

@@ -341,7 +341,6 @@ void SearchResultView::OnGestureEvent(ui::GestureEvent* event) {
       if (actions_view()->IsValidActionIndex(
               OmniBoxZeroStateAction::kRemoveSuggestion)) {
         ScrollRectToVisible(GetLocalBounds());
-        NotifyAccessibilityEvent(ax::mojom::Event::kSelection, true);
         SetSelected(true, base::nullopt);
         confirm_remove_by_long_press_ = true;
         OnSearchResultActionActivated(OmniBoxZeroStateAction::kRemoveSuggestion,

ash/search_box/search_box_view_base.cc

@@ -349,6 +349,7 @@ void SearchBoxViewBase::SetSearchBoxActive(bool active,
   if (event_type != ui::ET_KEY_PRESSED && event_type != ui::ET_KEY_RELEASED)
     UpdateKeyboardVisibility();
   UpdateButtonsVisisbility();
+  OnSearchBoxActiveChanged(active);
 
   NotifyActiveChanged();
 
@@ -438,6 +439,8 @@ void SearchBoxViewBase::ClearSearch() {
   NotifyQueryChanged();
 }
 
+void SearchBoxViewBase::OnSearchBoxActiveChanged(bool active) {}
+
 void SearchBoxViewBase::NotifyQueryChanged() {
   DCHECK(delegate_);
   delegate_->QueryChanged(this);

ash/search_box/search_box_view_base.h

@@ -118,6 +118,9 @@ class SearchBoxViewBase : public views::WidgetDelegateView,
 
   virtual void ClearSearch();
 
+  // Called when the search box active state changes.
+  virtual void OnSearchBoxActiveChanged(bool active);
+
  protected:
   // Fires query change notification.
   void NotifyQueryChanged();