Commit 777137bc authored by Hiroki Sato's avatar Hiroki Sato Committed by Commit Bot

arc-a11y: Fix wrong focus restore

CL:2398438 changed the focus handling in ARC++ accessibility and when
a user visits various windows in the same task, it intended to remember
the last focused node for each window. However, there is a bug.

ArcAccessiblityHelperService allocates chrome node id to each node and
window, and that id is different if the root window id is different.
As Android accessibility framework exposes only visible windows, even in
the same task, the same chrome node id can be reused because visible
root window can easily changes in a task.
Thus, we should not have used chrome id to remember the last focused
node, instead, we should use the android side window id.

This change fixes the above issue.

AX-Relnotes: n/a.
Bug: b:169201305 (Initial focus issue in TextEditActivity)
Test: tast arc.AccessiblityEvent (without CL:2467224)
Test: unit_test AXTreeSourceArcTest
Change-Id: If55058d9d66a636d54b73d3cd92a4bd2b5fea214
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2467738Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Commit-Queue: Hiroki Sato <hirokisato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816850}
parent 6de62f50
......@@ -153,7 +153,10 @@ void AXTreeSourceArc::SerializeNode(AccessibilityInfoDataWrapper* info_data,
void AXTreeSourceArc::NotifyAccessibilityEventInternal(
const AXEventData& event_data) {
window_id_ = event_data.window_id;
if (window_id_ != event_data.window_id) {
android_focused_id_.reset();
window_id_ = event_data.window_id;
}
is_notification_ = event_data.notification_key.has_value();
is_input_method_window_ = event_data.is_input_method_window;
......@@ -424,11 +427,9 @@ bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
// We do it for WINDOW_STATE_CHANGED event from a window or a root node.
bool from_root_or_window = (source_node && !source_node->IsNode()) ||
IsRootOfNodeTree(event_data.source_id);
auto itr = root_window_id_to_last_focus_node_id_.find(*root_id_);
if (from_root_or_window &&
itr != root_window_id_to_last_focus_node_id_.end()) {
auto itr = window_id_to_last_focus_node_id_.find(event_data.window_id);
if (from_root_or_window && itr != window_id_to_last_focus_node_id_.end())
new_focus = GetFromId(itr->second);
}
// Otherwise, try focus on the first focusable node.
if (!IsValid(new_focus))
......@@ -445,9 +446,10 @@ bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
}
if (android_focused_id_.has_value()) {
root_window_id_to_last_focus_node_id_[*root_id_] = *android_focused_id_;
window_id_to_last_focus_node_id_[event_data.window_id] =
*android_focused_id_;
} else {
root_window_id_to_last_focus_node_id_.erase(*root_id_);
window_id_to_last_focus_node_id_.erase(event_data.window_id);
}
AccessibilityInfoDataWrapper* focused_node =
......
......@@ -179,8 +179,8 @@ class AXTreeSourceArc : public ui::AXTreeSource<AccessibilityInfoDataWrapper*,
std::map<int32_t, std::string> cached_names_;
std::map<int32_t, ax::mojom::Role> cached_roles_;
// Cache of mapping from the root window id to the last focused node id.
std::map<int32_t, int32_t> root_window_id_to_last_focus_node_id_;
// Cache of mapping from the *Android* window id to the last focused node id.
std::map<int32_t, int32_t> window_id_to_last_focus_node_id_;
// Mapping from Chrome node ID to its cached computed bounds.
// This simplifies bounds calculations.
......
......@@ -718,6 +718,7 @@ TEST_F(AXTreeSourceArcTest, OnWindowStateChangedEvent) {
event->window_data = std::vector<mojom::AccessibilityWindowInfoDataPtr>();
event->window_data->push_back(AXWindowInfoData::New());
event->window_id = 1;
AXWindowInfoData* root_window = event->window_data->back().get();
root_window->window_id = 100;
root_window->root_node_id = 10;
......@@ -778,9 +779,11 @@ TEST_F(AXTreeSourceArcTest, OnWindowStateChangedEvent) {
EXPECT_EQ(node3->id, data.focus_id);
// Simulate opening another window in this task.
// |root_window->window_id| can be the same as the previous one, but
// |event->window_id| of the event are always different for different window.
// This is the same as new WINDOW_STATE_CHANGED event, so focus is at the
// first accessible node (node2).
root_window->window_id = 200;
event->window_id = 2;
event->event_type = AXEventType::WINDOW_STATE_CHANGED;
event->source_id = node1->id;
CallNotifyAccessibilityEvent(event.get());
......@@ -790,7 +793,7 @@ TEST_F(AXTreeSourceArcTest, OnWindowStateChangedEvent) {
// Simulate closing the second window and coming back to the first window.
// The focus back to the last focus node, which is node3.
root_window->window_id = 100;
event->window_id = 1;
event->event_type = AXEventType::WINDOW_STATE_CHANGED;
event->source_id = root->id;
CallNotifyAccessibilityEvent(event.get());
......
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