IAccessibleText: Made content editables return embedded object information and made all objects include any widespace.

BUG=491027
R=dmazzoni@chromium.org,aboxhall@chromium.org

Review URL: https://codereview.chromium.org/1223153002

Cr-Commit-Position: refs/heads/master@{#342700}

6 files changed, 49 insertions, 23 deletions

diff --git a/content/browser/accessibility/accessibility_tree_formatter_win.cc b/content/browser/accessibility/accessibility_tree_formatter_win.cc
index 3df2daf..2f75968 100644
--- a/content/browser/accessibility/accessibility_tree_formatter_win.cc
+++ b/content/browser/accessibility/accessibility_tree_formatter_win.cc
@@ -93,15 +93,15 @@ void AccessibilityTreeFormatter::AddProperties(
   IAccessibleStateToStringVector(ia_state, &state_strings);
   IAccessible2StateToStringVector(ax_object->ia2_state(), &state_strings);
   base::ListValue* states = new base::ListValue;
-  for (auto it = state_strings.begin(); it != state_strings.end(); ++it)
-    states->AppendString(base::UTF16ToUTF8(*it));
+  for (const auto& state_string : state_strings)
+    states->AppendString(base::UTF16ToUTF8(state_string));
   dict->Set("states", states);
 
   const std::vector<base::string16>& ia2_attributes =
       ax_object->ia2_attributes();
   base::ListValue* attributes = new base::ListValue;
-  for (auto it = ia2_attributes.begin(); it != ia2_attributes.end(); ++it)
-    attributes->AppendString(base::UTF16ToUTF8(*it));
+  for (const auto& ia2_attribute : ia2_attributes)
+    attributes->AppendString(base::UTF16ToUTF8(ia2_attribute));
   dict->Set("attributes", attributes);
 
   dict->SetString("role_name", ax_object->role_name());
diff --git a/content/browser/accessibility/browser_accessibility.cc b/content/browser/accessibility/browser_accessibility.cc
index b43f7c5..98cab7b 100644
--- a/content/browser/accessibility/browser_accessibility.cc
+++ b/content/browser/accessibility/browser_accessibility.cc
@@ -89,6 +89,11 @@ bool BrowserAccessibility::IsDescendantOf(
   return false;
 }
 
+bool BrowserAccessibility::IsTextOnlyObject() const {
+  return GetRole() == ui::AX_ROLE_STATIC_TEXT ||
+      GetRole() == ui::AX_ROLE_LINE_BREAK;
+}
+
 BrowserAccessibility* BrowserAccessibility::PlatformGetChild(
     uint32 child_index) const {
   DCHECK(child_index < PlatformChildCount());
diff --git a/content/browser/accessibility/browser_accessibility.h b/content/browser/accessibility/browser_accessibility.h
index 3193dd84..d23fc4c 100644
--- a/content/browser/accessibility/browser_accessibility.h
+++ b/content/browser/accessibility/browser_accessibility.h
@@ -69,6 +69,9 @@ class CONTENT_EXPORT BrowserAccessibility {
   // Return true if this object is equal to or a descendant of |ancestor|.
   bool IsDescendantOf(const BrowserAccessibility* ancestor) const;
 
+  // Returns true if this object is used only for representing text.
+  bool IsTextOnlyObject() const;
+
   // Returns true if this is a leaf node on this platform, meaning any
   // children should not be exposed to this platform's native accessibility
   // layer. Each platform subclass should implement this itself.
diff --git a/content/browser/accessibility/browser_accessibility_win.cc b/content/browser/accessibility/browser_accessibility_win.cc
index 54c942e..57e5e56 100644
--- a/content/browser/accessibility/browser_accessibility_win.cc
+++ b/content/browser/accessibility/browser_accessibility_win.cc
@@ -3425,15 +3425,21 @@ void BrowserAccessibilityWin::UpdateStep1ComputeWinAttributes() {
 }
 
 void BrowserAccessibilityWin::UpdateStep2ComputeHypertext() {
+  if (!PlatformChildCount()) {
+    win_attributes_->hypertext += name();
+    return;
+  }
+
   // Construct the hypertext for this node, which contains the concatenation
-  // of all of the static text of this node's children and an embedded object
-  // character for all non-static-text children. Build up a map from the
-  // character index of each embedded object character to the id of the
+  // of all of the static text and widespace of this node's children and an
+  // embedded object character for all the other children. Build up a map from
+  // the character index of each embedded object character to the id of the
   // child object it points to.
   for (unsigned int i = 0; i < PlatformChildCount(); ++i) {
     BrowserAccessibilityWin* child =
         PlatformGetChild(i)->ToBrowserAccessibilityWin();
-    if (child->GetRole() == ui::AX_ROLE_STATIC_TEXT) {
+    DCHECK(child);
+    if (child->IsTextOnlyObject()) {
       win_attributes_->hypertext += child->name();
     } else {
       int32 char_offset = hypertext().size();
@@ -3523,8 +3529,7 @@ void BrowserAccessibilityWin::UpdateStep3FireEvents(bool is_subtree_creation) {
     // Changing a static text node can affect the IAccessibleText hypertext
     // of the parent node, so force an update on the parent.
     BrowserAccessibilityWin* parent = GetParent()->ToBrowserAccessibilityWin();
-    if (parent &&
-        GetRole() == ui::AX_ROLE_STATIC_TEXT &&
+    if (parent && IsTextOnlyObject() &&
         name() != old_win_attributes_->name) {
       parent->UpdateStep1ComputeWinAttributes();
       parent->UpdateStep2ComputeHypertext();
@@ -3696,10 +3701,12 @@ int BrowserAccessibilityWin::GetSelectionAnchor() const {
   if (!anchor_object)
     return -1;
 
-  if (anchor_object->IsDescendantOf(this))
-    return GetHypertextOffsetFromDescendant(*anchor_object);
-
-  if (IsDescendantOf(anchor_object)) {
+  // Includes the case when anchor_object == this.
+  if (IsDescendantOf(anchor_object) ||
+      // Text only objects that are direct descendants should behave as if they
+      // are part of this object when computing hypertext.
+      (anchor_object->GetParent() == this &&
+      anchor_object->IsTextOnlyObject())) {
     int anchor_offset;
     if (!root->GetIntAttribute(ui::AX_ATTR_ANCHOR_OFFSET, &anchor_offset))
       return -1;
@@ -3707,6 +3714,9 @@ int BrowserAccessibilityWin::GetSelectionAnchor() const {
     return anchor_offset;
   }
 
+  if (anchor_object->IsDescendantOf(this))
+    return GetHypertextOffsetFromDescendant(*anchor_object);
+
   return -1;
 }
 
@@ -3721,10 +3731,11 @@ int BrowserAccessibilityWin::GetSelectionFocus() const {
   if (!focus_object)
     return -1;
 
-  if (focus_object->IsDescendantOf(this))
-    return GetHypertextOffsetFromDescendant(*focus_object);
-
-  if (IsDescendantOf(focus_object)) {
+  // Includes the case when focus_object == this.
+  if (IsDescendantOf(focus_object) ||
+      // Text only objects that are direct descendants should behave as if they
+      // are part of this object when computing hypertext.
+      (focus_object->GetParent() == this && focus_object->IsTextOnlyObject())) {
     int focus_offset;
     if (!root->GetIntAttribute(ui::AX_ATTR_FOCUS_OFFSET, &focus_offset))
       return -1;
@@ -3732,6 +3743,9 @@ int BrowserAccessibilityWin::GetSelectionFocus() const {
     return focus_offset;
   }
 
+  if (focus_object->IsDescendantOf(this))
+    return GetHypertextOffsetFromDescendant(*focus_object);
+
   return -1;
 }
 
@@ -3782,9 +3796,13 @@ base::string16 BrowserAccessibilityWin::GetValueText() {
 }
 
 base::string16 BrowserAccessibilityWin::TextForIAccessibleText() {
-  if (IsEditableText() || GetRole() == ui::AX_ROLE_MENU_LIST_OPTION)
-    return value();
-  return (GetRole() == ui::AX_ROLE_STATIC_TEXT) ? name() : hypertext();
+  switch (GetRole()) {
+    case ui::AX_ROLE_TEXT_FIELD:
+    case ui::AX_ROLE_MENU_LIST_OPTION:
+      return value();
+    default:
+      return hypertext();
+  }
 }
 
 bool BrowserAccessibilityWin::IsSameHypertextCharacter(size_t old_char_index,
diff --git a/content/browser/accessibility/browser_accessibility_win_unittest.cc b/content/browser/accessibility/browser_accessibility_win_unittest.cc
index 6cf3de3..e37a0d8d 100644
--- a/content/browser/accessibility/browser_accessibility_win_unittest.cc
+++ b/content/browser/accessibility/browser_accessibility_win_unittest.cc
@@ -916,7 +916,7 @@ TEST_F(BrowserAccessibilityTest, TestCaretAndSelectionInSimpleFields) {
   ASSERT_EQ(0, CountedBrowserAccessibility::num_instances());
 }
 
-TEST_F(BrowserAccessibilityTest, DISABLED_TestCaretInContentEditables) {
+TEST_F(BrowserAccessibilityTest, TestCaretInContentEditables) {
   ui::AXNodeData root;
   root.id = 1;
   root.role = ui::AX_ROLE_ROOT_WEB_AREA;
diff --git a/content/browser/accessibility/dump_accessibility_events_browsertest.cc b/content/browser/accessibility/dump_accessibility_events_browsertest.cc
index 304fc07..581d519 100644
--- a/content/browser/accessibility/dump_accessibility_events_browsertest.cc
+++ b/content/browser/accessibility/dump_accessibility_events_browsertest.cc
@@ -264,7 +264,7 @@ IN_PROC_BROWSER_TEST_F(DumpAccessibilityEventsTest,
 }
 
 IN_PROC_BROWSER_TEST_F(DumpAccessibilityEventsTest,
-                       AccessibilityEventsTextChanged) {
+                       DISABLED_AccessibilityEventsTextChanged) {
   RunEventTest(FILE_PATH_LITERAL("text-changed.html"));
 }