Enforce valid role prior to calling AXNodeData::SetName
Callers are expected to set a valid role prior to calling SetName. We
check for Role::kNone (the role was explicitly changed to the
presentational role) but not Role::kUnknown (the role was never set /
AKA the default).
This commit adds Role::kUnknown to the existing DCHECK and sets
roles on several Views which were missing roles.
AX-Relnotes: n/a
Bug: 1348081
Change-Id: I7511fbdfdc78944978641d19e94f368dc6d72fcc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3816142
Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Joanmarie Diggs <jdiggs@igalia.com>
Cr-Commit-Position: refs/heads/main@{#1045987}
diff --git a/ui/accessibility/ax_node_data.cc b/ui/accessibility/ax_node_data.cc
index 41b61b4..2500c1c 100644
--- a/ui/accessibility/ax_node_data.cc
+++ b/ui/accessibility/ax_node_data.cc
@@ -620,10 +620,12 @@
void AXNodeData::SetName(const std::string& name) {
// Elements with role='presentation' have Role::kNone. They should not be
- // named. This check is only relevant if the name is not empty.
+ // named. Objects with Role::kUnknown were never given a role. This check
+ // is only relevant if the name is not empty.
// TODO(accessibility): It would be nice to have a means to set the name
// and role at the same time to avoid this ordering requirement.
- DCHECK(role != ax::mojom::Role::kNone || name.empty())
+ DCHECK(name.empty() ||
+ (role != ax::mojom::Role::kNone && role != ax::mojom::Role::kUnknown))
<< "Cannot set name to '" << name << "' on class: '"
<< GetStringAttribute(ax::mojom::StringAttribute::kClassName)
<< "' because a valid role is needed to set the default NameFrom "
diff --git a/ui/accessibility/ax_node_data_unittest.cc b/ui/accessibility/ax_node_data_unittest.cc
index 2092adb..0a8c0349 100644
--- a/ui/accessibility/ax_node_data_unittest.cc
+++ b/ui/accessibility/ax_node_data_unittest.cc
@@ -366,6 +366,11 @@
TEST(AXNodeDataTest, SetName) {
AXNodeData data;
+ // SetName should not be called on a role of kUnknown. That role means the
+ // role has not yet been set, and we need a role to identify the NameFrom on
+ // objects where it has not been set. This is enforced by a DCHECK.
+ EXPECT_DCHECK_DEATH(data.SetName("no role yet"));
+
// SetName should not be called on a role of kNone. That role is used for
// presentational objects which should not be included in the accessibility
// tree. This is enforced by a DCHECK.