Commit dd204543 authored by Sara Kato's avatar Sara Kato Committed by Chromium LUCI CQ

arc-a11y: Create an array of text properties to be checked.

The array will ensure that the same properties are checked in
ComputeNameFromContents() and HasText().

This will address b:172611390, as the on/off status was propagated as
stateDescription (in ARC++ R), as opposed to name. By having the same
properties checked in ComputeNameFromContents() and HasText(), this will
also mean that the switch will not receive focus.

....On/Off"

Test: AccessibilityNodeInfoDataWrapperTest.NameFromTextProperties
Test: manual. Toggling hangouts switch will read "Improve Hangouts
Bug: b:172611390
Change-Id: Ibdedf1a2dcde5db1ba7d78043d056423d6f74cb0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2543527Reviewed-by: default avatarHiroki Sato <hirokisato@chromium.org>
Commit-Queue: Sara Kato <sarakato@chromium.org>
Auto-Submit: Sara Kato <sarakato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834513}
parent bb410933
...@@ -32,6 +32,9 @@ using AXRangeInfoData = mojom::AccessibilityRangeInfoData; ...@@ -32,6 +32,9 @@ using AXRangeInfoData = mojom::AccessibilityRangeInfoData;
using AXStringListProperty = mojom::AccessibilityStringListProperty; using AXStringListProperty = mojom::AccessibilityStringListProperty;
using AXStringProperty = mojom::AccessibilityStringProperty; using AXStringProperty = mojom::AccessibilityStringProperty;
constexpr mojom::AccessibilityStringProperty
AccessibilityNodeInfoDataWrapper::text_properties_[];
AccessibilityNodeInfoDataWrapper::AccessibilityNodeInfoDataWrapper( AccessibilityNodeInfoDataWrapper::AccessibilityNodeInfoDataWrapper(
AXTreeSourceArc* tree_source, AXTreeSourceArc* tree_source,
AXNodeInfoData* node) AXNodeInfoData* node)
...@@ -638,10 +641,11 @@ bool AccessibilityNodeInfoDataWrapper::HasCoveringSpan( ...@@ -638,10 +641,11 @@ bool AccessibilityNodeInfoDataWrapper::HasCoveringSpan(
} }
bool AccessibilityNodeInfoDataWrapper::HasText() const { bool AccessibilityNodeInfoDataWrapper::HasText() const {
// The same properties are checked as ComputeNameFromContentsInternal. for (const auto it : text_properties_) {
return HasNonEmptyStringProperty(node_ptr_, if (HasNonEmptyStringProperty(node_ptr_, it))
AXStringProperty::CONTENT_DESCRIPTION) || return true;
HasNonEmptyStringProperty(node_ptr_, AXStringProperty::TEXT); }
return false;
} }
bool AccessibilityNodeInfoDataWrapper::HasAccessibilityFocusableText() const { bool AccessibilityNodeInfoDataWrapper::HasAccessibilityFocusableText() const {
...@@ -678,18 +682,13 @@ void AccessibilityNodeInfoDataWrapper::ComputeNameFromContentsInternal( ...@@ -678,18 +682,13 @@ void AccessibilityNodeInfoDataWrapper::ComputeNameFromContentsInternal(
if (IsVirtualNode() || IsAccessibilityFocusableContainer()) if (IsVirtualNode() || IsAccessibilityFocusableContainer())
return; return;
// Take the name from either content description or text. It's not clear
// whether labeled by should be taken into account here.
std::string name; std::string name;
if (!GetProperty(AXStringProperty::CONTENT_DESCRIPTION, &name) || for (const auto it : text_properties_) {
name.empty()) { if (GetProperty(it, &name) && !name.empty()) {
GetProperty(AXStringProperty::TEXT, &name); // Stop when we get a name for this subtree.
} names->push_back(name);
return;
// Stop when we get a name for this subtree. }
if (!name.empty()) {
names->push_back(name);
return;
} }
// Otherwise, continue looking for a name in this subtree. // Otherwise, continue looking for a name in this subtree.
......
...@@ -90,6 +90,13 @@ class AccessibilityNodeInfoDataWrapper : public AccessibilityInfoDataWrapper { ...@@ -90,6 +90,13 @@ class AccessibilityNodeInfoDataWrapper : public AccessibilityInfoDataWrapper {
mojom::AccessibilityLiveRegionType container_live_status_ = mojom::AccessibilityLiveRegionType container_live_status_ =
mojom::AccessibilityLiveRegionType::NONE; mojom::AccessibilityLiveRegionType::NONE;
// Properties which should be checked for recursive text computation.
// It's not clear whether labeled by should be taken into account here.
static constexpr mojom::AccessibilityStringProperty text_properties_[3] = {
mojom::AccessibilityStringProperty::TEXT,
mojom::AccessibilityStringProperty::CONTENT_DESCRIPTION,
mojom::AccessibilityStringProperty::STATE_DESCRIPTION};
// This property is a cached value so that we can avoid same computation. // This property is a cached value so that we can avoid same computation.
// mutable because once the value is computed it won't change. // mutable because once the value is computed it won't change.
mutable base::Optional<bool> has_important_property_cache_; mutable base::Optional<bool> has_important_property_cache_;
......
...@@ -315,6 +315,55 @@ TEST_F(AccessibilityNodeInfoDataWrapperTest, NameFromDescendants) { ...@@ -315,6 +315,55 @@ TEST_F(AccessibilityNodeInfoDataWrapperTest, NameFromDescendants) {
data.GetStringAttribute(ax::mojom::StringAttribute::kName, &name)); data.GetStringAttribute(ax::mojom::StringAttribute::kName, &name));
} }
TEST_F(AccessibilityNodeInfoDataWrapperTest, NameFromTextProperties) {
AXNodeInfoData root;
root.id = 10;
AccessibilityNodeInfoDataWrapper root_wrapper(tree_source(), &root);
SetIdToWrapper(&root_wrapper);
SetProperty(&root, AXStringProperty::CLASS_NAME, "");
SetProperty(&root, AXBooleanProperty::IMPORTANCE, true);
SetProperty(&root, AXBooleanProperty::CLICKABLE, true);
SetProperty(&root, AXBooleanProperty::FOCUSABLE, true);
SetProperty(&root, AXIntListProperty::CHILD_NODE_IDS, std::vector<int>({1}));
AXNodeInfoData child1;
child1.id = 1;
AccessibilityNodeInfoDataWrapper child1_wrapper(tree_source(), &child1);
SetIdToWrapper(&child1_wrapper);
// Set all properties that will be used in name computation.
SetProperty(&child1, AXStringProperty::TEXT, "text");
SetProperty(&child1, AXStringProperty::CONTENT_DESCRIPTION,
"content_description");
SetProperty(&child1, AXStringProperty::STATE_DESCRIPTION,
"state_description");
AccessibilityNodeInfoDataWrapper wrapper(tree_source(), &root);
ui::AXNodeData data = CallSerialize(wrapper);
std::string name;
ASSERT_TRUE(
data.GetStringAttribute(ax::mojom::StringAttribute::kName, &name));
ASSERT_EQ("text", name);
// Unset TEXT property, and confirm that CONTENT_DESCRIPTION is used as name.
SetProperty(&child1, AXStringProperty::TEXT, "");
data = CallSerialize(wrapper);
ASSERT_TRUE(
data.GetStringAttribute(ax::mojom::StringAttribute::kName, &name));
ASSERT_EQ("content_description", name);
// Unset CONTENT_DESCRIPTION property, and confirm that STATE_DESCRIPTION is
// used as name.
SetProperty(&child1, AXStringProperty::CONTENT_DESCRIPTION, "");
data = CallSerialize(wrapper);
ASSERT_TRUE(
data.GetStringAttribute(ax::mojom::StringAttribute::kName, &name));
ASSERT_EQ("state_description", name);
}
TEST_F(AccessibilityNodeInfoDataWrapperTest, TextFieldNameAndValue) { TEST_F(AccessibilityNodeInfoDataWrapperTest, TextFieldNameAndValue) {
AXNodeInfoData node; AXNodeInfoData node;
SetProperty(&node, AXStringProperty::CLASS_NAME, ""); SetProperty(&node, AXStringProperty::CLASS_NAME, "");
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment