Commit 5adf8e82 authored by Sara Kato's avatar Sara Kato Committed by Commit Bot

arc-a11y: android_focused_id_ to remain unchanged if adjusted node is

invalid.

On a focus event, AXTreeSource searches the first a11y focusable node
under the event source. When no focusable node is found, it sets the
focus on the event source node, but it invoked undesired behavior.

For instance, in ArcSettings application focus was requested, by sending a
requestFocus() event. This results in android_focused_id_ being computed
again in ax_tree_source_arc. However as this is forced focused event,
sometimes the new node is not valid. In this case, rather than default
back to the event_source (and move the focus there), the focus should
remain unchanged. Removing requestFocus() would cause undesired
behaviors in other areas of the app.

This CL adds a change, so that when no focusable node is found under the
focus event source node, we don't change focus so that it won't invoke
unexpected behavior.

Note that the a11y event of type FOCUSED can be dispatched in various
ways (e.g. via ViewParent#requestSendAccessibilityEvent is another way
to request the a11y event).
Also, even if it's not the "forced" focus event, technically
FindFirstFocusableNode can return null as we're doing different
computation than the Android framework.

This change also fixes HasText() computation in CanBeAccessibilityFocused.

AX-Relnotes: n/a.
Bug: b:156562991
Test: Manual. Open arc settings and observe no scrolling
Test: AXTreeSourceArcTest.*
Change-Id: If07e0c342dab929bf6f516a1753beda31bd9522f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2291552
Commit-Queue: Sara Kato <sarakato@chromium.org>
Reviewed-by: default avatarHiroki Sato <hirokisato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#788564}
parent c5710a19
...@@ -90,13 +90,12 @@ bool AccessibilityNodeInfoDataWrapper::IsImportantInAndroid() const { ...@@ -90,13 +90,12 @@ bool AccessibilityNodeInfoDataWrapper::IsImportantInAndroid() const {
} }
bool AccessibilityNodeInfoDataWrapper::CanBeAccessibilityFocused() const { bool AccessibilityNodeInfoDataWrapper::CanBeAccessibilityFocused() const {
// Using HasText() here is incomplete because it doesn't match the if (!IsAccessibilityFocusableContainer() && !HasAccessibilityFocusableText())
// populated ax name. However, this method is used only from AXTreeSourceArc return false;
// and not used for actual focusability computation of ChromeVox, and it's ui::AXNodeData data;
// enough to check only hasText(). Serialize(&data);
return (IsAccessibilityFocusableContainer() || // TODO (sarakato): Move name computation to a separate function.
HasAccessibilityFocusableText()) && return data.HasStringAttribute(ax::mojom::StringAttribute::kName);
HasText();
} }
bool AccessibilityNodeInfoDataWrapper::IsAccessibilityFocusableContainer() bool AccessibilityNodeInfoDataWrapper::IsAccessibilityFocusableContainer()
......
...@@ -380,8 +380,8 @@ bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) { ...@@ -380,8 +380,8 @@ bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
// 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(source_node); FindFirstFocusableNode(source_node);
android_focused_id_ = IsValid(adjusted_node) ? adjusted_node->GetId() if (IsValid(adjusted_node))
: event_data.source_id; android_focused_id_ = adjusted_node->GetId();
} }
} else if (event_data.event_type == AXEventType::VIEW_SELECTED) { } else if (event_data.event_type == AXEventType::VIEW_SELECTED) {
// In Android, VIEW_SELECTED event is dispatched in the two cases below: // In Android, VIEW_SELECTED event is dispatched in the two cases below:
......
...@@ -1050,6 +1050,7 @@ TEST_F(AXTreeSourceArcTest, SyncFocus) { ...@@ -1050,6 +1050,7 @@ TEST_F(AXTreeSourceArcTest, SyncFocus) {
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); SetProperty(node1, AXBooleanProperty::VISIBLE_TO_USER, true);
SetProperty(node1, AXStringProperty::CONTENT_DESCRIPTION, "node1");
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());
...@@ -1065,7 +1066,7 @@ TEST_F(AXTreeSourceArcTest, SyncFocus) { ...@@ -1065,7 +1066,7 @@ TEST_F(AXTreeSourceArcTest, SyncFocus) {
AXNodeInfoData* node3 = event->node_data.back().get(); AXNodeInfoData* node3 = event->node_data.back().get();
node3->id = 3; node3->id = 3;
// Initially |node1| has a focus. // Initially |node1| has focus.
CallNotifyAccessibilityEvent(event.get()); CallNotifyAccessibilityEvent(event.get());
ui::AXTreeData data; ui::AXTreeData data;
EXPECT_TRUE(CallGetTreeData(&data)); EXPECT_TRUE(CallGetTreeData(&data));
......
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