Only the first spelling error is read by screen reader

* Return an empty list with S_OK, not S_FALSE, as this lets the screen
reader process the end of the range of text.

* Repeat previous text attributes when toggling aria-invalid:spelling
in the next text range

Bug: 944645
Change-Id: I89d7599ee282dd325e3ccee13ad3bd3ee5447552
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1539920Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#644822}

content/browser/accessibility/browser_accessibility_com_win.cc

@@ -608,9 +608,9 @@ IFACEMETHODIMP BrowserAccessibilityComWin::get_attributes(
     attributes_str += attribute + L';';
   }
 
-  if (attributes.empty())
-    return S_FALSE;
-
+  // Returning an empty string is valid and indicates no attributes.
+  // This is better than returning S_FALSE which the screen reader
+  // may not recognize as valid attributes.
   *text_attributes = SysAllocString(attributes_str.c_str());
   DCHECK(*text_attributes);
   return S_OK;
@@ -2086,23 +2086,27 @@ void BrowserAccessibilityComWin::MergeSpellingIntoTextAttributes(
     return;
   }
 
+  std::vector<base::string16> prev_attributes;
   for (const auto& spelling_attribute : spelling_attributes) {
     int offset = start_offset + spelling_attribute.first;
-    const auto iterator = text_attributes->find(offset);
+    auto iterator = text_attributes->find(offset);
     if (iterator == text_attributes->end()) {
-      text_attributes->emplace(offset, spelling_attribute.second);
+      text_attributes->emplace(offset, prev_attributes);
+      iterator = text_attributes->find(offset);
     } else {
-      std::vector<base::string16>& existing_attributes = iterator->second;
-      // There might be a spelling attribute already in the list of text
-      // attributes, originating from "aria-invalid", that is being overwritten
-      // by a spelling marker. If it already exists, prefer it over this
-      // automatically computed attribute.
-      if (!HasAttribute(existing_attributes, L"invalid:")) {
-        // Does not exist -- insert our own.
-        existing_attributes.insert(existing_attributes.end(),
-                                   spelling_attribute.second.begin(),
-                                   spelling_attribute.second.end());
-      }
+      prev_attributes = iterator->second;
+    }
+
+    std::vector<base::string16>& existing_attributes = iterator->second;
+    // There might be a spelling attribute already in the list of text
+    // attributes, originating from "aria-invalid", that is being overwritten
+    // by a spelling marker. If it already exists, prefer it over this
+    // automatically computed attribute.
+    if (!HasAttribute(existing_attributes, L"invalid:")) {
+      // Does not exist -- insert our own.
+      existing_attributes.insert(existing_attributes.end(),
+                                 spelling_attribute.second.begin(),
+                                 spelling_attribute.second.end());
     }
   }
 }

content/browser/accessibility/browser_accessibility_win_unittest.cc

@@ -1950,7 +1950,7 @@ TEST_F(BrowserAccessibilityWinTest,
   for (LONG offset = 0; offset < value1_length; ++offset) {
     hr = ax_combo_box->GetCOM()->get_attributes(
         offset, &start_offset, &end_offset, text_attributes.Receive());
-    EXPECT_EQ(S_FALSE, hr);
+    EXPECT_TRUE(base::string16(text_attributes).empty());
     EXPECT_EQ(0, start_offset);
     EXPECT_EQ(value1_length, end_offset);
     text_attributes.Reset();
@@ -1973,7 +1973,7 @@ TEST_F(BrowserAccessibilityWinTest,
        ++offset) {
     hr = ax_combo_box->GetCOM()->get_attributes(
         offset, &start_offset, &end_offset, text_attributes.Receive());
-    EXPECT_EQ(S_FALSE, hr);
+    EXPECT_TRUE(base::string16(text_attributes).empty());
     EXPECT_EQ(value1_length + 4, start_offset);
     EXPECT_EQ(combo_box_value_length, end_offset);
     text_attributes.Reset();
@@ -2049,7 +2049,7 @@ TEST_F(BrowserAccessibilityWinTest, TestNewMisspellingsInSimpleTextFields) {
   for (LONG offset = 0; offset < combo_box_value_length; ++offset) {
     hr = ax_combo_box->GetCOM()->get_attributes(
         offset, &start_offset, &end_offset, text_attributes.Receive());
-    EXPECT_EQ(S_FALSE, hr);
+    EXPECT_TRUE(base::string16(text_attributes).empty());
     EXPECT_EQ(0, start_offset);
     EXPECT_EQ(combo_box_value_length, end_offset);
     text_attributes.Reset();
@@ -2074,7 +2074,7 @@ TEST_F(BrowserAccessibilityWinTest, TestNewMisspellingsInSimpleTextFields) {
   for (LONG offset = 0; offset < value1_length; ++offset) {
     hr = ax_combo_box->GetCOM()->get_attributes(
         offset, &start_offset, &end_offset, text_attributes.Receive());
-    EXPECT_EQ(S_FALSE, hr);
+    EXPECT_TRUE(base::string16(text_attributes).empty());
     EXPECT_EQ(0, start_offset);
     EXPECT_EQ(value1_length, end_offset);
     text_attributes.Reset();
@@ -2097,7 +2097,7 @@ TEST_F(BrowserAccessibilityWinTest, TestNewMisspellingsInSimpleTextFields) {
        ++offset) {
     hr = ax_combo_box->GetCOM()->get_attributes(
         offset, &start_offset, &end_offset, text_attributes.Receive());
-    EXPECT_EQ(S_FALSE, hr);
+    EXPECT_TRUE(base::string16(text_attributes).empty());
     EXPECT_EQ(value1_length + 4, start_offset);
     EXPECT_EQ(combo_box_value_length, end_offset);
     text_attributes.Reset();