Add robust implementation of PosInSet and SetSize

Build on existing naive PosInSet and SetSize implementation to handle
cases such as inconsistent or invalid assignments of kPosInSet and
kSetSize attributes

Change-Id: I966f3ad00e778e0d68bcb48d160f1be5818b587b
Reviewed-on: https://chromium-review.googlesource.com/c/1340523
Commit-Queue: Akihiro Ota <akihiroota@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609017}

ui/accessibility/ax_node.cc

@@ -473,7 +473,8 @@ void AXNode::IdVectorToNodeVector(std::vector<int32_t>& ids,
   }
 }
 
-bool AXNode::IsPosInSetUsedInRole() const {
+// Determines the roles in which PosInSet and SetSize are used
+bool AXNode::IsSetSizePosInSetUsedInRole() const {
   switch (data().role) {
     case ax::mojom::Role::kArticle:
     case ax::mojom::Role::kListItem:
@@ -491,43 +492,104 @@ bool AXNode::IsPosInSetUsedInRole() const {
   }
 }
 
-// Finds the position of this node within a list.
-// Only takes into account elements that have same role as node.
-int32_t AXNode::PosInSet() const {
+// Finds the 0-based index of first element in the container (with same role as
+// node) where kPosInSet is assigned. Returns -1 if there are no elements with
+// kPosInSet assigned
+int32_t AXNode::FirstAssignedPosInSet() const {
   AXNode* parent = GetUnignoredParent();
+  for (int i = 0; i < parent->GetUnignoredChildCount(); ++i) {
+    AXNode* candidate = parent->GetUnignoredChildAtIndex(i);
+    if (!(candidate->data().role == data().role))
+      continue;
+    if (candidate->HasIntAttribute(ax::mojom::IntAttribute::kPosInSet))
+      return i;
+  }
+  return -1;
+}
 
+// Calculates node's position relative to first PosInSet-assigned element.
+// Returns node's 0-based index within container (relative to nodes with the
+// same role) if no element assigned PosInSet
+int32_t AXNode::RelativePosFromFirstAssigned(
+    int32_t first_assigned_index) const {
+  AXNode* parent = GetUnignoredParent();
+  // Find nodes index in container
+  int nodes_index = 0;
+  for (int i = 0; i < parent->GetUnignoredChildCount(); ++i) {
+    AXNode* candidate = parent->GetUnignoredChildAtIndex(i);
+    if (candidate == this)
+      break;
+    if (candidate->data().role == data().role)
+      ++nodes_index;
+  }
+
+  if (first_assigned_index == -1)
+    return nodes_index;
+
+  // Calculate relative position
+  return nodes_index - first_assigned_index;
+}
+
+// Finds the position of this node within its container, relative to other
+// nodes with the same role.
+// Returns 1-based position if present in container, and 0 if not.
+int32_t AXNode::PosInSet() const {
   // Error checks
+  if (!IsSetSizePosInSetUsedInRole())
+    return 0;
+  AXNode* parent = GetUnignoredParent();
   if (!parent)
     return 0;
   if (parent->data().role != ax::mojom::Role::kList)
     return 0;
-  if (!IsPosInSetUsedInRole())
-    return 0;
 
-  int position = 0;
-  for (int i = 0; i < parent->GetUnignoredChildCount(); ++i) {
-    AXNode* candidate = parent->GetUnignoredChildAtIndex(i);
-    if (candidate->data().role == data().role)
-      ++position;
-    if (candidate == this)
-      return position;
+  // Check if kPosInSet assigned and return if provided
+  if (HasIntAttribute(ax::mojom::IntAttribute::kPosInSet)) {
+    return GetIntAttribute(ax::mojom::IntAttribute::kPosInSet);
   }
-  return 0;
+
+  // Caluclate PosInSet
+  // 1. Find index of first element (with same role) that has kPosInSet Assigned
+  // 2. Find relative position of this node compared to first assigned
+  // 3. The PosInSet of this node = PosInSet(first assigned) + relative position
+
+  int32_t first_assigned_index = FirstAssignedPosInSet();
+  int32_t relative_position =
+      RelativePosFromFirstAssigned(first_assigned_index);
+
+  // If no element assigned PosInSet, return this node's 1-based position in the
+  // container
+  if (first_assigned_index == -1)
+    return relative_position + 1;
+
+  return parent->GetUnignoredChildAtIndex(first_assigned_index)
+             ->GetIntAttribute(ax::mojom::IntAttribute::kPosInSet) +
+         relative_position;
 }
 
-// Finds the total number of elements of the list this node is contained within.
-// Only counts the elements that have the same role as node.
+// Calculates the number of elements within node's container that have the
+// same role as node.
 int32_t AXNode::SetSize() const {
-  AXNode* parent = GetUnignoredParent();
-
   // Error checks
+  if (!IsSetSizePosInSetUsedInRole())
+    return 0;
+  AXNode* parent = GetUnignoredParent();
   if (!parent)
     return 0;
   if (parent->data().role != ax::mojom::Role::kList)
     return 0;
-  if (!IsPosInSetUsedInRole())
-    return 0;
 
+  // TODO (akihiroota): List objects should report SetSize
+
+  // Check if kSetSize assigned and return if provided
+  if (HasIntAttribute(ax::mojom::IntAttribute::kSetSize))
+    return GetIntAttribute(ax::mojom::IntAttribute::kSetSize);
+
+  // Check if kSetSize assigned in container and return if provided
+  if (parent->HasIntAttribute(ax::mojom::IntAttribute::kSetSize))
+    return parent->GetIntAttribute(ax::mojom::IntAttribute::kSetSize);
+
+  // Calculate SetSize
   int count = 0;
   for (int i = 0; i < parent->GetUnignoredChildCount(); ++i) {
     AXNode* child = parent->GetUnignoredChildAtIndex(i);

ui/accessibility/ax_node.h

@@ -181,14 +181,13 @@ class AX_EXPORT AXNode final {
     return data().GetHtmlAttribute(attribute, value);
   }
 
-  // Returns the position of node within a list. Returns 1-based index if
-  // contained within a list, and 0 if not.
+  // Finds the position of this node within its container, relative to others
+  // with the same role.
+  // Returns 1-based index if present in container, and 0 if not.
   int32_t PosInSet() const;
-  // Returns the total number of nodes in the same list as node. Returns 0
-  // if the node is not contained wihtin a list.
+  // Calculates the number of elements within node's container that have the
+  // same role as node. Returns 0 if the node is not present wihtin a container.
   int32_t SetSize() const;
-  // Returns true if the aria-posinset attribute is used in node's role
-  bool IsPosInSetUsedInRole() const;
 
   const std::string& GetInheritedStringAttribute(
       ax::mojom::StringAttribute attribute) const;
@@ -256,6 +255,17 @@ class AX_EXPORT AXNode final {
   void IdVectorToNodeVector(std::vector<int32_t>& ids,
                             std::vector<AXNode*>* nodes) const;
 
+  // Helpers for PosInset and SetSize
+  // Returns true if the PosInSet or SetSize attributes are used in node's role
+  bool IsSetSizePosInSetUsedInRole() const;
+  // Calculates 0-based index of first element with PosInSet assigned.
+  // Returns -1 if no elements assign PosInSet
+  int32_t FirstAssignedPosInSet() const;
+  // Calculates node's position relative to first PosInSet-assigned element.
+  // Returns node's 0-based index within container (relative to nodes with the
+  // same role) if no element assigned PosInSet
+  int32_t RelativePosFromFirstAssigned(int32_t earliest_index) const;
+
   OwnerTree* tree_;  // Owns this.
   int index_in_parent_;
   AXNode* parent_;

ui/accessibility/ax_tree_unittest.cc

@@ -1472,8 +1472,41 @@ TEST(AXTreeTest, ChildTreeIds) {
   EXPECT_EQ(0U, child_tree_93_nodes.size());
 }
 
-// Simple test for PosInSet and SetSize.
-TEST(AXTreeTest, GetPosInSetSetSize) {
+// Tests PosInSet and SetSize int attributes work if assigned
+TEST(AXTreeTest, TestSetSizePosInSetAssigned) {
+  AXTreeUpdate tree_update;
+  tree_update.root_id = 1;
+  tree_update.nodes.resize(4);
+  tree_update.nodes[0].id = 1;
+  tree_update.nodes[0].role = ax::mojom::Role::kList;
+  tree_update.nodes[0].child_ids = {2, 3, 4};
+  tree_update.nodes[1].id = 2;
+  tree_update.nodes[1].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[1].AddIntAttribute(ax::mojom::IntAttribute::kPosInSet, 2);
+  tree_update.nodes[1].AddIntAttribute(ax::mojom::IntAttribute::kSetSize, 12);
+  tree_update.nodes[2].id = 3;
+  tree_update.nodes[2].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[2].AddIntAttribute(ax::mojom::IntAttribute::kPosInSet, 5);
+  tree_update.nodes[2].AddIntAttribute(ax::mojom::IntAttribute::kSetSize, 12);
+  tree_update.nodes[3].id = 4;
+  tree_update.nodes[3].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[3].AddIntAttribute(ax::mojom::IntAttribute::kPosInSet, 9);
+  tree_update.nodes[3].AddIntAttribute(ax::mojom::IntAttribute::kSetSize, 12);
+  AXTree tree(tree_update);
+
+  AXNode* item1 = tree.GetFromId(2);
+  EXPECT_EQ(item1->PosInSet(), 2);
+  EXPECT_EQ(item1->SetSize(), 12);
+  AXNode* item2 = tree.GetFromId(3);
+  EXPECT_EQ(item2->PosInSet(), 5);
+  EXPECT_EQ(item2->SetSize(), 12);
+  AXNode* item3 = tree.GetFromId(4);
+  EXPECT_EQ(item3->PosInSet(), 9);
+  EXPECT_EQ(item3->SetSize(), 12);
+}
+
+// Tests that PosInSet and SetSize can be calculated if not assigned.
+TEST(AXTreeTest, TestSetSizePosInSetUnassigned) {
   AXTreeUpdate tree_update;
   tree_update.root_id = 1;
   tree_update.nodes.resize(4);
@@ -1499,41 +1532,67 @@ TEST(AXTreeTest, GetPosInSetSetSize) {
   EXPECT_EQ(item3->SetSize(), 3);
 }
 
-// A test for PosInSet and SetSize on a list containing various roles.
-TEST(AXTreeTest, GetPosInSetSetSizeDiverseList) {
+// Tests PosInSet unassigned, while SetSize assigned in container
+TEST(AXTreeTest, TestSetSizeAssignedInContainer) {
   AXTreeUpdate tree_update;
   tree_update.root_id = 1;
-  tree_update.nodes.resize(9);
+  tree_update.nodes.resize(4);
   tree_update.nodes[0].id = 1;
   tree_update.nodes[0].role = ax::mojom::Role::kList;
-  tree_update.nodes[0].child_ids = {2, 3, 4, 5, 6, 7, 8, 9};
+  tree_update.nodes[0].child_ids = {2, 3, 4};
+  tree_update.nodes[0].AddIntAttribute(ax::mojom::IntAttribute::kSetSize, 7);
   tree_update.nodes[1].id = 2;
   tree_update.nodes[1].role = ax::mojom::Role::kListItem;
   tree_update.nodes[2].id = 3;
   tree_update.nodes[2].role = ax::mojom::Role::kListItem;
   tree_update.nodes[3].id = 4;
-  tree_update.nodes[3].role = ax::mojom::Role::kMenuItem;
+  tree_update.nodes[3].role = ax::mojom::Role::kListItem;
+  AXTree tree(tree_update);
+
+  // Items should inherit SetSize from container if not specified
+  AXNode* item1 = tree.GetFromId(2);
+  EXPECT_EQ(item1->SetSize(), 7);
+  AXNode* item2 = tree.GetFromId(3);
+  EXPECT_EQ(item2->SetSize(), 7);
+  AXNode* item3 = tree.GetFromId(4);
+  EXPECT_EQ(item3->SetSize(), 7);
+}
+
+// Tests PosInSet and SetSize on a list containing various roles.
+TEST(AXTreeTest, TestSetSizePosInSetDiverseList) {
+  AXTreeUpdate tree_update;
+  tree_update.root_id = 1;
+  tree_update.nodes.resize(9);
+  tree_update.nodes[0].id = 1;
+  tree_update.nodes[0].role = ax::mojom::Role::kList;
+  tree_update.nodes[0].child_ids = {2, 3, 4, 5, 6, 7, 8, 9};
+  tree_update.nodes[1].id = 2;
+  tree_update.nodes[1].role = ax::mojom::Role::kListItem;  // 1 of 3
+  tree_update.nodes[2].id = 3;
+  tree_update.nodes[2].role = ax::mojom::Role::kMenuItem;  // 1 of 2
+  tree_update.nodes[3].id = 4;
+  tree_update.nodes[3].role = ax::mojom::Role::kListItem;  // 2 of 3
   tree_update.nodes[4].id = 5;
-  tree_update.nodes[4].role = ax::mojom::Role::kMenuItem;
+  tree_update.nodes[4].role = ax::mojom::Role::kMenuItem;  // 2 of 2
   tree_update.nodes[5].id = 6;
-  tree_update.nodes[5].role = ax::mojom::Role::kArticle;
+  tree_update.nodes[5].role = ax::mojom::Role::kArticle;  // 1 of 2
   tree_update.nodes[6].id = 7;
-  tree_update.nodes[6].role = ax::mojom::Role::kArticle;
+  tree_update.nodes[6].role = ax::mojom::Role::kArticle;  // 2 of 2
   tree_update.nodes[7].id = 8;
-  tree_update.nodes[7].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[7].role = ax::mojom::Role::kListItem;  // 3 of 3
   tree_update.nodes[8].id = 9;
-  tree_update.nodes[8].role = ax::mojom::Role::kImage;
+  tree_update.nodes[8].role = ax::mojom::Role::kImage;  // 0 of 0
   AXTree tree(tree_update);
 
   AXNode* listitem1 = tree.GetFromId(2);
   EXPECT_EQ(listitem1->PosInSet(), 1);
   EXPECT_EQ(listitem1->SetSize(), 3);
   AXNode* listitem2 = tree.GetFromId(3);
-  EXPECT_EQ(listitem2->PosInSet(), 2);
-  EXPECT_EQ(listitem2->SetSize(), 3);
+  EXPECT_EQ(listitem2->PosInSet(), 1);
+  EXPECT_EQ(listitem2->SetSize(), 2);
   AXNode* menuitem1 = tree.GetFromId(4);
-  EXPECT_EQ(menuitem1->PosInSet(), 1);
-  EXPECT_EQ(menuitem1->SetSize(), 2);
+  EXPECT_EQ(menuitem1->PosInSet(), 2);
+  EXPECT_EQ(menuitem1->SetSize(), 3);
   AXNode* menuitem2 = tree.GetFromId(5);
   EXPECT_EQ(menuitem2->PosInSet(), 2);
   EXPECT_EQ(menuitem2->SetSize(), 2);
@@ -1547,8 +1606,89 @@ TEST(AXTreeTest, GetPosInSetSetSizeDiverseList) {
   EXPECT_EQ(listitem3->PosInSet(), 3);
   EXPECT_EQ(listitem3->SetSize(), 3);
   AXNode* image1 = tree.GetFromId(9);
+  // Roles that do not use PosInSet or SetSize should return 0
   EXPECT_EQ(image1->PosInSet(), 0);
   EXPECT_EQ(image1->SetSize(), 0);
 }
 
+// Tests PosInSet and SetSize on a nested list
+TEST(AXTreeTest, TestSetSizePosInSetNestedList) {
+  AXTreeUpdate tree_update;
+  tree_update.root_id = 1;
+  tree_update.nodes.resize(7);
+  tree_update.nodes[0].id = 1;
+  tree_update.nodes[0].role = ax::mojom::Role::kList;
+  tree_update.nodes[0].child_ids = {2, 3, 4, 7};
+  tree_update.nodes[1].id = 2;
+  tree_update.nodes[1].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[2].id = 3;
+  tree_update.nodes[2].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[3].id = 4;
+  tree_update.nodes[3].role = ax::mojom::Role::kList;
+  tree_update.nodes[3].child_ids = {5, 6};
+  tree_update.nodes[4].id = 5;
+  tree_update.nodes[4].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[5].id = 6;
+  tree_update.nodes[5].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[6].id = 7;
+  tree_update.nodes[6].role = ax::mojom::Role::kListItem;
+  AXTree tree(tree_update);
+
+  AXNode* outer_item1 = tree.GetFromId(2);
+  EXPECT_EQ(outer_item1->PosInSet(), 1);
+  EXPECT_EQ(outer_item1->SetSize(), 3);
+  AXNode* outer_item2 = tree.GetFromId(3);
+  EXPECT_EQ(outer_item2->PosInSet(), 2);
+  EXPECT_EQ(outer_item2->SetSize(), 3);
+
+  // List object itself should not report posinset or setsize
+  // TODO (akihiroota): Lists should report setsize in the future
+  AXNode* inner_list = tree.GetFromId(4);
+  EXPECT_EQ(inner_list->PosInSet(), 0);
+  EXPECT_EQ(inner_list->SetSize(), 0);
+
+  AXNode* inner_item1 = tree.GetFromId(5);
+  EXPECT_EQ(inner_item1->PosInSet(), 1);
+  EXPECT_EQ(inner_item1->SetSize(), 2);
+  AXNode* inner_item2 = tree.GetFromId(6);
+  EXPECT_EQ(inner_item2->PosInSet(), 2);
+  EXPECT_EQ(inner_item2->SetSize(), 2);
+
+  AXNode* outer_item3 = tree.GetFromId(7);
+  EXPECT_EQ(outer_item3->PosInSet(), 3);
+  EXPECT_EQ(outer_item3->SetSize(), 3);
+}
+
+// Tests PosInSet can be calculated if one item specifies PosInSet, but others
+// are missing.
+TEST(AXTreeTest, TestPosInSetMissing) {
+  AXTreeUpdate tree_update;
+  tree_update.root_id = 1;
+  tree_update.nodes.resize(4);
+  tree_update.nodes[0].id = 1;
+  tree_update.nodes[0].role = ax::mojom::Role::kList;
+  tree_update.nodes[0].child_ids = {2, 3, 4};
+  tree_update.nodes[0].AddIntAttribute(ax::mojom::IntAttribute::kSetSize, 20);
+  tree_update.nodes[1].id = 2;
+  tree_update.nodes[1].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[2].id = 3;
+  tree_update.nodes[2].role = ax::mojom::Role::kListItem;
+  tree_update.nodes[2].AddIntAttribute(ax::mojom::IntAttribute::kPosInSet, 13);
+  tree_update.nodes[3].id = 4;
+  tree_update.nodes[3].role = ax::mojom::Role::kListItem;
+  AXTree tree(tree_update);
+
+  // Item1 should have pos of 12, since item2 is assigned a pos of 13
+  AXNode* item1 = tree.GetFromId(2);
+  EXPECT_EQ(item1->PosInSet(), 12);
+  EXPECT_EQ(item1->SetSize(), 20);
+  AXNode* item2 = tree.GetFromId(3);
+  EXPECT_EQ(item2->PosInSet(), 13);
+  EXPECT_EQ(item2->SetSize(), 20);
+  // Item2 should have pos of 14, since item2 is assigned a pos of 13
+  AXNode* item3 = tree.GetFromId(4);
+  EXPECT_EQ(item3->PosInSet(), 14);
+  EXPECT_EQ(item3->SetSize(), 20);
+}
+
 }  // namespace ui