AXSelection: Added declarative test for aria-hidden and strengthened tests for...

AXSelection: Added declarative test for aria-hidden and strengthened tests for aria-hidden and list bullets

Tests now check the results of both shrinking a selection as well as extending a selection.
In the case of aria-hidden, we want to check whether a DOM selection on aria-hidden endpoints would be correctly extended or shrunk when converted to an equivalent AX selection, since aria-hidden objects are an example of an object that is ignored in the accessibility tree.
In the case of a list bullet, we want to check whether an AXSelection that has an endpoint on a list bullet, would be correctly extended or shrunk when converted to a DOM selection, since list bullets are an example of an object not present in the DOM tree.
R=dmazzoni@chromium.org

Bug: 639340
Change-Id: Ia9e7dee6efe5c0059faae101a1a07b5d7d35ef5c
Reviewed-on: https://chromium-review.googlesource.com/c/1327221
Commit-Queue: Nektarios Paisios <nektar@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607631}

third_party/blink/renderer/modules/accessibility/ax_position.cc

@@ -248,8 +248,12 @@ const AXPosition AXPosition::FromPosition(
                 *document, *node_after_position,
                 ToContainerNodeOrNull(container_node), adjustment_behavior);
             if (previous_child) {
-              return CreatePositionAfterObject(*previous_child,
-                                               adjustment_behavior);
+              // |CreatePositionAfterObject| cannot be used here because it will
+              // try to create a position before the object that comes after
+              // |previous_child|, which in this case is the ignored object
+              // itself.
+              return CreateLastPositionInObject(*previous_child,
+                                                adjustment_behavior);
             }
 
             return CreateFirstPositionInObject(*container, adjustment_behavior);
