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

arc-a11y: Drop selection/focus events from invisible nodes

There is an invisible list in PlayStore app, and when the loading
completed, a selection event is dispatched from the list.
This made ChromeVox move the focus to the list when opening PlayStore.

Before http://crrev/c/2129381, this was not observed because selection
event handling was incomplete, but now observable.

This CL prevents the behavior by dropping selection and focus events
from invisible nodes.

Bug: b:154293371
Test: unit_tests --gtest_filter="AXTreeSourceArcTest.*"
Test: manual. Open PlayStore, focus no longer moves to the invisible node.
Change-Id: Icfe4c931bef53b573878c3a55223e9e553b47307
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2154358
Commit-Queue: Hiroki Sato <hirokisato@chromium.org>
Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Reviewed-by: default avatarSara Kato <sarakato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760378}
parent 3066ad47
...@@ -137,7 +137,7 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(AXEventData* event_data) { ...@@ -137,7 +137,7 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(AXEventData* event_data) {
if (event_data->event_type == AXEventType::VIEW_FOCUSED) { if (event_data->event_type == AXEventType::VIEW_FOCUSED) {
AccessibilityInfoDataWrapper* focused_node = AccessibilityInfoDataWrapper* focused_node =
GetFromId(event_data->source_id); GetFromId(event_data->source_id);
if (focused_node) { if (focused_node && focused_node->IsVisibleToUser()) {
// Sometimes Android sets focus on unfocusable node, e.g. ListView. // Sometimes Android sets focus on unfocusable node, e.g. ListView.
AccessibilityInfoDataWrapper* adjusted_node = AccessibilityInfoDataWrapper* adjusted_node =
FindFirstFocusableNode(focused_node); FindFirstFocusableNode(focused_node);
...@@ -159,7 +159,7 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(AXEventData* event_data) { ...@@ -159,7 +159,7 @@ void AXTreeSourceArc::NotifyAccessibilityEvent(AXEventData* event_data) {
if (!is_range_change) { if (!is_range_change) {
AccessibilityInfoDataWrapper* selected_node = AccessibilityInfoDataWrapper* selected_node =
GetSelectedNodeInfoFromAdapterView(event_data); GetSelectedNodeInfoFromAdapterView(event_data);
if (!selected_node) if (!selected_node || !selected_node->IsVisibleToUser())
return; return;
android_focused_id_ = selected_node->GetId(); android_focused_id_ = selected_node->GetId();
......
...@@ -993,6 +993,7 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) { ...@@ -993,6 +993,7 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) {
list->id = 1; list->id = 1;
SetProperty(list, AXBooleanProperty::FOCUSABLE, true); SetProperty(list, AXBooleanProperty::FOCUSABLE, true);
SetProperty(list, AXBooleanProperty::IMPORTANCE, true); SetProperty(list, AXBooleanProperty::IMPORTANCE, true);
SetProperty(list, AXBooleanProperty::VISIBLE_TO_USER, true);
SetProperty(list, AXIntListProperty::CHILD_NODE_IDS, SetProperty(list, AXIntListProperty::CHILD_NODE_IDS,
std::vector<int>({2, 3, 4})); std::vector<int>({2, 3, 4}));
...@@ -1010,6 +1011,7 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) { ...@@ -1010,6 +1011,7 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) {
simple_item->id = 3; simple_item->id = 3;
SetProperty(simple_item, AXBooleanProperty::FOCUSABLE, true); SetProperty(simple_item, AXBooleanProperty::FOCUSABLE, true);
SetProperty(simple_item, AXBooleanProperty::IMPORTANCE, true); SetProperty(simple_item, AXBooleanProperty::IMPORTANCE, true);
SetProperty(simple_item, AXBooleanProperty::VISIBLE_TO_USER, true);
simple_item->collection_item_info = AXCollectionItemInfoData::New(); simple_item->collection_item_info = AXCollectionItemInfoData::New();
// This node is not focusable. // This node is not focusable.
...@@ -1017,6 +1019,7 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) { ...@@ -1017,6 +1019,7 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) {
AXNodeInfoData* wrap_node = event->node_data.back().get(); AXNodeInfoData* wrap_node = event->node_data.back().get();
wrap_node->id = 4; wrap_node->id = 4;
SetProperty(wrap_node, AXBooleanProperty::IMPORTANCE, true); SetProperty(wrap_node, AXBooleanProperty::IMPORTANCE, true);
SetProperty(wrap_node, AXBooleanProperty::VISIBLE_TO_USER, true);
SetProperty(wrap_node, AXIntListProperty::CHILD_NODE_IDS, SetProperty(wrap_node, AXIntListProperty::CHILD_NODE_IDS,
std::vector<int>({5})); std::vector<int>({5}));
wrap_node->collection_item_info = AXCollectionItemInfoData::New(); wrap_node->collection_item_info = AXCollectionItemInfoData::New();
...@@ -1027,6 +1030,7 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) { ...@@ -1027,6 +1030,7 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) {
item->id = 5; item->id = 5;
SetProperty(item, AXBooleanProperty::FOCUSABLE, true); SetProperty(item, AXBooleanProperty::FOCUSABLE, true);
SetProperty(item, AXBooleanProperty::IMPORTANCE, true); SetProperty(item, AXBooleanProperty::IMPORTANCE, true);
SetProperty(item, AXBooleanProperty::VISIBLE_TO_USER, true);
// A selected event from Slider is kValueChanged. // A selected event from Slider is kValueChanged.
event->source_id = slider->id; event->source_id = slider->id;
...@@ -1055,7 +1059,14 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) { ...@@ -1055,7 +1059,14 @@ TEST_F(AXTreeSourceArcTest, OnViewSelectedEvent) {
EXPECT_TRUE(CallGetTreeData(&data)); EXPECT_TRUE(CallGetTreeData(&data));
EXPECT_EQ(simple_item->id, data.focus_id); EXPECT_EQ(simple_item->id, data.focus_id);
// An event from an invisible node is dropped.
SetProperty(simple_item, AXBooleanProperty::VISIBLE_TO_USER, false);
CallNotifyAccessibilityEvent(event.get());
EXPECT_EQ(2,
GetDispatchedEventCount(ax::mojom::Event::kFocus)); // not changed
// A selected event from non collection node is dropped. // A selected event from non collection node is dropped.
SetProperty(simple_item, AXBooleanProperty::VISIBLE_TO_USER, true);
event->source_id = item->id; event->source_id = item->id;
event->int_properties->clear(); event->int_properties->clear();
CallNotifyAccessibilityEvent(event.get()); CallNotifyAccessibilityEvent(event.get());
...@@ -1123,6 +1134,7 @@ TEST_F(AXTreeSourceArcTest, OnFocusEvent) { ...@@ -1123,6 +1134,7 @@ TEST_F(AXTreeSourceArcTest, OnFocusEvent) {
SetProperty(root, AXIntListProperty::CHILD_NODE_IDS, SetProperty(root, AXIntListProperty::CHILD_NODE_IDS,
std::vector<int>({1, 2})); std::vector<int>({1, 2}));
SetProperty(root, AXBooleanProperty::IMPORTANCE, true); SetProperty(root, AXBooleanProperty::IMPORTANCE, true);
SetProperty(root, AXBooleanProperty::VISIBLE_TO_USER, true);
root->collection_info = AXCollectionInfoData::New(); root->collection_info = AXCollectionInfoData::New();
root->collection_info->row_count = 2; root->collection_info->row_count = 2;
root->collection_info->column_count = 1; root->collection_info->column_count = 1;
...@@ -1396,6 +1408,7 @@ TEST_F(AXTreeSourceArcTest, SyncFocus) { ...@@ -1396,6 +1408,7 @@ TEST_F(AXTreeSourceArcTest, SyncFocus) {
node1->id = 1; node1->id = 1;
SetProperty(node1, AXBooleanProperty::FOCUSABLE, true); SetProperty(node1, AXBooleanProperty::FOCUSABLE, true);
SetProperty(node1, AXBooleanProperty::IMPORTANCE, true); SetProperty(node1, AXBooleanProperty::IMPORTANCE, true);
SetProperty(node1, AXBooleanProperty::VISIBLE_TO_USER, true);
node1->bounds_in_screen = gfx::Rect(0, 0, 50, 50); node1->bounds_in_screen = gfx::Rect(0, 0, 50, 50);
event->node_data.emplace_back(AXNodeInfoData::New()); event->node_data.emplace_back(AXNodeInfoData::New());
...@@ -1403,6 +1416,7 @@ TEST_F(AXTreeSourceArcTest, SyncFocus) { ...@@ -1403,6 +1416,7 @@ TEST_F(AXTreeSourceArcTest, SyncFocus) {
node2->id = 2; node2->id = 2;
SetProperty(node2, AXBooleanProperty::FOCUSABLE, true); SetProperty(node2, AXBooleanProperty::FOCUSABLE, true);
SetProperty(node2, AXBooleanProperty::IMPORTANCE, true); SetProperty(node2, AXBooleanProperty::IMPORTANCE, true);
SetProperty(node2, AXBooleanProperty::VISIBLE_TO_USER, true);
node2->bounds_in_screen = gfx::Rect(50, 50, 100, 100); node2->bounds_in_screen = gfx::Rect(50, 50, 100, 100);
// Add a child node to |node1|, but it's not an important node. // Add a child node to |node1|, but it's not an important node.
......
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