Change name property computation

When an Android node has content description set, this results
in the name field not being updated, resulting in the name (which is
the content description text) being read out.
The desired result is that the name (the currently entered text) be read out.

This CL changes the name computation to be based on the combined text,
content_description, label and pane_title.
For an editText, the value of the node will be set to the text
(and not included in name computation), to allow
the newly entered text to be read out correctly.

Bug: b:140316122, b:130441287
Test: arc.accessibilitySpeech (added in crrev/1813097)
Test: existing test passes (arc.accessibilityEvent/Tree)
Test: AXTreeSourceArcTest.AccessibleNameComputation*
Change-Id: I72b7a098f468b5ed7c31f2050b7bfadae68b9988
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1786363
Commit-Queue: Sara Kato <sarakato@chromium.org>
Auto-Submit: Sara Kato <sarakato@chromium.org>
Reviewed-by: David Tseng <dtseng@chromium.org>
Reviewed-by: Sara Kato <sarakato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704945}

chrome/browser/chromeos/arc/accessibility/accessibility_node_info_data_wrapper.cc

@@ -274,40 +274,57 @@ void AccessibilityNodeInfoDataWrapper::Serialize(
   // String properties.
   int labelled_by = -1;
 
-  // Accessible name computation picks the first non-empty string from content
-  // description, text, labelled by text, or pane title.
-  std::string name;
-  bool has_name = GetProperty(AXStringProperty::CONTENT_DESCRIPTION, &name);
-  if (name.empty())
-    has_name |= GetProperty(AXStringProperty::TEXT, &name);
-  if (name.empty() && GetProperty(AXIntProperty::LABELED_BY, &labelled_by)) {
+  // Accessible name computation is a concatenated string comprising of:
+  // content description, text, labelled by text, and pane title.
+  std::string text;
+  std::string content_description;
+  std::string label;
+  std::string pane_title;
+  GetProperty(AXStringProperty::CONTENT_DESCRIPTION, &content_description);
+  GetProperty(AXStringProperty::TEXT, &text);
+
+  if (GetProperty(AXIntProperty::LABELED_BY, &labelled_by)) {
     ArcAccessibilityInfoData* labelled_by_node =
         tree_source_->GetFromId(labelled_by);
     if (labelled_by_node && labelled_by_node->IsNode()) {
       ui::AXNodeData labelled_by_data;
       tree_source_->SerializeNode(labelled_by_node, &labelled_by_data);
-      has_name |= labelled_by_data.GetStringAttribute(
-          ax::mojom::StringAttribute::kName, &name);
+      labelled_by_data.GetStringAttribute(ax::mojom::StringAttribute::kName,
+                                          &label);
     }
   }
-  if (name.empty())
-    has_name |= GetProperty(AXStringProperty::PANE_TITLE, &name);
+
+  GetProperty(AXStringProperty::PANE_TITLE, &pane_title);
 
   // If it exists, set tooltip value as description on node.
   std::string tooltip;
   if (GetProperty(AXStringProperty::TOOLTIP, &tooltip)) {
     out_data->AddStringAttribute(ax::mojom::StringAttribute::kDescription,
                                  tooltip);
-    if (GetProperty(AXStringProperty::TEXT, &name)) {
-      out_data->SetName(name);
-    }
   }
 
-  if (has_name) {
-    if (out_data->role == ax::mojom::Role::kTextField)
-      out_data->AddStringAttribute(ax::mojom::StringAttribute::kValue, name);
-    else
-      out_data->SetName(name);
+  if (!text.empty() || !content_description.empty() || !label.empty() ||
+      !pane_title.empty()) {
+    // Append non empty properties to name attribute.
+    std::vector<std::string> names;
+    if (!content_description.empty())
+      names.push_back(content_description);
+    if (!label.empty())
+      names.push_back(label);
+    if (!pane_title.empty())
+      names.push_back(pane_title);
+    // For a textField, the editable text is contained in the text property, and
+    // this should be set as the value.
+    // This ensures that the edited text will be read out appropriately.
+    if (out_data->role == ax::mojom::Role::kTextField) {
+      out_data->SetValue(text);
+    } else {
+      names.push_back(text);
+    }
+    // TODO (sarakato): Exposing all possible labels for a node, may result in
+    // too much being spoken. For ARC ++, this may result in divergent behaviour
+    // from Talkback.
+    out_data->SetName(base::JoinString(names, " "));
   } else if (GetProperty(AXBooleanProperty::CLICKABLE)) {
     // Compute the name by joining all nodes with names.
     std::vector<std::string> names;

chrome/browser/chromeos/arc/accessibility/ax_tree_source_arc_unittest.cc

@@ -338,6 +338,76 @@ TEST_F(AXTreeSourceArcTest, ReorderChildrenByLayout) {
   EXPECT_EQ(10, GetDispatchedEventCount(ax::mojom::Event::kFocus));
 }
 
+TEST_F(AXTreeSourceArcTest, AccessibleNameComputationTextField) {
+  auto event = AXEventData::New();
+  event->source_id = 0;
+  event->task_id = 1;
+  event->event_type = AXEventType::VIEW_FOCUSED;
+  event->node_data.push_back(AXNodeInfoData::New());
+  AXNodeInfoData* root = event->node_data.back().get();
+  root->id = 0;
+
+  std::unique_ptr<ui::AXNodeData> data;
+  SetProperty(root, AXStringProperty::CLASS_NAME, "");
+  SetProperty(root, AXIntListProperty::CHILD_NODE_IDS, std::vector<int>({1}));
+
+  // Child.
+  event->node_data.push_back(AXNodeInfoData::New());
+  AXNodeInfoData* child1 = event->node_data.back().get();
+  child1->id = 1;
+  SetProperty(child1, AXIntListProperty::CHILD_NODE_IDS,
+              std::vector<int>({2, 3}));
+
+  // Second child.
+  // This test requires two children.
+  // see SerializeNode function in arc/a11y/ax_tree_source_arc.cc
+  event->node_data.push_back(AXNodeInfoData::New());
+  AXNodeInfoData* child2 = event->node_data.back().get();
+  child2->id = 2;
+
+  // Third child.
+  event->node_data.push_back(AXNodeInfoData::New());
+  AXNodeInfoData* child3 = event->node_data.back().get();
+  child3->id = 3;
+
+  // Populate the tree source with the data.
+  CallNotifyAccessibilityEvent(event.get());
+
+  SetProperty(child2, AXBooleanProperty::EDITABLE, true);
+  SetProperty(child2, AXStringProperty::TEXT, "foo@example.com");
+  SetProperty(child2, AXStringProperty::CONTENT_DESCRIPTION,
+              "Type your email here.");
+
+  CallSerializeNode(child2, &data);
+
+  std::string prop;
+  ASSERT_TRUE(
+      data->GetStringAttribute(ax::mojom::StringAttribute::kName, &prop));
+  EXPECT_EQ("Type your email here.", prop);
+
+  ASSERT_TRUE(
+      data->GetStringAttribute(ax::mojom::StringAttribute::kValue, &prop));
+  EXPECT_EQ("foo@example.com", prop);
+
+  ASSERT_FALSE(data->GetStringAttribute(
+      ax::mojom::StringAttribute::kDescription, &prop));
+
+  // Case for when text property is empty.
+  SetProperty(child3, AXBooleanProperty::EDITABLE, true);
+  SetProperty(child3, AXStringProperty::CONTENT_DESCRIPTION,
+              "Type your email here.");
+
+  CallSerializeNode(child3, &data);
+
+  ASSERT_TRUE(
+      data->GetStringAttribute(ax::mojom::StringAttribute::kName, &prop));
+  EXPECT_EQ("Type your email here.", prop);
+  ASSERT_TRUE(
+      data->GetStringAttribute(ax::mojom::StringAttribute::kValue, &prop));
+  ASSERT_FALSE(data->GetStringAttribute(
+      ax::mojom::StringAttribute::kDescription, &prop));
+}
+
 TEST_F(AXTreeSourceArcTest, AccessibleNameComputation) {
   auto event = AXEventData::New();
   event->source_id = 0;
@@ -374,9 +444,9 @@ TEST_F(AXTreeSourceArcTest, AccessibleNameComputation) {
   SetProperty(root, AXStringProperty::TEXT, "");
 
   CallSerializeNode(root, &data);
-  ASSERT_TRUE(
+  // With crrev/1786363, empty text on node will not set the name.
+  ASSERT_FALSE(
       data->GetStringAttribute(ax::mojom::StringAttribute::kName, &name));
-  EXPECT_EQ("", name);
 
   // Text (non-empty).
   root->string_properties->clear();
@@ -402,7 +472,7 @@ TEST_F(AXTreeSourceArcTest, AccessibleNameComputation) {
   CallSerializeNode(root, &data);
   ASSERT_TRUE(
       data->GetStringAttribute(ax::mojom::StringAttribute::kName, &name));
-  EXPECT_EQ("label content description", name);
+  EXPECT_EQ("label content description label text", name);
 
   // Name from contents.