Fix Narrator's Scan Mode navigation

When using Narrator on Windows with Scan Mode turned on and navigating
with the down arrow, it blocks for a specific accessibility tree.
e.g.,

++++genericContainer
++++++image
++++textField editable focusable
++++++genericContainer editable
++++++++staticText editable

This was caused by a degenerate range where both endpoints were on two
different anchors. Apparently, the attribute value returned by
AXPlatformNodeTextRangeProvider::GetAttributeValue were incorrect when
on a degenerate range, causing Windows ATs to behave undesirably.

This issue is fixed by refactoring
AXPlatformNodeTextRangeProvider::NormalizeTextRange so that it applies
to degenerate ranges too. Now, if both endpoints of a range are equals
but on two different anchors, the normalization will move both endpoints
to the start of the next anchor.

I added a unit test and a descriptive comment about NormalizeTextRange().
Also, I moved the order of NormalizeTextRange() and
NormalizeAsUnignoredTextRange().

Bug: 928948
Change-Id: Icd4a3a3d6f569a35b7699d569aaef95449443774
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033632
Commit-Queue: Benjamin Beaudry <benjamin.beaudry@microsoft.com>
Reviewed-by: Kurt Catti-Schmidt <kschmi@microsoft.com>
Reviewed-by: Ethan Jimenez <ethavar@microsoft.com>
Reviewed-by: Kevin Babbitt <kbabbitt@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#739278}

ui/accessibility/platform/ax_platform_node_textrangeprovider_win.cc

@@ -172,13 +172,7 @@ STDMETHODIMP AXPlatformNodeTextRangeProviderWin::ExpandToEnclosingUnit(
     TextUnit unit) {
   WIN_ACCESSIBILITY_API_HISTOGRAM(UMA_API_TEXTRANGE_EXPANDTOENCLOSINGUNIT);
   UIA_VALIDATE_TEXTRANGEPROVIDER_CALL();
-
-  // Normalize the start position so that we do not incorrectly expand it
-  // backwards if it happens to be sitting at the end of a node.
-  AXPositionInstance normalized_start =
-      start_->AsLeafTextPositionBeforeCharacter();
-  if (!normalized_start->IsNullPosition())
-    start_ = std::move(normalized_start);
+  NormalizeTextRange();
 
   // Determine if start is on a boundary of the specified TextUnit, if it is
   // not, move backwards until it is. Move the end forwards from start until it
@@ -1120,6 +1114,35 @@ AXPlatformNodeTextRangeProviderWin::MoveEndpointByUnitHelper(
   return current_endpoint;
 }
 
+void AXPlatformNodeTextRangeProviderWin::NormalizeTextRange() {
+  if (!start_->IsValid() || !end_->IsValid())
+    return;
+
+  // If either endpoint is anchored to an ignored node,
+  // first snap them both to be unignored positions.
+  NormalizeAsUnignoredTextRange();
+
+  AXPositionInstance normalized_start =
+      start_->AsLeafTextPositionBeforeCharacter();
+  // For a degenerate range, the |end_| will always be the same as the
+  // normalized start, so there's no need to compute the normalized end.
+  // However, a degenerate range might go undetected if there's an ignored node
+  // (or many) between the two endpoints. For this reason, we need to
+  // compare the |end_| with both the |start_| and the |normalized_start|.
+  bool is_degenerate = *start_ == *end_ || *normalized_start == *end_;
+  AXPositionInstance normalized_end =
+      is_degenerate ? normalized_start->Clone()
+                    : end_->AsLeafTextPositionAfterCharacter();
+
+  if (!normalized_start->IsNullPosition() &&
+      !normalized_end->IsNullPosition()) {
+    start_ = std::move(normalized_start);
+    end_ = std::move(normalized_end);
+  }
+
+  DCHECK_LE(*start_, *end_);
+}
+
 void AXPlatformNodeTextRangeProviderWin::NormalizeAsUnignoredTextRange() {
   if (!start_->IsValid() || !end_->IsValid())
     return;
@@ -1152,45 +1175,6 @@ void AXPlatformNodeTextRangeProviderWin::NormalizeAsUnignoredTextRange() {
   DCHECK_LE(*start_, *end_);
 }
 
-void AXPlatformNodeTextRangeProviderWin::NormalizeTextRange() {
-  if (!start_->IsValid() || !end_->IsValid())
-    return;
-
-  // If either endpoint is anchored to an ignored node,
-  // first snap them both to be unignored positions.
-  NormalizeAsUnignoredTextRange();
-
-  if (*start_ == *end_)
-    return;
-
-  AXPositionInstance normalized_start =
-      start_->AsLeafTextPositionBeforeCharacter();
-  AXPositionInstance normalized_end = end_->AsLeafTextPositionAfterCharacter();
-
-  // Handle the fringe case when |normalized_start| and |normalized_end| end up
-  // inverted after AsLeafTextPosition{Before|After}Character() calls.
-  // Consider the following case:
-  //    text1<start_>|IGNORED node|<end_>text2
-  // Due to |start_| and |end_| positions spanning ignored nodes, we end up
-  // with the following inverted normalized positions:
-  //    <normalized_end>text1|IGNORED node|<normalized_start>text2
-  // So we want to create a collapsed range by setting |normalized_end| to
-  // |normalized_start|.
-  if (!normalized_start->IsNullPosition() &&
-      !normalized_end->IsNullPosition() &&
-      *normalized_end < *normalized_start) {
-    normalized_end = normalized_start->Clone();
-  }
-
-  if (!normalized_start->IsNullPosition() &&
-      !normalized_end->IsNullPosition()) {
-    start_ = std::move(normalized_start);
-    end_ = std::move(normalized_end);
-  }
-
-  DCHECK_LE(*start_, *end_);
-}
-
 AXPlatformNodeDelegate* AXPlatformNodeTextRangeProviderWin::GetRootDelegate(
     const ui::AXTreeID tree_id) {
   const AXTreeManager* ax_tree_manager =

ui/accessibility/platform/ax_platform_node_textrangeprovider_win.h

@@ -146,8 +146,16 @@ class AX_EXPORT __declspec(uuid("3071e40d-a10d-45ff-a59f-6e8e1138e2c1"))
       const int count,
       int* units_moved);
 
-  void NormalizeAsUnignoredTextRange();
+  // A text range normalization is necessary to prevent a |start_| endpoint to
+  // be positioned at the end of an anchor when it can be at the start of the
+  // next anchor. After normalization, it is guaranteed that:
+  // * both endpoints of a range are always positioned on unignored anchors;
+  // * both endpoints of a range are never between a grapheme cluster;
+  // * if the range is degenerate, both endpoints of a range are on the same
+  //   anchor.
   void NormalizeTextRange();
+  void NormalizeAsUnignoredTextRange();
+
   AXPlatformNodeDelegate* GetRootDelegate(const ui::AXTreeID tree_id);
   AXNode* GetSelectionCommonAnchor();
   void RemoveFocusFromPreviousSelectionIfNeeded(

ui/accessibility/platform/ax_platform_node_textrangeprovider_win_unittest.cc

@@ -5236,6 +5236,102 @@ TEST_F(AXPlatformNodeTextRangeProviderTest,
   EXPECT_EQ(0, GetEnd(range_span_ignored_nodes.Get())->text_offset());
 }
 
+TEST_F(AXPlatformNodeTextRangeProviderTest,
+       TestNormalizeTextRangeForceSameAnchorOnDegenerateRange) {
+  // ++1 kRootWebArea
+  // ++++2 kGenericContainer
+  // ++++++3 kImage
+  // ++++4 kTextField
+  // ++++++5 kGenericContainer
+  // ++++++++6 kStaticText
+  // ++++++++++7 kInlineTextBox
+  ui::AXNodeData root_1;
+  ui::AXNodeData generic_container_2;
+  ui::AXNodeData image_3;
+  ui::AXNodeData text_field_4;
+  ui::AXNodeData generic_container_5;
+  ui::AXNodeData static_text_6;
+  ui::AXNodeData inline_box_7;
+
+  root_1.id = 1;
+  generic_container_2.id = 2;
+  image_3.id = 3;
+  text_field_4.id = 4;
+  generic_container_5.id = 5;
+  static_text_6.id = 6;
+  inline_box_7.id = 7;
+
+  root_1.role = ax::mojom::Role::kRootWebArea;
+  root_1.child_ids = {generic_container_2.id, text_field_4.id};
+
+  generic_container_2.role = ax::mojom::Role::kGenericContainer;
+  generic_container_2.AddBoolAttribute(
+      ax::mojom::BoolAttribute::kIsLineBreakingObject, true);
+  generic_container_2.child_ids = {image_3.id};
+
+  image_3.role = ax::mojom::Role::kImage;
+
+  text_field_4.role = ax::mojom::Role::kTextField;
+  text_field_4.child_ids = {generic_container_5.id};
+  text_field_4.SetValue("3.14");
+
+  generic_container_5.role = ax::mojom::Role::kGenericContainer;
+  generic_container_5.child_ids = {static_text_6.id};
+
+  static_text_6.role = ax::mojom::Role::kStaticText;
+  static_text_6.child_ids = {inline_box_7.id};
+  static_text_6.SetName("3.14");
+
+  inline_box_7.role = ax::mojom::Role::kInlineTextBox;
+  inline_box_7.SetName("3.14");
+
+  ui::AXTreeUpdate update;
+  ui::AXTreeData tree_data;
+  tree_data.tree_id = ui::AXTreeID::CreateNewAXTreeID();
+  update.tree_data = tree_data;
+  update.has_tree_data = true;
+  update.root_id = root_1.id;
+  update.nodes.push_back(root_1);
+  update.nodes.push_back(generic_container_2);
+  update.nodes.push_back(image_3);
+  update.nodes.push_back(text_field_4);
+  update.nodes.push_back(generic_container_5);
+  update.nodes.push_back(static_text_6);
+  update.nodes.push_back(inline_box_7);
+
+  Init(update);
+  AXNodePosition::SetTree(tree_.get());
+
+  AXPlatformNodeWin* owner = static_cast<AXPlatformNodeWin*>(
+      AXPlatformNodeFromNode(GetNodeFromTree(tree_data.tree_id, 1)));
+
+  // TextPosition, anchor_id=3, text_offset=1, annotated_text=/xFFFC<>
+  AXNodePosition::AXPositionInstance range_start =
+      AXNodePosition::CreateTextPosition(tree_data.tree_id, /*anchor_id=*/3,
+                                         /*text_offset=*/1,
+                                         ax::mojom::TextAffinity::kDownstream);
+
+  // TextPosition, anchor_id=7, text_offset=0, annotated_text=<p>i
+  AXNodePosition::AXPositionInstance range_end =
+      AXNodePosition::CreateTextPosition(tree_data.tree_id, /*anchor_id=*/7,
+                                         /*text_offset=*/0,
+                                         ax::mojom::TextAffinity::kDownstream);
+
+  ComPtr<AXPlatformNodeTextRangeProviderWin> range;
+  ComPtr<ITextRangeProvider> text_range_provider =
+      AXPlatformNodeTextRangeProviderWin::CreateTextRangeProvider(
+          owner, std::move(range_start), std::move(range_end));
+  text_range_provider->QueryInterface(IID_PPV_ARGS(&range));
+
+  NormalizeTextRange(range.Get());
+  EXPECT_EQ(*GetStart(range.Get()), *GetEnd(range.Get()));
+
+  EXPECT_EQ(true, GetStart(range.Get())->AtStartOfAnchor());
+  EXPECT_EQ(true, GetEnd(range.Get())->AtStartOfAnchor());
+  EXPECT_EQ(7, GetStart(range.Get())->anchor_id());
+  EXPECT_EQ(7, GetEnd(range.Get())->anchor_id());
+}
+
 TEST_F(AXPlatformNodeTextRangeProviderTest, TestValidateStartAndEnd) {
   // This test updates the tree structure to test a specific edge case -
   // CreatePositionAtFormatBoundary when text lies at the beginning and end