[go: nahoru, domu]

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.