@@ -473,10 +477,10 @@ const AXPosition AXPosition::AsUnignoredPosition(
   // container's unignored parent.
   //
   // 3. The container object is ignored and this is an "after children"
-  // position. Find the next object in the tree and recurse.
+  // position. Find the previous or the next object in the tree and recurse.
   //
   // 4. The child after a tree position is ignored, but the container object is
-  // not. Return an "after children" position.
+  // not. Return a "before children" or an "after children" position.
 
   const AXObject* container = container_object_;
   const AXObject* child = ChildAfterTreePosition();
@@ -522,8 +526,14 @@ const AXPosition AXPosition::AsUnignoredPosition(
   }
 
   // Case 4.
-  if (child && child->AccessibilityIsIgnored())
-    return CreateLastPositionInObject(*container);
+  if (child && child->AccessibilityIsIgnored()) {
+    switch (adjustment_behavior) {
+      case AXPositionAdjustmentBehavior::kMoveRight:
+        return CreateLastPositionInObject(*container);
+      case AXPositionAdjustmentBehavior::kMoveLeft:
+        return CreateFirstPositionInObject(*container);
+    }
+  }
 
   // The position is not ignored.
   return *this;
@@ -584,8 +594,8 @@ const AXPosition AXPosition::AsValidDOMPosition(
          "node should have an associated layout object.";
   const Node* container_node =
       ToAXLayoutObject(container)->GetNodeOrContainingBlockNode();
-  DCHECK(container_node)
-      << "All anonymous layout objects should have a containing block element.";
+  DCHECK(container_node) << "All anonymous layout objects and list markers "
+                            "should have a containing block element.";
   DCHECK(!container->IsDetached());
   auto& ax_object_cache_impl = container->AXObjectCache();
   const AXObject* new_container =
@@ -595,7 +605,14 @@ const AXPosition AXPosition::AsValidDOMPosition(
   if (new_container == container->ParentObjectUnignored()) {
     position.text_offset_or_child_index_ = container->IndexInParent();
   } else {
-    position.text_offset_or_child_index_ = 0;
+    switch (adjustment_behavior) {
+      case AXPositionAdjustmentBehavior::kMoveRight:
+        position.text_offset_or_child_index_ = new_container->ChildCount();
+        break;
+      case AXPositionAdjustmentBehavior::kMoveLeft:
+        position.text_offset_or_child_index_ = 0;
+        break;
+    }
   }
   DCHECK(position.IsValid());
   return position.AsValidDOMPosition(adjustment_behavior);
@@ -608,7 +625,8 @@ const PositionWithAffinity AXPosition::ToPositionWithAffinity(
     return {};
 
   const Node* container_node = adjusted_position.container_object_->GetNode();
-  DCHECK(container_node);
+  DCHECK(container_node) << "AX positions that are valid DOM positions should "
+                            "always be connected to their DOM nodes.";
   if (!adjusted_position.IsTextPosition()) {
     // AX positions that are unumbiguously at the start or end of a container,
     // should convert to the corresponding DOM positions at the start or end of

third_party/blink/renderer/modules/accessibility/ax_position_test.cc

@@ -873,6 +873,9 @@ TEST_F(AccessibilityTest, FromPositionInARIAHidden) {
   ASSERT_NE(nullptr, ax_container);
   ASSERT_EQ(ax::mojom::Role::kMain, ax_container->RoleValue());
   ASSERT_EQ(2, ax_container->ChildCount());
+  const AXObject* ax_before = GetAXObjectByElementId("before");
+  ASSERT_NE(nullptr, ax_before);
+  ASSERT_EQ(ax::mojom::Role::kParagraph, ax_before->RoleValue());
   const AXObject* ax_after = GetAXObjectByElementId("after");
   ASSERT_NE(nullptr, ax_after);
   ASSERT_EQ(ax::mojom::Role::kParagraph, ax_after->RoleValue());
@@ -885,14 +888,23 @@ TEST_F(AccessibilityTest, FromPositionInARIAHidden) {
   const auto positions = {position_first, position_before, position_after};
 
   for (const auto& position : positions) {
+    //
+    // |kMoveLeft| will create "after children" positions that are anchored to
+    // the paragraph before the element that is aria-hidden.
+    //
+    // |kMoveRight| will create positions that are anchored to the paragraph
+    // after the element that is aria-hidden.
+    //
+
     const auto ax_position_left =
         AXPosition::FromPosition(position, TextAffinity::kDownstream,
                                  AXPositionAdjustmentBehavior::kMoveLeft);
     EXPECT_TRUE(ax_position_left.IsValid());
     EXPECT_FALSE(ax_position_left.IsTextPosition());
-    EXPECT_EQ(ax_container, ax_position_left.ContainerObject());
+    EXPECT_EQ(ax_before, ax_position_left.ContainerObject());
     EXPECT_EQ(1, ax_position_left.ChildIndex());
-    EXPECT_EQ(ax_after, ax_position_left.ChildAfterTreePosition());
+    // This is an "after children" position.
+    EXPECT_EQ(nullptr, ax_position_left.ChildAfterTreePosition());
 
     const auto ax_position_right =
         AXPosition::FromPosition(position, TextAffinity::kDownstream,

third_party/blink/renderer/modules/accessibility/ax_selection.cc

@@ -187,7 +187,7 @@ const SelectionInDOMTree AXSelection::AsSelection(
 
 void AXSelection::Select(const AXSelectionBehavior selection_behavior) {
   if (!IsValid()) {
-    NOTREACHED();
+    NOTREACHED() << "Trying to select an invalid accessibility selection.";
     return;
   }
 

third_party/blink/renderer/modules/accessibility/ax_selection.h

@@ -22,6 +22,11 @@ namespace blink {
 // accessibility tree but not in the DOM tree, determines whether setting the
 // selection will shrink or extend the |AXSelection| to encompass endpoints that
 // are in the DOM.
+// Conversely, if a DOM selection is converted to an |AXSelection| via the
+// |AsSelection| method, but the endpoints of the DOM selection are not present
+// in the accessibility tree, e.g. they are aria-hidden, determines whether the
+// conversion will shrink or extend the DOM selection to encompass endpoints
+// that are in the accessibility tree.
 enum class AXSelectionBehavior {
   kShrinkToValidDOMRange,
   kExtendToValidDOMRange

third_party/blink/renderer/modules/accessibility/ax_selection_test.cc

@@ -4,6 +4,8 @@
 
 #include "third_party/blink/renderer/modules/accessibility/ax_selection.h"
 
+#include <string>
+
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/dom/node.h"
@@ -83,65 +85,191 @@ TEST_F(AccessibilitySelectionTest, SetSelectionInTextWithWhiteSpace) {
 //
 // Get selection tests.
 // Retrieving a selection with endpoints which have no corresponding objects in
-// the accessibility tree, e.g. which are aria-hidden, should shring
-// |AXSelection| to valid endpoints.
+// the accessibility tree, e.g. which are aria-hidden, should shrink or extend
+// the |AXSelection| to valid endpoints.
 //
 
 TEST_F(AccessibilitySelectionTest, SetSelectionInARIAHidden) {
-  SetSelectionText(R"HTML(
-      <div role="main">
-        <p>Before aria-hidden.</p>
-        ^<p aria-hidden="true">Aria-hidden 1.</p>
-        <p>In the middle of aria-hidden.</p>
-        <p aria-hidden="true">Aria-hidden 2.</p>|
-        <p>After aria-hidden.</p>
+  SetBodyInnerHTML(R"HTML(
+      <div id="main" role="main">
+        <p id="beforeHidden">Before aria-hidden.</p>
+        <p id="hidden1" aria-hidden="true">Aria-hidden 1.</p>
+        <p id="betweenHidden">In between two aria-hidden elements.</p>
+        <p id="hidden2" aria-hidden="true">Aria-hidden 2.</p>
+        <p id="afterHidden">After aria-hidden.</p>
       </div>
       )HTML");
 
-  const SelectionInDOMTree selection =
-      GetFrame().Selection().GetSelectionInDOMTree();
+  const Node* hidden_1 = GetElementById("hidden1");
+  ASSERT_NE(nullptr, hidden_1);
+  const Node* hidden_2 = GetElementById("hidden2");
+  ASSERT_NE(nullptr, hidden_2);
+
+  const AXObject* ax_main = GetAXObjectByElementId("main");
+  ASSERT_NE(nullptr, ax_main);
+  ASSERT_EQ(ax::mojom::Role::kMain, ax_main->RoleValue());
+  const AXObject* ax_before = GetAXObjectByElementId("beforeHidden");
+  ASSERT_NE(nullptr, ax_before);
+  ASSERT_EQ(ax::mojom::Role::kParagraph, ax_before->RoleValue());
+  const AXObject* ax_between = GetAXObjectByElementId("betweenHidden");
+  ASSERT_NE(nullptr, ax_between);
+  ASSERT_EQ(ax::mojom::Role::kParagraph, ax_between->RoleValue());
+  const AXObject* ax_after = GetAXObjectByElementId("afterHidden");
+  ASSERT_NE(nullptr, ax_after);
+  ASSERT_EQ(ax::mojom::Role::kParagraph, ax_after->RoleValue());
+
+  ASSERT_NE(nullptr, GetAXObjectByElementId("hidden1"));
+  ASSERT_TRUE(GetAXObjectByElementId("hidden1")->AccessibilityIsIgnored());
+  ASSERT_NE(nullptr, GetAXObjectByElementId("hidden2"));
+  ASSERT_TRUE(GetAXObjectByElementId("hidden2")->AccessibilityIsIgnored());
+
+  const auto hidden_1_first = Position::FirstPositionInNode(*hidden_1);
+  const auto hidden_2_first = Position::FirstPositionInNode(*hidden_2);
+  const auto selection = SelectionInDOMTree::Builder()
+                             .SetBaseAndExtent(hidden_1_first, hidden_2_first)
+                             .Build();
 
   const auto ax_selection_shrink = AXSelection::FromSelection(
       selection, AXSelectionBehavior::kShrinkToValidDOMRange);
-  EXPECT_EQ("", GetSelectionText(ax_selection_shrink));
-
   const auto ax_selection_extend = AXSelection::FromSelection(
       selection, AXSelectionBehavior::kExtendToValidDOMRange);
-  EXPECT_EQ("", GetSelectionText(ax_selection_extend));
+
+  // The shrunk selection should encompass only the |AXObject| between the two
+  // aria-hidden elements and nothing else. This means that its anchor should be
+  // before and its focus after the |AXObject| in question.
+  ASSERT_FALSE(ax_selection_shrink.Base().IsTextPosition());
+  EXPECT_EQ(ax_main, ax_selection_shrink.Base().ContainerObject());
+  EXPECT_EQ(ax_between->IndexInParent(),
+            ax_selection_shrink.Base().ChildIndex());
+  ASSERT_FALSE(ax_selection_shrink.Extent().IsTextPosition());
+  EXPECT_EQ(ax_between, ax_selection_shrink.Extent().ContainerObject());
+  EXPECT_EQ(1, ax_selection_shrink.Extent().ChildIndex());
+
+  // The extended selection should start after the children of the paragraph
+  // before the first aria-hidden element and end right before the paragraph
+  // after the last aria-hidden element.
+  EXPECT_FALSE(ax_selection_extend.Base().IsTextPosition());
+  EXPECT_EQ(ax_before, ax_selection_extend.Base().ContainerObject());
+  EXPECT_EQ(1, ax_selection_extend.Base().ChildIndex());
+  EXPECT_FALSE(ax_selection_extend.Extent().IsTextPosition());
+  EXPECT_EQ(ax_main, ax_selection_extend.Extent().ContainerObject());
+  EXPECT_EQ(ax_after->IndexInParent(),
+            ax_selection_extend.Extent().ChildIndex());
+
+  // Even though the two AX selections have different anchors and foci, the text
+  // selected in the accessibility tree should not differ, because any
+  // differences in the equivalent DOM selections concern elements that are
+  // aria-hidden. However, the AX selections should still differ if converted to
+  // DOM selections.
+  const std::string selection_text(
+      "++<Main>\n"
+      "++++<Paragraph>\n"
+      "++++++<StaticText: Before aria-hidden.>\n"
+      "^++++<Paragraph>\n"
+      "++++++<StaticText: In between two aria-hidden elements.>\n"
+      "|++++<Paragraph>\n"
+      "++++++<StaticText: After aria-hidden.>\n");
+  EXPECT_EQ(selection_text, GetSelectionText(ax_selection_shrink));
+  EXPECT_EQ(selection_text, GetSelectionText(ax_selection_extend));
 }
 
 //
 // Set selection tests.
 // Setting the selection from an |AXSelection| that has endpoints which are not
-// present in the layout tree should shring the selection to visible endpoints.
+// present in the layout tree should shrink or extend the selection to visible
+// endpoints.
 //
 
 TEST_F(AccessibilitySelectionTest, SetSelectionAroundListBullet) {
-  SetSelectionText(R"HTML(
+  SetBodyInnerHTML(R"HTML(
       <div role="main">
         <ul>
-          ^<li>Item 1.</li>
-          <li>Item 2.</li>|
+          <li id="item1">Item 1.</li>
+          <li id="item2">Item 2.</li>
         </ul>
       </div>
       )HTML");
 
-  const SelectionInDOMTree selection =
+  const Node* item_1 = GetElementById("item1");
+  ASSERT_NE(nullptr, item_1);
+  ASSERT_FALSE(item_1->IsTextNode());
+  const Node* item_2 = GetElementById("item2");
+  ASSERT_NE(nullptr, item_2);
+  ASSERT_FALSE(item_2->IsTextNode());
+  const Node* text_2 = item_2->firstChild();
+  ASSERT_NE(nullptr, text_2);
+  ASSERT_TRUE(text_2->IsTextNode());
+
+  const AXObject* ax_item_1 = GetAXObjectByElementId("item1");
+  ASSERT_NE(nullptr, ax_item_1);
+  ASSERT_EQ(ax::mojom::Role::kListItem, ax_item_1->RoleValue());
+  const AXObject* ax_bullet_1 = ax_item_1->FirstChild();
+  ASSERT_NE(nullptr, ax_bullet_1);
+  ASSERT_EQ(ax::mojom::Role::kListMarker, ax_bullet_1->RoleValue());
+  const AXObject* ax_item_2 = GetAXObjectByElementId("item2");
+  ASSERT_NE(nullptr, ax_item_2);
+  ASSERT_EQ(ax::mojom::Role::kListItem, ax_item_2->RoleValue());
+  const AXObject* ax_text_2 = ax_item_2->LastChild();
+  ASSERT_NE(nullptr, ax_text_2);
+  ASSERT_EQ(ax::mojom::Role::kStaticText, ax_text_2->RoleValue());
+
+  AXSelection::Builder builder;
+  AXSelection ax_selection =
+      builder.SetBase(AXPosition::CreateFirstPositionInObject(*ax_bullet_1))
+          .SetExtent(AXPosition::CreateLastPositionInObject(*ax_text_2))
+          .Build();
+
+  // The list bullet is not included in the DOM tree. Shrinking the
+  // |AXSelection| should skip over it by creating an anchor before the first
+  // child of the first <li>, i.e. the text node containing the text "Item 1.".
+  // This should be further optimized to a "before children" position at the
+  // first <li>.
+  ax_selection.Select(AXSelectionBehavior::kShrinkToValidDOMRange);
+  const SelectionInDOMTree shrunk_selection =
       GetFrame().Selection().GetSelectionInDOMTree();
 
-  const auto ax_selection_shrink = AXSelection::FromSelection(
-      selection, AXSelectionBehavior::kShrinkToValidDOMRange);
-  EXPECT_EQ("", GetSelectionText(ax_selection_shrink));
+  EXPECT_EQ(item_1, shrunk_selection.Base().AnchorNode());
+  EXPECT_TRUE(shrunk_selection.Base().IsBeforeChildren());
+  ASSERT_TRUE(shrunk_selection.Extent().IsOffsetInAnchor());
+  EXPECT_EQ(text_2, shrunk_selection.Extent().AnchorNode());
+  EXPECT_EQ(7, shrunk_selection.Extent().OffsetInContainerNode());
 
-  const auto ax_selection_extend = AXSelection::FromSelection(
-      selection, AXSelectionBehavior::kExtendToValidDOMRange);
-  EXPECT_EQ("", GetSelectionText(ax_selection_extend));
+  // The list bullet is not included in the DOM tree. Extending the
+  // |AXSelection| should move the anchor to before the first <li>.
+  ax_selection.Select(AXSelectionBehavior::kExtendToValidDOMRange);
+  const SelectionInDOMTree extended_selection =
+      GetFrame().Selection().GetSelectionInDOMTree();
+
+  ASSERT_TRUE(extended_selection.Base().IsOffsetInAnchor());
+  EXPECT_EQ(item_1->parentNode(), extended_selection.Base().AnchorNode());
+  EXPECT_EQ(static_cast<int>(item_1->NodeIndex()),
+            extended_selection.Base().OffsetInContainerNode());
+  ASSERT_TRUE(extended_selection.Extent().IsOffsetInAnchor());
+  EXPECT_EQ(text_2, extended_selection.Extent().AnchorNode());
+  EXPECT_EQ(7, extended_selection.Extent().OffsetInContainerNode());
+
+  // The |AXSelection| should remain unaffected by any shrinking and should
+  // include both list bullets.
+  EXPECT_EQ(
+      "++<Main>\n"
+      "++++<List>\n"
+      "++++++<ListItem>\n"
+      "++++++++<ListMarker: \xE2\x80\xA2 >\n"
+      "++++++++<StaticText: Item 1.>\n"
+      "++++++<ListItem>\n"
+      "++++++++<ListMarker: \xE2\x80\xA2 >\n"
+      "++++++++<StaticText: Item 2.|>\n",
+      GetSelectionText(ax_selection));
 }
 
 //
 // Declarative tests.
 //
 
+TEST_F(AccessibilitySelectionTest, ARIAHidden) {
+  RunSelectionTest("aria-hidden");
+}
+
 TEST_F(AccessibilitySelectionTest, List) {
   RunSelectionTest("list");
 }

third_party/blink/renderer/modules/accessibility/testing/accessibility_selection_test.cc

@@ -356,7 +356,7 @@ std::string AccessibilitySelectionTest::GetSelectionText(
   return AXSelectionSerializer(selection).Serialize(subtree);
 }
 
-const AXSelection AccessibilitySelectionTest::SetSelectionText(
+AXSelection AccessibilitySelectionTest::SetSelectionText(
     const std::string& selection_text) const {
   HTMLElement* body = GetDocument().body();
   if (!body)
@@ -369,7 +369,7 @@ const AXSelection AccessibilitySelectionTest::SetSelectionText(
   return ax_selections.front();
 }
 
-const AXSelection AccessibilitySelectionTest::SetSelectionText(
+AXSelection AccessibilitySelectionTest::SetSelectionText(
     const std::string& selection_text,
     HTMLElement& element) const {
   const Vector<AXSelection> ax_selections =

third_party/blink/renderer/modules/accessibility/testing/accessibility_selection_test.h

@@ -43,13 +43,15 @@ class AccessibilitySelectionTest : public AccessibilityTest {
                                const AXObject& subtree) const;
 
   // Sets |selection_text| as inner HTML of the document body and returns the
-  // root of the accessibility tree at body.
-  const AXSelection SetSelectionText(const std::string& selection_text) const;
-
-  // Sets |selection_text| as inner HTML of |element| and returns the root of
-  // the accessibility subtree at |element|.
-  const AXSelection SetSelectionText(const std::string& selection_text,
-                                     HTMLElement& element) const;
+  // resulting |AXSelection|. If there are multiple selection markers, returns
+  // only the first selection.
+  AXSelection SetSelectionText(const std::string& selection_text) const;
+
+  // Sets |selection_text| as inner HTML of |element| and returns the resulting
+  // |AXSelection|. If there are multiple selection markers, returns only the
+  // first selection.
+  AXSelection SetSelectionText(const std::string& selection_text,
+                               HTMLElement& element) const;
 
   // Compares two HTML files containing a DOM selection and the equivalent
   // accessibility selection.

third_party/blink/renderer/modules/accessibility/testing/data/selection/aria-hidden-ax.txt

+
+================================================================================
+AXSelection from AX object anchored position in "Main": "", 1 to AX object anchored position in "Main": "", 2
+================================================================================
+++<Main>
+++++<Paragraph>
+++++++<StaticText: Before aria-hidden.>
+^++++<Paragraph>
+++++++<StaticText: In the middle of aria-hidden.>
+|++++<Paragraph>
+++++++<StaticText: After aria-hidden.>

third_party/blink/renderer/modules/accessibility/testing/data/selection/aria-hidden.html

+<!DOCTYPE html>
+<html>
+  <body>
+    <div role="main">
+      <p>Before aria-hidden.</p>
+      ^<p aria-hidden="true">Aria-hidden 1.</p>
+      <p>In the middle of aria-hidden.</p>
+      <p aria-hidden="true">Aria-hidden 2.</p>|
+      <p>After aria-hidden.</p>
+    </div>
+  </body>
+</html>