Commit 8c109179 authored by David Tseng's avatar David Tseng Committed by Commit Bot

Fire focus and blur events before all others in automation

ChromeVox recently began to "stutter" or duplicate output. This occurs most frequently when navigating by headings.

The underlying issue.

ChromeVox requests focus whenever it navigates. This, in turn, triggers actions in Blink. This correctly emits a focus event, along with the correct event from action annotation.

Once it finally arrives in automation, we dispatch the event in js.

In js, we suppress focus events in favor of tracking the focus computation given by AutomationInternalCustomBindings. Unfortunately, if, within the same event bundle, there is an event immediately before the focus event (either generated or not), the event triggers a synthesized focus event, before the actual focus event.

In the end, the incorrect event from value is associated with the synthesized focus event.

This has enormous impact on users as we unpredictably flood users with duplicate feedback. This seems worse on some devices (depending on the event bundle's contents and ordering).

Technical details.
What's happening:

we receive events (in order) like:
1. layout complete (event from page)
2. blur (event from action)
3. focus (event from action)
in the same event bundle.

Before any of these get fired, we unserialize all of the tree data, so the focused node is already updated as far as the tree data is concerned.

After this, we proceed to fire events.

The first one, is the layout complete. This translates into a *focus* event js-side, along with event from page, because that's what the layout complete event had set.

If we honored the actual focus event first, then we would pick up on the right event from action value.

Bug: 881495
Change-Id: I73b1afc3f1b224ade0cef29a89e00c179438a575
Reviewed-on: https://chromium-review.googlesource.com/1211106Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: David Tseng <dtseng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#589987}
parent 2aaad002
...@@ -225,8 +225,23 @@ bool AutomationAXTreeWrapper::OnAccessibilityEvents( ...@@ -225,8 +225,23 @@ bool AutomationAXTreeWrapper::OnAccessibilityEvents(
if (!is_active_profile) if (!is_active_profile)
return true; return true;
// Send all blur and focus events first. This ensures we correctly dispatch
// these events, which gets re-targetted in the js bindings and ensures it
// receives the correct value for |event_from|.
for (const auto& event : event_bundle.events) {
if (event.event_type != ax::mojom::Event::kFocus &&
event.event_type != ax::mojom::Event::kBlur)
continue;
api::automation::EventType automation_event_type =
ToAutomationEvent(event.event_type);
owner_->SendAutomationEvent(event_bundle.tree_id,
event_bundle.mouse_location, event,
automation_event_type);
}
// Send auto-generated AXEventGenerator events. // Send auto-generated AXEventGenerator events.
for (auto targeted_event : *this) { for (const auto& targeted_event : *this) {
api::automation::EventType event_type = api::automation::EventType event_type =
ToAutomationEvent(targeted_event.event_params.event); ToAutomationEvent(targeted_event.event_params.event);
if (IsEventTypeHandledByAXEventGenerator(event_type)) { if (IsEventTypeHandledByAXEventGenerator(event_type)) {
...@@ -240,7 +255,11 @@ bool AutomationAXTreeWrapper::OnAccessibilityEvents( ...@@ -240,7 +255,11 @@ bool AutomationAXTreeWrapper::OnAccessibilityEvents(
} }
ClearEvents(); ClearEvents();
for (auto event : event_bundle.events) { for (const auto& event : event_bundle.events) {
if (event.event_type == ax::mojom::Event::kFocus ||
event.event_type == ax::mojom::Event::kBlur)
continue;
api::automation::EventType automation_event_type = api::automation::EventType automation_event_type =
ToAutomationEvent(event.event_type); ToAutomationEvent(event.event_type);
......
...@@ -1733,7 +1733,7 @@ bool AutomationInternalCustomBindings::SendTreeChangeEvent( ...@@ -1733,7 +1733,7 @@ bool AutomationInternalCustomBindings::SendTreeChangeEvent(
void AutomationInternalCustomBindings::SendAutomationEvent( void AutomationInternalCustomBindings::SendAutomationEvent(
int tree_id, int tree_id,
const gfx::Point& mouse_location, const gfx::Point& mouse_location,
ui::AXEvent& event, const ui::AXEvent& event,
api::automation::EventType event_type) { api::automation::EventType event_type) {
auto event_params = std::make_unique<base::DictionaryValue>(); auto event_params = std::make_unique<base::DictionaryValue>();
event_params->SetInteger("treeID", tree_id); event_params->SetInteger("treeID", tree_id);
......
...@@ -82,7 +82,7 @@ class AutomationInternalCustomBindings : public ObjectBackedNativeHandler { ...@@ -82,7 +82,7 @@ class AutomationInternalCustomBindings : public ObjectBackedNativeHandler {
ui::AXNode* node); ui::AXNode* node);
void SendAutomationEvent(int tree_id, void SendAutomationEvent(int tree_id,
const gfx::Point& mouse_location, const gfx::Point& mouse_location,
ui::AXEvent& event, const ui::AXEvent& event,
api::automation::EventType event_type); api::automation::EventType event_type);
private: private:
......
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