Fixes for contenteditable caret and selection handling in Windows.

content/browser/accessibility/browser_accessibility_win.cc

Index: content/browser/accessibility/browser_accessibility_win.cc
diff --git a/content/browser/accessibility/browser_accessibility_win.cc b/content/browser/accessibility/browser_accessibility_win.cc
index 74e7f11e9007eb663a1439485e688d45cbe89219..eb4fa5204e736cc1ff1c140a4e2410d346175951 100644
--- a/content/browser/accessibility/browser_accessibility_win.cc
+++ b/content/browser/accessibility/browser_accessibility_win.cc
@@ -2064,7 +2064,11 @@ STDMETHODIMP BrowserAccessibilityWin::get_nSelections(LONG* n_selections) {
   *n_selections = 0;
   int selection_start, selection_end;
   GetSelectionOffsets(&selection_start, &selection_end);
-  if (selection_start >= 0 && selection_end >= 0 &&
+
+  // A selection exists either: when both selection endpoints are inside this
+  // object but their values are not equal, or when one endpoint is inside and
+  // the other outside this object.
+  if ((selection_start >= 0 || selection_end >= 0) &&
       selection_start != selection_end)
     *n_selections = 1;
 
@@ -3648,6 +3652,9 @@ void BrowserAccessibilityWin::IntAttributeToIA2(
 
 int32 BrowserAccessibilityWin::GetHyperlinkIndexFromChild(
     const BrowserAccessibilityWin& child) const {
+  if (hyperlinks().empty())
+    return -1;
+
   auto iterator = std::find(
       hyperlinks().begin(), hyperlinks().end(), child.GetId());
   if (iterator == hyperlinks().end())
@@ -3658,8 +3665,7 @@ int32 BrowserAccessibilityWin::GetHyperlinkIndexFromChild(
 
 int32 BrowserAccessibilityWin::GetHypertextOffsetFromHyperlinkIndex(
     int32 hyperlink_index) const {
-  auto& offsets_map = hyperlink_offset_to_index();
-  for (auto& offset_index : offsets_map) {
+  for (auto& offset_index : hyperlink_offset_to_index()) {
     if (offset_index.second == hyperlink_index)
       return offset_index.first;
   }
@@ -3690,6 +3696,86 @@ int32 BrowserAccessibilityWin::GetHypertextOffsetFromDescendant(
   return parent_object->GetHypertextOffsetFromChild(*current_object);
 }
 
+int BrowserAccessibilityWin::GetHypertextOffsetFromEndpoint(
+    const BrowserAccessibilityWin& endpoint_object, int endpoint_offset) const {
+  // 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 embedded object character 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 that
+  // one needs to travel to find the endpoint.
+
+
+  // Case 1.
+
+  // IsDescendantOf includes the case when endpoint_object == this.
+  if (IsDescendantOf(&endpoint_object) ||
+      // Handle the case when the endpoint is a direct text-only child.
+      // The offset should still be valid on the parent.
+      (endpoint_object.IsTextOnlyObject() &&
+      endpoint_object.GetParent() == this)) {
+    return endpoint_offset;
+  }
+

[dmazzoni, 2015/09/30 20:09:56]

Please remove the double blank lines within a function

+
+  const BrowserAccessibility* common_parent = this;
+  while (common_parent && !endpoint_object.IsDescendantOf(common_parent)) {
+    common_parent = common_parent->GetParent();
+  }
+  if (!common_parent)
+    return -1;
+
+  // TODO(nektar): Add a const version of ToBrowserAccessibilityWin.

[dmazzoni, 2015/09/30 20:09:56]

Please just do this as part of this changelist

+  auto common_parent_win = static_cast<const BrowserAccessibilityWin*>(
+      common_parent);
+
+  // Text only objects must have a parent.
+  DCHECK(!IsTextOnlyObject() || GetParent());
+  DCHECK(!endpoint_object.IsTextOnlyObject() || endpoint_object.GetParent());
+  // Text only objects that are direct descendants should behave as if they
+  // are part of their parent when computing hyperlink offsets.
+  const BrowserAccessibilityWin* non_text_this = IsTextOnlyObject() ?

[dmazzoni, 2015/09/30 20:09:56]

how about nearest_non_text_ancestor

+      GetParent()->ToBrowserAccessibilityWin() : this;
+  const BrowserAccessibilityWin& non_text_endpoint =
+      endpoint_object.IsTextOnlyObject() ?
+      *(endpoint_object.GetParent()->ToBrowserAccessibilityWin()) :

[dmazzoni, 2015/09/30 20:09:56]

I'd indent this and the next line 4 more spaces because they're part of the
ternary operator expression

+      endpoint_object;
+
+
+  // Case 2.
+
+  // 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.
+  if (common_parent_win == non_text_this)
+    return non_text_this->GetHypertextOffsetFromDescendant(non_text_endpoint);
+
+
+  // Case 3.
+
+  // 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.
+  int32 current_offset = static_cast<int>(

[dmazzoni, 2015/09/30 20:09:56]

nit: <int32> to match the type you're assigning to

+      common_parent_win->GetHypertextOffsetFromDescendant(*non_text_this));
+  DCHECK_GE(current_offset, 0);
+  if (common_parent_win != &non_text_endpoint) {
+    endpoint_offset = static_cast<int>(
+        common_parent_win->GetHypertextOffsetFromDescendant(non_text_endpoint));
+    DCHECK_GE(endpoint_offset, 0);
+  }
+
+  if (endpoint_offset < current_offset)
+    return 0;
+  if (endpoint_offset > current_offset)
+    return TextForIAccessibleText().length();
+
+  NOTREACHED();
+  return -1;
+}
+
 int BrowserAccessibilityWin::GetSelectionAnchor() const {
   BrowserAccessibility* root = manager()->GetRoot();
   int32 anchor_id;
@@ -3701,23 +3787,11 @@ int BrowserAccessibilityWin::GetSelectionAnchor() const {
   if (!anchor_object)
     return -1;
 
-  // 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;
-
-    return anchor_offset;
-  }
-
-  if (anchor_object->IsDescendantOf(this))
-    return GetHypertextOffsetFromDescendant(*anchor_object);
+  int anchor_offset;
+  if (!root->GetIntAttribute(ui::AX_ATTR_ANCHOR_OFFSET, &anchor_offset))
+    return -1;
 
-  return -1;
+  return GetHypertextOffsetFromEndpoint(*anchor_object, anchor_offset);
 }
 
 int BrowserAccessibilityWin::GetSelectionFocus() const {
@@ -3731,22 +3805,11 @@ int BrowserAccessibilityWin::GetSelectionFocus() const {
   if (!focus_object)
     return -1;
 
-  // 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;
-
-    return focus_offset;
-  }
-
-  if (focus_object->IsDescendantOf(this))
-    return GetHypertextOffsetFromDescendant(*focus_object);
+  int focus_offset;
+  if (!root->GetIntAttribute(ui::AX_ATTR_FOCUS_OFFSET, &focus_offset))
+    return -1;
 
-  return -1;
+  return GetHypertextOffsetFromEndpoint(*focus_object, focus_offset);
 }
 
 void BrowserAccessibilityWin::GetSelectionOffsets(
@@ -3764,15 +3827,58 @@ void BrowserAccessibilityWin::GetSelectionOffsets(
   if (*selection_start < 0 || *selection_end < 0)
     return;
 
+  // If the selection is collapsed, then return the caret position only if the
+  // caret is active on this object.
+  // The focus object indicates the caret position, but we will use the anchor
+  // for consistency with Firefox.
+  // TODO(nektar): Investigate why Firefox uses the anchor and not the focus.
+  if (*selection_start == *selection_end) {
+    BrowserAccessibility* root = manager()->GetRoot();
+    int32 anchor_id;
+    if (!root || !root->GetIntAttribute(
+        ui::AX_ATTR_ANCHOR_OBJECT_ID, &anchor_id)) {
+      return;
+    }
+
+    BrowserAccessibilityWin* anchor_object = manager()->GetFromID(
+        anchor_id)->ToBrowserAccessibilityWin();
+    if (!anchor_object)
+      return;
+
+    if (!anchor_object->IsDescendantOf(this) &&
+        (IsTextOnlyObject() && GetParent() != anchor_object)) {
+      *selection_start = -1;
+      *selection_end = -1;
+      return;
+    }
+  }
+
   if (*selection_end < *selection_start)
     std::swap(*selection_start, *selection_end);
 
-  // IA2 Spec says that the end of the selection should be after the last
-  // embedded object character that is part of the selection, if there is one.
-  if (hyperlink_offset_to_index().find(*selection_end) !=
-      hyperlink_offset_to_index().end()) {
+  // The IA2 Spec says that if a selection is present and if the end of the
+  // selection falls on an embedded object character, it should be incremented
+  // by one so that it points after the embedded object character.
+  LONG hyperlink_index;
+  auto current_object = const_cast<BrowserAccessibilityWin*>(this);
+  HRESULT hr = current_object->get_hyperlinkIndex(
+      *selection_end, &hyperlink_index);
+  if (hr != S_OK)
+    return;
+
+  base::win::ScopedComPtr<IAccessibleHyperlink> hyperlink;
+  hr = current_object->get_hyperlink(
+      hyperlink_index, hyperlink.Receive());
+  DCHECK(SUCCEEDED(hr));
+  base::win::ScopedComPtr<IAccessibleText> ax_hyperlink;
+  hr = hyperlink.QueryInterface(ax_hyperlink.Receive());
+  DCHECK(SUCCEEDED(hr));
+  LONG nSelections = -1;
+  hr = ax_hyperlink->get_nSelections(&nSelections);
+  DCHECK(SUCCEEDED(hr));
+
+  if (nSelections > 0)
     ++(*selection_end);
-  }
 }
 
 base::string16 BrowserAccessibilityWin::GetNameRecursive() const {
@@ -3799,7 +3905,7 @@ base::string16 BrowserAccessibilityWin::GetValueText() {
   return value;
 }
 
-base::string16 BrowserAccessibilityWin::TextForIAccessibleText() {
+base::string16 BrowserAccessibilityWin::TextForIAccessibleText() const {
   switch (GetRole()) {
     case ui::AX_ROLE_TEXT_FIELD:
     case ui::AX_ROLE_MENU_LIST_OPTION: