Preserve author-specified reference order in AtkRelationSet
BrowserAccessibility::GetTargetNodesForRelation was converting the
correctly-ordered vector of AxIds into a set, causing the target order
to be based on when the accessible object was created. This order does
not necessarily correspond to the position in the accessibility tree,
let alone to the author-provided order.
This situation wasn't problematic because of the few ARIA properties
whose value is an ID reference list, none were likely being used for
assistive technology navigation. Because aria-details is changing to
support multiple targets for which ATs will be expected to provide
navigation, we need to start preserving the author-specified reference
order.
There are only a few properties for which there are multiple targets,
and the list of referenced IDs for those properties is expected to be
quite small. Thus converting the vector to a set doesn't buy us much.
Therefore, return the target nodes as a vector, checking for duplicate
AxIds.
Bug: 1047988
Change-Id: I8352a073b71cc1863cbd25326d0cd48a4abd3d2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2034690
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Joanmarie Diggs <jdiggs@igalia.com>
Cr-Commit-Position: refs/heads/master@{#738216}
diff --git a/content/browser/accessibility/browser_accessibility.cc b/content/browser/accessibility/browser_accessibility.cc
index 6c33dd8..bf13061 100644
--- a/content/browser/accessibility/browser_accessibility.cc
+++ b/content/browser/accessibility/browser_accessibility.cc
@@ -1312,16 +1312,28 @@
return GetFromNodeID(target_id);
}
-std::set<ui::AXPlatformNode*> BrowserAccessibility::GetTargetNodesForRelation(
+std::vector<ui::AXPlatformNode*>
+BrowserAccessibility::GetTargetNodesForRelation(
ax::mojom::IntListAttribute attr) {
DCHECK(ui::IsNodeIdIntListAttribute(attr));
std::vector<int32_t> target_ids;
if (!GetIntListAttribute(attr, &target_ids))
- return std::set<ui::AXPlatformNode*>();
+ return std::vector<ui::AXPlatformNode*>();
- std::set<int32_t> target_id_set(target_ids.begin(), target_ids.end());
- return GetNodesForNodeIdSet(target_id_set);
+ // If we use std::set to eliminate duplicates, the resulting set will be
+ // sorted by the id and we will lose the original order provided by the
+ // author which may be of interest to ATs. The number of ids should be small.
+
+ std::vector<ui::AXPlatformNode*> nodes;
+ for (int32_t target_id : target_ids) {
+ if (ui::AXPlatformNode* node = GetFromNodeID(target_id)) {
+ if (std::find(nodes.begin(), nodes.end(), node) == nodes.end())
+ nodes.push_back(node);
+ }
+ }
+
+ return nodes;
}
std::set<ui::AXPlatformNode*> BrowserAccessibility::GetReverseRelations(
diff --git a/content/browser/accessibility/browser_accessibility.h b/content/browser/accessibility/browser_accessibility.h
index e621f00..599e5bfe 100644
--- a/content/browser/accessibility/browser_accessibility.h
+++ b/content/browser/accessibility/browser_accessibility.h
@@ -536,7 +536,7 @@
bool HasVisibleCaretOrSelection() const override;
ui::AXPlatformNode* GetTargetNodeForRelation(
ax::mojom::IntAttribute attr) override;
- std::set<ui::AXPlatformNode*> GetTargetNodesForRelation(
+ std::vector<ui::AXPlatformNode*> GetTargetNodesForRelation(
ax::mojom::IntListAttribute attr) override;
std::set<ui::AXPlatformNode*> GetReverseRelations(
ax::mojom::IntAttribute attr) override;
diff --git a/ui/accessibility/platform/ax_platform_node_auralinux.cc b/ui/accessibility/platform/ax_platform_node_auralinux.cc
index 814853e..fc273df 100644
--- a/ui/accessibility/platform/ax_platform_node_auralinux.cc
+++ b/ui/accessibility/platform/ax_platform_node_auralinux.cc
@@ -2972,13 +2972,12 @@
// Now we do the same for each possible relation defined by an
// IntListAttribute. In this case we need to handle each target in the list.
- for (unsigned i = 0; i < G_N_ELEMENTS(kIntListRelations); i++) {
- const AtkIntListRelation& relation = kIntListRelations[i];
-
- std::set<AXPlatformNode*> targets =
+ for (const auto& relation : kIntListRelations) {
+ std::vector<AXPlatformNode*> targets =
GetDelegate()->GetTargetNodesForRelation(relation.attribute);
- for (AXPlatformNode* target : targets)
+ for (AXPlatformNode* target : targets) {
AddRelationToSet(relation_set, relation.relation, target);
+ }
if (!relation.reverse_relation.has_value())
continue;
diff --git a/ui/accessibility/platform/ax_platform_node_auralinux_unittest.cc b/ui/accessibility/platform/ax_platform_node_auralinux_unittest.cc
index 1d665dcf..fe7462da 100644
--- a/ui/accessibility/platform/ax_platform_node_auralinux_unittest.cc
+++ b/ui/accessibility/platform/ax_platform_node_auralinux_unittest.cc
@@ -2155,6 +2155,89 @@
ATK_RELATION_LABELLED_BY, ATK_RELATION_LABEL_FOR);
}
+TEST_F(AXPlatformNodeAuraLinuxTest, TestAtkRelationsTargetIndex) {
+ AXNodeData root;
+ root.id = 1;
+ root.role = ax::mojom::Role::kRootWebArea;
+
+ AXNodeData label1;
+ label1.id = 2;
+ label1.role = ax::mojom::Role::kStaticText;
+ root.child_ids.push_back(2);
+
+ AXNodeData label2;
+ label2.id = 3;
+ label2.role = ax::mojom::Role::kList;
+ root.child_ids.push_back(3);
+
+ AXNodeData label3;
+ label3.id = 4;
+ label3.role = ax::mojom::Role::kTable;
+ root.child_ids.push_back(4);
+
+ AXNodeData button1;
+ button1.id = 5;
+ button1.role = ax::mojom::Role::kButton;
+ button1.AddIntListAttribute(ax::mojom::IntListAttribute::kLabelledbyIds,
+ {2, 3, 4});
+ root.child_ids.push_back(5);
+
+ AXNodeData button2;
+ button2.id = 6;
+ button2.role = ax::mojom::Role::kButton;
+ button2.AddIntListAttribute(ax::mojom::IntListAttribute::kLabelledbyIds,
+ {4, 3, 2});
+ root.child_ids.push_back(6);
+
+ AXNodeData button3;
+ button3.id = 7;
+ button3.role = ax::mojom::Role::kButton;
+ button3.AddIntListAttribute(ax::mojom::IntListAttribute::kLabelledbyIds,
+ {4, 4, 2, 2, 3});
+ root.child_ids.push_back(7);
+
+ Init(root, label1, label2, label3, button1, button2, button3);
+
+ auto test_index = [&](AtkObject* source, AtkObject* target,
+ AtkRelationType relation_type, int index) {
+ AtkRelationSet* relation_set = atk_object_ref_relation_set(source);
+ ASSERT_TRUE(atk_relation_set_contains(relation_set, relation_type));
+
+ AtkRelation* relation =
+ atk_relation_set_get_relation_by_type(relation_set, relation_type);
+ GPtrArray* targets = atk_relation_get_target(relation);
+ ASSERT_TRUE(ATK_IS_OBJECT(g_ptr_array_index(targets, index)));
+ ASSERT_TRUE(ATK_OBJECT(g_ptr_array_index(targets, index)) == target);
+
+ g_object_unref(G_OBJECT(relation_set));
+ };
+
+ AtkObject* root_atk_object(GetRootAtkObject());
+ EXPECT_TRUE(ATK_IS_OBJECT(root_atk_object));
+ g_object_ref(root_atk_object);
+
+ AtkObject* atk_label1(AtkObjectFromNode(GetRootNode()->children()[0]));
+ AtkObject* atk_label2(AtkObjectFromNode(GetRootNode()->children()[1]));
+ AtkObject* atk_label3(AtkObjectFromNode(GetRootNode()->children()[2]));
+ AtkObject* atk_button1(AtkObjectFromNode(GetRootNode()->children()[3]));
+ AtkObject* atk_button2(AtkObjectFromNode(GetRootNode()->children()[4]));
+ AtkObject* atk_button3(AtkObjectFromNode(GetRootNode()->children()[5]));
+
+ test_index(atk_button1, atk_label1, ATK_RELATION_LABELLED_BY, 0);
+ test_index(atk_button1, atk_label2, ATK_RELATION_LABELLED_BY, 1);
+ test_index(atk_button1, atk_label3, ATK_RELATION_LABELLED_BY, 2);
+
+ test_index(atk_button2, atk_label3, ATK_RELATION_LABELLED_BY, 0);
+ test_index(atk_button2, atk_label2, ATK_RELATION_LABELLED_BY, 1);
+ test_index(atk_button2, atk_label1, ATK_RELATION_LABELLED_BY, 2);
+
+ test_index(atk_button3, atk_label3, ATK_RELATION_LABELLED_BY, 0);
+ test_index(atk_button3, atk_label1, ATK_RELATION_LABELLED_BY, 1);
+ test_index(atk_button3, atk_label2, ATK_RELATION_LABELLED_BY, 2);
+
+ g_object_unref(root_atk_object);
+}
+
TEST_F(AXPlatformNodeAuraLinuxTest, TestAtkTextTextFieldGetNSelectionsZero) {
Init(BuildTextField());
AtkObject* root_atk_object(GetRootAtkObject());
diff --git a/ui/accessibility/platform/ax_platform_node_delegate.h b/ui/accessibility/platform/ax_platform_node_delegate.h
index 83c36e8..cfd65dc 100644
--- a/ui/accessibility/platform/ax_platform_node_delegate.h
+++ b/ui/accessibility/platform/ax_platform_node_delegate.h
@@ -252,9 +252,9 @@
ax::mojom::IntAttribute attr) = 0;
// Given a node ID attribute (one where IsNodeIdIntListAttribute is true),
- // return a set of all target nodes for which this delegate's node has that
+ // return a vector of all target nodes for which this delegate's node has that
// relationship attribute.
- virtual std::set<AXPlatformNode*> GetTargetNodesForRelation(
+ virtual std::vector<AXPlatformNode*> GetTargetNodesForRelation(
ax::mojom::IntListAttribute attr) = 0;
// Given a node ID attribute (one where IsNodeIdIntAttribute is true), return
diff --git a/ui/accessibility/platform/ax_platform_node_delegate_base.cc b/ui/accessibility/platform/ax_platform_node_delegate_base.cc
index 1fd4f6e..71c13ab9 100644
--- a/ui/accessibility/platform/ax_platform_node_delegate_base.cc
+++ b/ui/accessibility/platform/ax_platform_node_delegate_base.cc
@@ -500,15 +500,27 @@
return nodes;
}
-std::set<AXPlatformNode*> AXPlatformNodeDelegateBase::GetTargetNodesForRelation(
+std::vector<AXPlatformNode*>
+AXPlatformNodeDelegateBase::GetTargetNodesForRelation(
ax::mojom::IntListAttribute attr) {
DCHECK(IsNodeIdIntListAttribute(attr));
std::vector<int32_t> target_ids;
if (!GetData().GetIntListAttribute(attr, &target_ids))
- return std::set<AXPlatformNode*>();
+ return std::vector<AXPlatformNode*>();
- std::set<int32_t> target_id_set(target_ids.begin(), target_ids.end());
- return GetNodesForNodeIds(target_id_set);
+ // If we use std::set to eliminate duplicates, the resulting set will be
+ // sorted by the id and we will lose the original order which may be of
+ // interest to ATs. The number of ids should be small.
+
+ std::vector<ui::AXPlatformNode*> nodes;
+ for (int32_t target_id : target_ids) {
+ if (ui::AXPlatformNode* node = GetFromNodeID(target_id)) {
+ if (std::find(nodes.begin(), nodes.end(), node) == nodes.end())
+ nodes.push_back(node);
+ }
+ }
+
+ return nodes;
}
std::set<AXPlatformNode*> AXPlatformNodeDelegateBase::GetReverseRelations(
diff --git a/ui/accessibility/platform/ax_platform_node_delegate_base.h b/ui/accessibility/platform/ax_platform_node_delegate_base.h
index b795519..2e31b849 100644
--- a/ui/accessibility/platform/ax_platform_node_delegate_base.h
+++ b/ui/accessibility/platform/ax_platform_node_delegate_base.h
@@ -177,9 +177,9 @@
ax::mojom::IntAttribute attr) override;
// Given a node ID attribute (one where IsNodeIdIntListAttribute is true),
- // return a set of all target nodes for which this delegate's node has that
+ // return a vector of all target nodes for which this delegate's node has that
// relationship attribute.
- std::set<AXPlatformNode*> GetTargetNodesForRelation(
+ std::vector<AXPlatformNode*> GetTargetNodesForRelation(
ax::mojom::IntListAttribute attr) override;
// Given a node ID attribute (one where IsNodeIdIntAttribute is true), return
