Commit e26092f5 authored by Hiroki Sato's avatar Hiroki Sato Committed by Commit Bot

Don't ignore nodes from virtual views.

Currently WebView nodes are explicitly not ignored and not sorted in ARC
accessibility. This rule should be applied to nodes from
AccessibilityNodeProvider, too.

This CL adds is_virtual_node in accessibility_helper.mojom and
AccessibilityInfoData.IsVirtualView(), replacing current IsWebViewNode().

Bug; b:145262470

Bug: b:151130475
Bug: crbug:1053397
Test: unit_tests --gtest_filter="AXTreeSourceArcTest.*"
Test: manual. Nodes from WebViews, Pickers, and Amazon Kindle app are correctly populated, works with Android side change (http://ag/10662400).
Change-Id: I216bf6f26cb8e911ce0fe899400d503db6073be4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2102288Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarSara Kato <sarakato@chromium.org>
Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Commit-Queue: Hiroki Sato <hirokisato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751313}
parent 88440166
......@@ -34,6 +34,7 @@ class AccessibilityInfoDataWrapper {
virtual int32_t GetId() const = 0;
virtual const gfx::Rect GetBounds() const = 0;
virtual bool IsVisibleToUser() const = 0;
virtual bool IsVirtualNode() const = 0;
virtual bool CanBeAccessibilityFocused() const = 0;
virtual void PopulateAXRole(ui::AXNodeData* out_data) const = 0;
virtual void PopulateAXState(ui::AXNodeData* out_data) const = 0;
......
......@@ -66,6 +66,10 @@ bool AccessibilityNodeInfoDataWrapper::IsVisibleToUser() const {
return GetProperty(AXBooleanProperty::VISIBLE_TO_USER);
}
bool AccessibilityNodeInfoDataWrapper::IsVirtualNode() const {
return node_ptr_->is_virtual_node;
}
bool AccessibilityNodeInfoDataWrapper::CanBeAccessibilityFocused() const {
// An important node with a non-generic role and:
// - actionable nodes
......
......@@ -30,6 +30,7 @@ class AccessibilityNodeInfoDataWrapper : public AccessibilityInfoDataWrapper {
int32_t GetId() const override;
const gfx::Rect GetBounds() const override;
bool IsVisibleToUser() const override;
bool IsVirtualNode() const override;
bool CanBeAccessibilityFocused() const override;
void PopulateAXRole(ui::AXNodeData* out_data) const override;
void PopulateAXState(ui::AXNodeData* out_data) const override;
......
......@@ -44,6 +44,10 @@ bool AccessibilityWindowInfoDataWrapper::IsVisibleToUser() const {
return true;
}
bool AccessibilityWindowInfoDataWrapper::IsVirtualNode() const {
return false;
}
bool AccessibilityWindowInfoDataWrapper::CanBeAccessibilityFocused() const {
// Windows are too generic to be Accessibility focused in Chrome, although
// they can be Accessibility focused in Android by virtue of having
......
......@@ -28,6 +28,7 @@ class AccessibilityWindowInfoDataWrapper : public AccessibilityInfoDataWrapper {
int32_t GetId() const override;
const gfx::Rect GetBounds() const override;
bool IsVisibleToUser() const override;
bool IsVirtualNode() const override;
bool CanBeAccessibilityFocused() const override;
void PopulateAXRole(ui::AXNodeData* out_data) const override;
void PopulateAXState(ui::AXNodeData* out_data) const override;
......
......@@ -139,33 +139,8 @@ bool IsImportantInAndroid(AXNodeInfoData* node) {
if (!node)
return false;
if (GetBooleanProperty(node, AXBooleanProperty::IMPORTANCE))
return true;
// WebView and its child nodes do not have accessibility importance set.
// This logic can be removed once the change in crrev/c/1890402 lands
// in all ARC containers.
return IsWebViewNode(node);
}
bool IsWebViewNode(AXNodeInfoData* node) {
if (!node)
return false;
if (HasStandardAction(node, AXActionType::NEXT_HTML_ELEMENT) ||
HasStandardAction(node, AXActionType::PREVIOUS_HTML_ELEMENT))
return true;
std::string chrome_role;
if (GetProperty(node->string_properties, AXStringProperty::CHROME_ROLE,
&chrome_role)) {
ax::mojom::Role role_value = ui::ParseRole(chrome_role.c_str());
if (role_value == ax::mojom::Role::kWebView ||
role_value == ax::mojom::Role::kRootWebArea)
return true;
}
return false;
return node->is_virtual_node ||
GetBooleanProperty(node, AXBooleanProperty::IMPORTANCE);
}
bool HasImportantProperty(AXNodeInfoData* node) {
......
......@@ -25,8 +25,6 @@ std::string ToLiveStatusString(mojom::AccessibilityLiveRegionType type);
bool IsImportantInAndroid(mojom::AccessibilityNodeInfoData* node);
bool IsWebViewNode(mojom::AccessibilityNodeInfoData* node);
bool HasImportantProperty(mojom::AccessibilityNodeInfoData* node);
bool HasStandardAction(mojom::AccessibilityNodeInfoData* node,
......
......@@ -588,12 +588,15 @@ void AXTreeSourceArc::GetChildren(
if (out_children->empty())
return;
if (IsWebViewNode(info_data->GetNode()))
if (info_data->IsVirtualNode())
return;
std::map<int32_t, size_t> id_to_index;
for (size_t i = 0; i < out_children->size(); i++)
for (size_t i = 0; i < out_children->size(); i++) {
if (out_children->at(i)->IsVirtualNode())
return;
id_to_index[out_children->at(i)->GetId()] = i;
}
// Sort children based on their enclosing bounding rectangles, based on their
// descendants.
......
......@@ -1188,7 +1188,7 @@ TEST_F(AXTreeSourceArcTest, SerializeAndUnserialize) {
EXPECT_EQ(1U, tree()->GetFromId(10)->GetUnignoredChildCount());
}
TEST_F(AXTreeSourceArcTest, SerializeWebView) {
TEST_F(AXTreeSourceArcTest, SerializeVirtualNode) {
auto event = AXEventData::New();
event->source_id = 10;
event->task_id = 1;
......@@ -1219,6 +1219,7 @@ TEST_F(AXTreeSourceArcTest, SerializeWebView) {
AXNodeInfoData* button1 = event->node_data.back().get();
button1->id = 2;
button1->bounds_in_screen = gfx::Rect(0, 0, 50, 50);
button1->is_virtual_node = true;
SetProperty(button1, AXStringProperty::CLASS_NAME, ui::kAXButtonClassname);
SetProperty(button1, AXBooleanProperty::VISIBLE_TO_USER, true);
SetProperty(
......@@ -1231,6 +1232,7 @@ TEST_F(AXTreeSourceArcTest, SerializeWebView) {
AXNodeInfoData* button2 = event->node_data.back().get();
button2->id = 3;
button2->bounds_in_screen = gfx::Rect(0, 0, 100, 100);
button2->is_virtual_node = true;
SetProperty(button2, AXStringProperty::CLASS_NAME, ui::kAXButtonClassname);
SetProperty(button2, AXBooleanProperty::VISIBLE_TO_USER, true);
SetProperty(
......
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Next MinVersion: 20
// Next MinVersion: 21
module arc.mojom;
......@@ -288,6 +288,7 @@ struct AccessibilityNodeInfoData {
[MinVersion=5] AccessibilityCollectionItemInfoData? collection_item_info;
[MinVersion=5] AccessibilityRangeInfoData? range_info;
[MinVersion=12] int32 window_id;
[MinVersion=20] bool is_virtual_node;
};
// AccessibilityWindowInfoData is a struct to contain info about
......
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