Converts from a tree to a text position if the selection is not inside a text object

This patch fixes screen reader announcements when moving by character or by line in editable fields,
either through a hard line break <br>, or through a blank line (<br><br>).

A tree position is returned by the AX selection API when either the anchor offset or the focus offset
refers to a child index and not to a character offset.

We were not handling tree positions on Mac at all,
whilst on Windows IA2 and Linux ATK we were not handling them if
any of the caret or selection APIs were called on the anchor or the focus of the selection
and not on one of their ancestors, e.g., calling IAccessibleText::get_caretOffset on a content editable
that contains an img element as a direct child, and the caret being placed before the image.

Tree positions were correctly handled on Windows IA2 and Linux ATK if the selection endpoints were inside a descendant of the current object,
or in a completely different part of the tree.

Bug: 951777
R=aleventhal@chromium.org

Change-Id: Iac3c673c4c15668367f379d979d3f2457c9fefb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1699017Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Auto-Submit: Nektarios Paisios <nektar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#678293}

content/browser/accessibility/browser_accessibility_cocoa.mm

@@ -231,10 +231,27 @@ AXPlatformRange CreateRangeFromTextMarkerRange(
   return AXPlatformRange(std::move(anchor), std::move(focus));
 }
 
+BrowserAccessibilityPositionInstance CreateTreePosition(
+    const BrowserAccessibility& object,
+    int offset) {
+  // A tree position is one for which the |offset| argument refers to a child
+  // index instead of a character offset inside a text object.
+  if (!object.instance_active())
+    return BrowserAccessibilityPosition::CreateNullPosition();
+
+  const BrowserAccessibilityManager* manager = object.manager();
+  DCHECK(manager);
+  return BrowserAccessibilityPosition::CreateTreePosition(
+      manager->ax_tree_id(), object.GetId(), offset);
+}
+
 BrowserAccessibilityPositionInstance CreateTextPosition(
     const BrowserAccessibility& object,
     int offset,
     ax::mojom::TextAffinity affinity) {
+  // A text position is one for which the |offset| argument refers to a
+  // character offset inside a text object. As such, text positions are only
+  // valid on platform leaf objects, e.g. static text nodes and text fields.
   if (!object.instance_active())
     return BrowserAccessibilityPosition::CreateNullPosition();
 
@@ -244,16 +261,20 @@ BrowserAccessibilityPositionInstance CreateTextPosition(
       manager->ax_tree_id(), object.GetId(), offset, affinity);
 }
 
