Omnibox: provide "n of m" position info with screen reader search suggestions

The Omnibox provides suggestion text that is friendly to screen reader users.
However, it is not possible for users to know where they are in the overall
list. Normally, screen reader lists provide "n of m" positional info at the
end of each item. Because we are using a bit of a hack so that focus
can stay in the edit box, we need to provide this info ourselves. The
downside of this approach is that the text will not show up as the shorter n/m
on Braille displays, but it's really a minor downside compared with the benefit.

Bug: 792954
Change-Id: I3b69bc92ed42c59521ddf80cf932323388f650ab
Reviewed-on: https://chromium-review.googlesource.com/817675
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: Justin Donnelly <jdonnelly@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523503}

chrome/browser/ui/cocoa/omnibox/omnibox_view_mac.mm

@@ -614,7 +614,9 @@ void OmniboxViewMac::OnTemporaryTextMaybeChanged(
 
   // Get friendly accessibility label.
   AnnounceAutocompleteForScreenReader(
-      AutocompleteMatchType::ToAccessibilityLabel(match, display_text));
+      AutocompleteMatchType::ToAccessibilityLabel(
+          match, display_text, model()->popup_model()->selected_line(),
+          model()->result().size()));
 }
 
 bool OmniboxViewMac::OnInlineAutocompleteTextMaybeChanged(

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

@@ -484,7 +484,8 @@ void OmniboxViewViews::OnTemporaryTextMaybeChanged(
 
   // Get friendly accessibility label.
   friendly_suggestion_text_ = AutocompleteMatchType::ToAccessibilityLabel(
-      match, display_text, &friendly_suggestion_text_prefix_length_);
+      match, display_text, model()->popup_model()->selected_line(),
+      model()->result().size(), &friendly_suggestion_text_prefix_length_);
 
   SetWindowTextAndCaretPos(display_text, display_text.length(), false,
                            notify_text_changed);

chrome/browser/ui/views/omnibox/omnibox_view_views_browsertest.cc

@@ -28,6 +28,7 @@
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/base/clipboard/scoped_clipboard_writer.h"
 #include "ui/base/ime/input_method.h"
+#include "ui/base/ime/text_edit_commands.h"
 #include "ui/base/ime/text_input_client.h"
 #include "ui/base/test/ui_controls.h"
 #include "ui/base/ui_base_switches.h"
@@ -474,20 +475,38 @@ IN_PROC_BROWSER_TEST_F(OmniboxViewViewsTest, FriendlyAccessibleLabel) {
   match.description = base::ASCIIToUTF16("Google");
   match.allowed_to_be_default_match = true;
 
+  // Populate suggestions for the omnibox popup.
+  AutocompleteController* autocomplete_controller =
+      omnibox_view->model()->popup_model()->autocomplete_controller();
+  AutocompleteResult& results = autocomplete_controller->result_;
+  ACMatches matches;
+  matches.push_back(match);
+  AutocompleteInput input(base::ASCIIToUTF16("g"),
+                          metrics::OmniboxEventProto::OTHER,
+                          TestSchemeClassifier());
+  results.AppendMatches(input, matches);
+  results.SortAndCull(
+      input, TemplateURLServiceFactory::GetForProfile(browser()->profile()));
+
+  // The omnibox popup should open with suggestions displayed.
   chrome::FocusLocationBar(browser());
+  omnibox_view->model()->popup_model()->OnResultChanged();
+  EXPECT_TRUE(omnibox_view->model()->popup_model()->IsOpen());
   OmniboxViewViews* omnibox_view_views =
       static_cast<OmniboxViewViews*>(omnibox_view);
-  omnibox_view_views->SetText(match_url);
+
+  // Ensure that the friendly accessibility label is updated.
   omnibox_view_views->OnTemporaryTextMaybeChanged(match_url, match, false,
                                                   false);
   omnibox_view->SelectAll(true);
 
+  // Test friendly label.
   const int kFriendlyPrefixLength = match.description.size() + 1;
   ui::AXNodeData node_data;
   omnibox_view_views->GetAccessibleNodeData(&node_data);
-  EXPECT_EQ(
-      base::ASCIIToUTF16("Google https://google.com location from history"),
-      node_data.GetString16Attribute(ui::AX_ATTR_VALUE));
+  EXPECT_EQ(base::ASCIIToUTF16(
+                "Google https://google.com location from history, 1 of 1"),
+            node_data.GetString16Attribute(ui::AX_ATTR_VALUE));
   // Selection offsets are moved over by length the inserted descriptive text
   // prefix ("Google") + 1 for the space.
   EXPECT_EQ(kFriendlyPrefixLength,

components/omnibox/browser/autocomplete_controller.h

@@ -146,6 +146,7 @@ class AutocompleteController : public AutocompleteProviderListener {
   FRIEND_TEST_ALL_PREFIXES(AutocompleteProviderTest, UpdateAssistedQueryStats);
   FRIEND_TEST_ALL_PREFIXES(OmniboxViewTest, DoesNotUpdateAutocompleteOnBlur);
   FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsTest, CloseOmniboxPopupOnTextDrag);
+  FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsTest, FriendlyAccessibleLabel);
   FRIEND_TEST_ALL_PREFIXES(OmniboxViewViewsTest, MaintainCursorAfterFocusCycle);
   FRIEND_TEST_ALL_PREFIXES(OmniboxPopupModelTest, SetSelectedLine);
 

components/omnibox/browser/autocomplete_match_type.cc

@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/omnibox/browser/autocomplete_match.h"
 #include "components/omnibox/browser/suggestion_answer.h"
@@ -57,10 +58,9 @@ static int AccessibilityLabelPrefixLength(base::string16 accessibility_label) {
   return length == base::string16::npos ? 0 : static_cast<int>(length);
 }
 
-base::string16 AutocompleteMatchType::ToAccessibilityLabel(
-    const AutocompleteMatch& match,
-    const base::string16& match_text,
-    int* label_prefix_length) {
+static base::string16 ToAccessibilityLabelImpl(const AutocompleteMatch& match,
+                                               const base::string16& match_text,
+                                               int* label_prefix_length) {
   // Types with a message ID of zero get |text| returned as-is.
   static constexpr int message_ids[] = {
       0,                             // URL_WHAT_YOU_TYPED
@@ -152,3 +152,17 @@ base::string16 AutocompleteMatchType::ToAccessibilityLabel(
              ? l10n_util::GetStringFUTF16(message, match_text, description)
              : l10n_util::GetStringFUTF16(message, match_text);
 }
+
+base::string16 AutocompleteMatchType::ToAccessibilityLabel(
+    const AutocompleteMatch& match,
+    const base::string16& match_text,
+    size_t match_index,
+    size_t total_matches,
+    int* label_prefix_length) {
+  base::string16 result =
+      ToAccessibilityLabelImpl(match, match_text, label_prefix_length);
+
+  return l10n_util::GetStringFUTF16(IDS_ACC_AUTOCOMPLETE_N_OF_M, result,
+                                    base::IntToString16(match_index + 1),
+                                    base::IntToString16(total_matches));
+}

components/omnibox/browser/autocomplete_match_type.h

@@ -76,6 +76,8 @@ struct AutocompleteMatchType {
   static base::string16 ToAccessibilityLabel(
       const AutocompleteMatch& match,
       const base::string16& match_text,
+      size_t match_index,
+      size_t total_matches,
       int* label_prefix_length = nullptr);
 };
 

components/omnibox/browser/autocomplete_match_type_unittest.cc

@@ -20,14 +20,14 @@ TEST(AutocompleteMatchTypeTest, AccessibilityLabelHistory) {
   AutocompleteMatch match;
   match.type = AutocompleteMatchType::URL_WHAT_YOU_TYPED;
   match.description = kTestTitle;
-  EXPECT_EQ(kTestUrl,
-            AutocompleteMatchType::ToAccessibilityLabel(match, kTestUrl));
+  EXPECT_EQ(kTestUrl + base::UTF8ToUTF16(", 2 of 9"),
+            AutocompleteMatchType::ToAccessibilityLabel(match, kTestUrl, 1, 9));
 
   // Decorated with title and match type.
   match.type = AutocompleteMatchType::HISTORY_URL;
   EXPECT_EQ(kTestTitle + base::UTF8ToUTF16(" ") + kTestUrl +
-                base::UTF8ToUTF16(" location from history"),
-            AutocompleteMatchType::ToAccessibilityLabel(match, kTestUrl));
+                base::UTF8ToUTF16(" location from history, 2 of 3"),
+            AutocompleteMatchType::ToAccessibilityLabel(match, kTestUrl, 1, 3));
 }
 
 TEST(AutocompleteMatchTypeTest, AccessibilityLabelSearch) {
@@ -37,8 +37,8 @@ TEST(AutocompleteMatchTypeTest, AccessibilityLabelSearch) {
   AutocompleteMatch match;
   match.type = AutocompleteMatchType::SEARCH_WHAT_YOU_TYPED;
   match.description = kSearchDesc;
-  EXPECT_EQ(kSearch + base::UTF8ToUTF16(" search"),
-            AutocompleteMatchType::ToAccessibilityLabel(match, kSearch));
+  EXPECT_EQ(kSearch + base::UTF8ToUTF16(" search, 6 of 8"),
+            AutocompleteMatchType::ToAccessibilityLabel(match, kSearch, 5, 8));
 }
 
 namespace {
@@ -68,7 +68,7 @@ TEST(AutocompleteMatchTypeTest, AccessibilityLabelAnswer) {
       "5 }] } }] }";
   match.answer = ParseAnswer(answer_json);
 
-  EXPECT_EQ(
-      kSearch + base::UTF8ToUTF16(", answer, sunny with a chance of hail"),
-      AutocompleteMatchType::ToAccessibilityLabel(match, kSearch));
+  EXPECT_EQ(kSearch + base::UTF8ToUTF16(
+                          ", answer, sunny with a chance of hail, 4 of 6"),
+            AutocompleteMatchType::ToAccessibilityLabel(match, kSearch, 3, 6));
 }

components/omnibox_strings.grdp

@@ -77,4 +77,8 @@
   <message name="IDS_ACC_AUTOCOMPLETE_CLIPBOARD" desc="Text for screenreaders describing a URL from a clipboard.">
     <ph name="LOCATION_TITLE">$2<ex>The Chromium Projects</ex></ph> <ph name="SHORT_URL">$1<ex>www.chromium.org</ex> location from clipboard</ph>
   </message>
+  <!-- Supplement accessibility label with item position, e.g. "1 of 3" -->
+  <message name="IDS_ACC_AUTOCOMPLETE_N_OF_M" desc="Text for screenreaders describing the current matche's position in the list of suggestions.">
+    <ph name="FRIENDLY_MATCH_TEXT">$1<ex>The Chromium Projects http://www.chromium.org bookmark</ex></ph>, <ph name="MATCH_POSITION">$2<ex>2</ex></ph> of <ph name="NUM_MATCHES">$3<ex>3</ex></ph>
+  </message>
 </grit-part>