Commit 1f299a4d authored by Hiroki Sato's avatar Hiroki Sato Committed by Chromium LUCI CQ

arc-a11y: Handle multiple windows in one task correctly

There are some cases that one task exposes multiple accessibility
windows in Android. After this change, non-root windows have a generic
container role instead of an application role so that while moving focus
between windows, ChromeVox won't announce entering or exiting the
application.

Also, we won't dispatch focus for non-focused windows. Each window has
a focused node, but previously all focus events are handled equally.
With this change, only the focused window is considered in the focus
handling logic.

AX-Relnotes: n/a.
Bug: b:150827488
Test: current unit tests pass.
Change-Id: Icd900ef1dcf84926240c6b55f000ba75a05e1ad8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2576227
Commit-Queue: Hiroki Sato <hirokisato@chromium.org>
Reviewed-by: default avatarSara Kato <sarakato@chromium.org>
Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836987}
parent 4dbb1763
......@@ -48,6 +48,7 @@ class AccessibilityInfoDataWrapper {
virtual std::string ComputeAXName(bool do_recursive) const = 0;
virtual void GetChildren(
std::vector<AccessibilityInfoDataWrapper*>* children) const = 0;
virtual int32_t GetWindowId() const = 0;
protected:
AXTreeSourceArc* tree_source_;
......
......@@ -556,6 +556,10 @@ void AccessibilityNodeInfoDataWrapper::GetChildren(
children->push_back(tree_source_->GetFromId(id));
}
int32_t AccessibilityNodeInfoDataWrapper::GetWindowId() const {
return node_ptr_->window_id;
}
bool AccessibilityNodeInfoDataWrapper::GetProperty(
AXBooleanProperty prop) const {
return arc::GetBooleanProperty(node_ptr_, prop);
......
......@@ -42,6 +42,7 @@ class AccessibilityNodeInfoDataWrapper : public AccessibilityInfoDataWrapper {
std::string ComputeAXName(bool do_recursive) const override;
void GetChildren(
std::vector<AccessibilityInfoDataWrapper*>* children) const override;
int32_t GetWindowId() const override;
mojom::AccessibilityNodeInfoData* node() { return node_ptr_; }
......
......@@ -81,7 +81,13 @@ void AccessibilityWindowInfoDataWrapper::PopulateAXRole(
out_data->role = ax::mojom::Role::kWindow;
return;
case mojom::AccessibilityWindowType::TYPE_APPLICATION:
if (tree_source_->GetRoot()->GetId() == GetId()) {
// Root of this task.
out_data->role = ax::mojom::Role::kApplication;
} else {
// A part of the main window.
out_data->role = ax::mojom::Role::kGenericContainer;
}
return;
case mojom::AccessibilityWindowType::TYPE_INPUT_METHOD:
out_data->role = ax::mojom::Role::kKeyboard;
......@@ -166,6 +172,10 @@ void AccessibilityWindowInfoDataWrapper::GetChildren(
children->push_back(tree_source_->GetFromId(window_ptr_->root_node_id));
}
int32_t AccessibilityWindowInfoDataWrapper::GetWindowId() const {
return window_ptr_->window_id;
}
bool AccessibilityWindowInfoDataWrapper::GetProperty(
mojom::AccessibilityWindowBooleanProperty prop) const {
return arc::GetBooleanProperty(window_ptr_, prop);
......
......@@ -40,6 +40,7 @@ class AccessibilityWindowInfoDataWrapper : public AccessibilityInfoDataWrapper {
std::string ComputeAXName(bool do_recursive) const override;
void GetChildren(
std::vector<AccessibilityInfoDataWrapper*>* children) const override;
int32_t GetWindowId() const override;
private:
bool GetProperty(mojom::AccessibilityWindowBooleanProperty prop) const;
......
......@@ -337,20 +337,20 @@ TEST_F(ArcAccessibilityHelperBridgeTest, WindowIdTaskIdMapping) {
event->event_type = arc::mojom::AccessibilityEventType::VIEW_FOCUSED;
event->node_data.push_back(arc::mojom::AccessibilityNodeInfoData::New());
event->node_data[0]->id = 10;
arc::SetProperty(event->node_data[0]->int_list_properties,
mojom::AccessibilityIntListProperty::CHILD_NODE_IDS,
{1, 2, 3});
event->node_data[0]->window_id = 100;
SetProperty(event->node_data[0]->int_list_properties,
mojom::AccessibilityIntListProperty::CHILD_NODE_IDS, {1, 2, 3});
for (int i = 1; i <= 3; i++) {
// This creates focusable nodes.
// TODO(hirokisato): consider mock AXTreeSourceArc.
event->node_data.push_back(arc::mojom::AccessibilityNodeInfoData::New());
event->node_data[i]->id = i;
arc::SetProperty(event->node_data[i]->boolean_properties,
event->node_data[i]->window_id = 100;
SetProperty(event->node_data[i]->boolean_properties,
mojom::AccessibilityBooleanProperty::IMPORTANCE, true);
arc::SetProperty(event->node_data[i]->boolean_properties,
mojom::AccessibilityBooleanProperty::VISIBLE_TO_USER,
true);
arc::SetProperty(event->node_data[i]->string_properties,
SetProperty(event->node_data[i]->boolean_properties,
mojom::AccessibilityBooleanProperty::VISIBLE_TO_USER, true);
SetProperty(event->node_data[i]->string_properties,
mojom::AccessibilityStringProperty::CONTENT_DESCRIPTION,
"node" + base::NumberToString(i) + " description");
}
......@@ -361,6 +361,8 @@ TEST_F(ArcAccessibilityHelperBridgeTest, WindowIdTaskIdMapping) {
event->window_data->back().get();
root_window->window_id = 100;
root_window->root_node_id = 10;
SetProperty(root_window->boolean_properties,
mojom::AccessibilityWindowBooleanProperty::FOCUSED, true);
// There's no active window.
helper_bridge->OnAccessibilityEvent(event.Clone());
......
......@@ -36,7 +36,7 @@ std::string ToLiveStatusString(mojom::AccessibilityLiveRegionType type);
template <class DataType, class PropType>
bool GetBooleanProperty(DataType* node, PropType prop) {
if (!node->boolean_properties)
if (!node || !node->boolean_properties)
return false;
auto it = node->boolean_properties->find(prop);
......
......@@ -31,6 +31,7 @@ using AXEventType = mojom::AccessibilityEventType;
using AXIntProperty = mojom::AccessibilityIntProperty;
using AXIntListProperty = mojom::AccessibilityIntListProperty;
using AXNodeInfoData = mojom::AccessibilityNodeInfoData;
using AXWindowBooleanProperty = mojom::AccessibilityWindowBooleanProperty;
using AXWindowInfoData = mojom::AccessibilityWindowInfoData;
using AXWindowIntListProperty = mojom::AccessibilityWindowIntListProperty;
......@@ -396,9 +397,20 @@ AXTreeSourceArc::GetSelectedNodeInfoFromAdapterView(
}
bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
AccessibilityInfoDataWrapper* source_node = GetFromId(event_data.source_id);
if (source_node) {
AccessibilityInfoDataWrapper* source_window =
GetFromId(source_node->GetWindowId());
if (!source_window ||
!GetBooleanProperty(source_window->GetWindow(),
AXWindowBooleanProperty::FOCUSED)) {
// Don't update focus in this task for events from non-focused window.
return true;
}
}
// TODO(hirokisato): Handle CLEAR_ACCESSIBILITY_FOCUS event.
if (event_data.event_type == AXEventType::VIEW_FOCUSED) {
AccessibilityInfoDataWrapper* source_node = GetFromId(event_data.source_id);
if (source_node && source_node->IsVisibleToUser()) {
// Sometimes Android sets focus on unfocusable node, e.g. ListView.
AccessibilityInfoDataWrapper* adjusted_node =
......@@ -410,14 +422,12 @@ bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
}
} else if (event_data.event_type == AXEventType::VIEW_ACCESSIBILITY_FOCUSED &&
UseFullFocusMode()) {
AccessibilityInfoDataWrapper* source_node = GetFromId(event_data.source_id);
if (source_node && source_node->IsVisibleToUser())
android_focused_id_ = source_node->GetId();
} else if (event_data.event_type == AXEventType::VIEW_SELECTED) {
// In Android, VIEW_SELECTED event is dispatched in the two cases below:
// 1. Changing a value in ProgressBar or TimePicker in ARC P.
// 2. Selecting an item in the context of an AdapterView.
AccessibilityInfoDataWrapper* source_node = GetFromId(event_data.source_id);
if (!source_node || !source_node->IsNode())
return false;
......@@ -438,7 +448,6 @@ bool AXTreeSourceArc::UpdateAndroidFocusedId(const AXEventData& event_data) {
// is fired from Android multiple times.
// The event of WINDOW_STATE_CHANGED is fired only once for each window
// change and use it as a trigger to move the a11y focus to the first node.
AccessibilityInfoDataWrapper* source_node = GetFromId(event_data.source_id);
AccessibilityInfoDataWrapper* new_focus = nullptr;
// If the current window has ever been visited in the current task, try
......
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