-AXPlatformRange CreateTextRange(const BrowserAccessibility& start_object,
-                                int start_offset,
-                                ax::mojom::TextAffinity start_affinity,
-                                const BrowserAccessibility& end_object,
-                                int end_offset,
-                                ax::mojom::TextAffinity end_affinity) {
+AXPlatformRange CreateAXPlatformRange(const BrowserAccessibility& start_object,
+                                      int start_offset,
+                                      ax::mojom::TextAffinity start_affinity,
+                                      const BrowserAccessibility& end_object,
+                                      int end_offset,
+                                      ax::mojom::TextAffinity end_affinity) {
   BrowserAccessibilityPositionInstance anchor =
-      CreateTextPosition(start_object, start_offset, start_affinity);
+      start_object.IsTextOnlyObject()
+          ? CreateTextPosition(start_object, start_offset, start_affinity)
+          : CreateTreePosition(start_object, start_offset);
   BrowserAccessibilityPositionInstance focus =
-      CreateTextPosition(end_object, end_offset, end_affinity);
+      end_object.IsTextOnlyObject()
+          ? CreateTextPosition(end_object, end_offset, end_affinity)
+          : CreateTreePosition(end_object, end_offset);
   // |AXPlatformRange| takes ownership of its anchor and focus.
   return AXPlatformRange(std::move(anchor), std::move(focus));
 }
@@ -2026,6 +2047,9 @@ NSString* const NSAccessibilityRequiredAttributeChrome = @"AXRequired";
   if (!focusObject)
     return nil;
 
+  // |anchorOffset| and / or |focusOffset| refer to a character offset if
+  // |anchorObject| / |focusObject| are text-only objects. Otherwise, they
+  // should be treated as child indices.
   int anchorOffset = manager->GetTreeData().sel_anchor_offset;
   int focusOffset = manager->GetTreeData().sel_focus_offset;
   if (anchorOffset < 0 || focusOffset < 0)
@@ -2036,9 +2060,9 @@ NSString* const NSAccessibilityRequiredAttributeChrome = @"AXRequired";
   ax::mojom::TextAffinity focusAffinity =
       manager->GetTreeData().sel_focus_affinity;
 
-  return CreateTextMarkerRange(CreateTextRange(*anchorObject, anchorOffset,
-                                               anchorAffinity, *focusObject,
-                                               focusOffset, focusAffinity));
+  return CreateTextMarkerRange(
+      CreateAXPlatformRange(*anchorObject, anchorOffset, anchorAffinity,
+                            *focusObject, focusOffset, focusAffinity));
 }
 
 - (NSValue*)size {

ui/accessibility/platform/ax_platform_node_base.cc

@@ -1330,20 +1330,43 @@ int AXPlatformNodeBase::GetHypertextOffsetFromEndpoint(
     AXPlatformNodeBase* endpoint_object,
     int endpoint_offset) {
   // There are three cases:
-  // 1. Either the selection endpoint is inside this object or is an ancestor
-  // of of this object. endpoint_offset should be returned.
-  // 2. The selection endpoint is a pure descendant of this object. The offset
-  // of the character corresponding to the subtree in which the endpoint is
-  // located should be returned.
+  // 1. The selection endpoint is inside this object but not one of its
+  // descendants, or is in an ancestor of this object. endpoint_offset should be
+  // returned, possibly adjusted from a child offset to a hypertext offset.
+  // 2. The selection endpoint is a descendant of this object. The offset of the
+  // character in this object's hypertext corresponding to the subtree in which
+  // the endpoint is located should be returned.
   // 3. The selection endpoint is in a completely different part of the tree.
-  // Either 0 or text_length should be returned depending on the direction
+  // Either 0 or hypertext length should be returned depending on the direction
   // that one needs to travel to find the endpoint.
+  //
+  // TODO(nektar): Replace all this logic with the use of AXNodePosition.
 
-  // Case 1.
+  // Case 1. Is the endpoint object equal to this object or an ancestor of this
+  // object?
   //
   // IsDescendantOf includes the case when endpoint_object == this.
-  if (IsDescendantOf(endpoint_object))
-    return endpoint_offset;
+  if (IsDescendantOf(endpoint_object)) {
+    if (endpoint_object->IsLeaf()) {
+      DCHECK_EQ(endpoint_object, this) << "Text objects cannot have children.";
+      return endpoint_offset;
+    } else {
+      DCHECK_GE(endpoint_offset, 0);
+      DCHECK_LE(endpoint_offset,
+                endpoint_object->GetDelegate()->GetChildCount());
+
+      // Adjust the |endpoint_offset| because the selection endpoint is a tree
+      // position, i.e. it represents a child index and not a text offset.
+      if (endpoint_offset == endpoint_object->GetDelegate()->GetChildCount()) {
+        return static_cast<int>(endpoint_object->GetHypertext().size());
+      } else {
+        auto* child = static_cast<AXPlatformNodeBase*>(FromNativeViewAccessible(
+            endpoint_object->GetDelegate()->ChildAtIndex(endpoint_offset)));
+        DCHECK(child);
+        return endpoint_object->GetHypertextOffsetFromChild(child);
+      }
+    }
+  }
 
   AXPlatformNodeBase* common_parent = this;
   int32_t index_in_common_parent = GetDelegate()->GetIndexInParent();
@@ -1358,24 +1381,28 @@ int AXPlatformNodeBase::GetHypertextOffsetFromEndpoint(
   DCHECK_GE(index_in_common_parent, 0);
   DCHECK(!(common_parent->IsTextOnlyObject()));
 
-  // Case 2.
+  // Case 2. Is the selection endpoint inside a descendant of this object?
   //
-  // We already checked in case 1 if our endpoint is inside this object.
-  // We can safely assume that it is a descendant or in a completely different
-  // part of the tree.
+  // We already checked in case 1 if our endpoint object is equal to this
+  // object. We can safely assume that it is a descendant or in a completely
+  // different part of the tree.
   if (common_parent == this) {
     int32_t hypertext_offset =
         GetHypertextOffsetFromDescendant(endpoint_object);
     auto* parent = static_cast<AXPlatformNodeBase*>(
         FromNativeViewAccessible(endpoint_object->GetParent()));
     if (parent == this && endpoint_object->IsTextOnlyObject()) {
+      // Due to a historical design decision, the hypertext of the immediate
+      // parents of text objects includes all their text. We therefore need to
+      // adjust the hypertext offset in the parent by adding any text offset.
       hypertext_offset += endpoint_offset;
     }
 
     return hypertext_offset;
   }
 
-  // Case 3.
+  // Case 3. Is the selection endpoint in a completely different part of the
+  // tree?
   //
   // We can safely assume that the endpoint is in another part of the tree or
   // at common parent, and that this object is a descendant of common parent.