Skip to content

GitLab

T
tangled
Commit 4856b612 authored by Victor Fei's avatar Victor Fei Committed by Commit Bot
Browse files
Options

Fire WIN HIDE event only on ignored ancestor 

Google drive expand/collapse state is not being announced due to
excessive HIDE/SHOW/REORDER events flooding NVDA, and NVDA end up
discarding most events.

For example, if aria-hidden="true" is set on an ancestor node, which
would cause that node many of its descendants to be set to IGNORED
state which triggering too many EVENT_OBJECT_HIDE and causes NVDA
to drop the events. In reality, we only want EVENT_OBJECT_HIDE to be
generated on the ancestor node where aria-hidden="true" is set, so
not to flood and confuse NVDA with excessive events.

This change fixes the above issue by firing EVENT_OBJECT_HIDE on the
root should the entire subtree changes to IGNORED state.

Suppressing excessive SHOW/REORDER events will be addressed in follow up
patches.

AX-RelNotes: NVDA can now announce Google drive "My Drive" expand/
collapse state.

Bug: 1019420
Change-Id: I4fdabd6740630a149c1b22cdb3180d2f75807b10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2309450
Commit-Queue: Victor Fei <vicfei@microsoft.com>
Reviewed-by: default avatarDominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: default avatarAdam Ettenberger <Adam.Ettenberger@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#795017}
parent 0ad61817
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 681 additions and 10 deletions
content/browser/accessibility/dump_accessibility_events_browsertest.cc
View file @ 4856b612
......@@ -528,6 +528,22 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
RunEventTest(FILE_PATH_LITERAL("css-display.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsAriaHiddenDescendants) {
RunEventTest(FILE_PATH_LITERAL("aria-hidden-descendants.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsAriaHiddenDescendantsAlreadyIgnored) {
RunEventTest(
FILE_PATH_LITERAL("aria-hidden-descendants-already-ignored.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsCSSDisplayDescendants) {
RunEventTest(FILE_PATH_LITERAL("css-display-descendants.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsCSSFlexTextUpdate) {
RunEventTest(FILE_PATH_LITERAL("css-flex-text-update.html"));
......@@ -538,6 +554,11 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
RunEventTest(FILE_PATH_LITERAL("css-visibility.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsCSSVisibilityDescendants) {
RunEventTest(FILE_PATH_LITERAL("css-visibility-descendants.html"));
}
IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsCSSCollapse) {
RunEventTest(FILE_PATH_LITERAL("css-visibility-collapse.html"));
......
content/test/data/accessibility/event/aria-hidden-descendants-already-ignored-expected-win.txt 0 → 100644
View file @ 4856b612
EVENT_OBJECT_HIDE on <div#heading-root.a> role=ROLE_SYSTEM_GROUPING name="Heading" INVISIBLE level=2
EVENT_OBJECT_REORDER on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL
IA2_EVENT_TEXT_INSERTED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL new_text={'<obj>' start=0 end=1}
IA2_EVENT_TEXT_REMOVED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL old_text={'<obj>' start=0 end=1}
content/test/data/accessibility/event/aria-hidden-descendants-already-ignored.html 0 → 100644
View file @ 4856b612
<!DOCTYPE html>
<html>
<body>
<!-- Hide events only need to occur on the root of what's shown/hidden,
not for each descendant, with some descendants already ignored. -->
<div role="toolbar">
<div id="heading-root" role="heading" aria-label="Heading" class="a">
<div> <!-- already ignored-->
<div> <!-- already ignored -->
<div id="heading-child">
<div id="heading-grandchild"></div>
</div>
</div>
</div>
</div>
</div>
<script>
function go() {
document.querySelector('.a').setAttribute("aria-hidden", "true");
}
</script>
</body>
</html>
content/test/data/accessibility/event/aria-hidden-descendants-expected-win.txt 0 → 100644
View file @ 4856b612
EVENT_OBJECT_HIDE on <div#heading-root.a> role=ROLE_SYSTEM_GROUPING name="Heading" INVISIBLE level=2
EVENT_OBJECT_REORDER on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL
EVENT_OBJECT_SHOW on <div#banner-root.b> role=ROLE_SYSTEM_GROUPING name="Banner"
IA2_EVENT_TEXT_INSERTED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL new_text={'<obj>' start=0 end=1}
IA2_EVENT_TEXT_REMOVED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL old_text={'<obj>' start=0 end=1}
content/test/data/accessibility/event/aria-hidden-descendants.html 0 → 100644
View file @ 4856b612
<!DOCTYPE html>
<html>
<body>
<!-- Show/hide events only need to occur on the root of what's shown/hidden,
not for each descendant -->
<div role="toolbar">
<div id="heading-root" role="heading" aria-label="Heading" class="a">
<div id="heading-child">
<div id="heading-grandchild"></div>
</div>
</div>
<div id="banner-root" role="banner" aria-label="Banner" aria-hidden="true" class="b">
<div id="banner-child">
<div id="banner-grandchild"></div>
</div>
</div>
</div>
<script>
function go() {
document.querySelector('.a').setAttribute("aria-hidden", "true");
document.querySelector('.b').removeAttribute("aria-hidden");
}
</script>
</body>
</html>
content/test/data/accessibility/event/css-display-descendants-expected-win.txt 0 → 100644
View file @ 4856b612
EVENT_OBJECT_HIDE on <div#heading-root.a> role=ROLE_SYSTEM_GROUPING name="Heading" level=2
EVENT_OBJECT_REORDER on <div#banner-child> role=ROLE_SYSTEM_GROUPING
EVENT_OBJECT_REORDER on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL
EVENT_OBJECT_SHOW on <div#banner-root.b> role=ROLE_SYSTEM_GROUPING name="Banner"
IA2_EVENT_TEXT_INSERTED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL new_text={'<obj>' start=0 end=1}
IA2_EVENT_TEXT_REMOVED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL old_text={'<obj>' start=0 end=1}
content/test/data/accessibility/event/css-display-descendants.html 0 → 100644
View file @ 4856b612
<!DOCTYPE html>
<html>
<head>
<style>
.a { display: block; }
.b { display: none; }
body.flip .a { display: none; }
body.flip .b { display: block; }
</style>
</head>
<body>
<!-- Show/hide events only need to occur on the root of what's shown/hidden,
not for each descendant -->
<div role="toolbar">
<div id="heading-root" role="heading" aria-label="Heading" class="a">
<div id="heading-child">
<div id="heading-grandchild"></div>
</div>
</div>
<div id="banner-root" role="banner" aria-label="Banner" class="b">
<div id="banner-child">
<div id="banner-grandchild"></div>
</div>
</div>
</div>
<script>
function go() {
document.body.className = 'flip';
}
</script>
</body>
</html>
content/test/data/accessibility/event/css-visibility-descendants-expected-win.txt 0 → 100644
View file @ 4856b612
EVENT_OBJECT_HIDE on <div#heading-root.a> role=ROLE_SYSTEM_GROUPING name="Heading" INVISIBLE level=2
EVENT_OBJECT_REORDER on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL
EVENT_OBJECT_SHOW on <div#banner-root.b> role=ROLE_SYSTEM_GROUPING name="Banner"
IA2_EVENT_TEXT_INSERTED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL new_text={'<obj>' start=0 end=1}
IA2_EVENT_TEXT_REMOVED on <div> role=ROLE_SYSTEM_TOOLBAR IA2_STATE_HORIZONTAL old_text={'<obj>' start=0 end=1}
content/test/data/accessibility/event/css-visibility-descendants.html 0 → 100644
View file @ 4856b612
<!DOCTYPE html>
<html>
<head>
<style>
.a { visibility: visible; }
.b { visibility: hidden; }
body.flip .a { visibility: hidden; }
body.flip .b { visibility: visible; }
</style>
</head>
<body>
<!-- Show/hide events only need to occur on the root of what's shown/hidden,
not for each descendant -->
<div role="toolbar">
<div id="heading-root" role="heading" aria-label="Heading" class="a">
<div id="heading-child">
<div id="heading-grandchild"></div>
</div>
</div>
<div id="banner-root" role="banner" aria-label="Banner" class="b">
<div id="banner-child">
<div id="banner-grandchild"></div>
</div>
</div>
</div>
<script>
function go() {
document.body.className = 'flip';
}
</script>
</body>
</html>
ui/accessibility/ax_event_generator.cc
View file @ 4856b612
......@@ -47,6 +47,19 @@ void RemoveEvent(std::set<AXEventGenerator::EventParams>* node_events,
}
}
// If a node toggled its ignored state, don't also fire children-changed because
// platforms likely will do that in response to ignored-changed.
// Suppress name- and description-changed because those can be emitted as a side
// effect of calculating alternative text values for a newly-displayed object.
// Ditto for text attributes such as foreground and background colors.
void RemoveEventsDueToIgnoredChanged(
std::set<AXEventGenerator::EventParams>* node_events) {
RemoveEvent(node_events, AXEventGenerator::Event::CHILDREN_CHANGED);
RemoveEvent(node_events, AXEventGenerator::Event::DESCRIPTION_CHANGED);
RemoveEvent(node_events, AXEventGenerator::Event::NAME_CHANGED);
RemoveEvent(node_events, AXEventGenerator::Event::TEXT_ATTRIBUTE_CHANGED);
}
} // namespace
AXEventGenerator::EventParams::EventParams(
......@@ -665,7 +678,51 @@ bool AXEventGenerator::ShouldFireLoadEvents(AXNode* node) {
data.relative_bounds.bounds.height();
}
void AXEventGenerator::TrimEventsDueToAncestorIgnoredChanged(
AXNode* node,
std::map<AXNode*, bool>& ancestor_ignored_changed_map) {
DCHECK(node);
// Recursively compute and cache ancestor ignored changed results in
// |ancestor_ignored_changed_map|, if |node|'s ancestors have become ignored
// and the ancestor's ignored changed results have not been cached.
if (node->parent() &&
!base::Contains(ancestor_ignored_changed_map, node->parent())) {
TrimEventsDueToAncestorIgnoredChanged(node->parent(),
ancestor_ignored_changed_map);
}
// If an ancestor of |node| changed to ignored state, update the corresponding
// entry in the map for |node| based on the ancestor result (i.e. if an
// ancestor changed to ignored state, set the entry in the map to true for the
// current node). If |node|'s state changed to ignored as well, we want to
// remove its IGNORED_CHANGED event.
const auto& map_iter = ancestor_ignored_changed_map.find(node->parent());
const auto& events_iter = tree_events_.find(node);
if (map_iter != ancestor_ignored_changed_map.end() && map_iter->second) {
ancestor_ignored_changed_map.insert(std::make_pair(node, true));
if (node->IsIgnored() && events_iter != tree_events_.end()) {
RemoveEvent(&(events_iter->second), Event::IGNORED_CHANGED);
RemoveEventsDueToIgnoredChanged(&(events_iter->second));
}
return;
}
// If ignored changed results are not cached, calculate the corresponding
// entry for |node| in the map using the ignored states and events of |node|.
if (events_iter != tree_events_.end() &&
HasEvent(events_iter->second, Event::IGNORED_CHANGED) &&
node->IsIgnored()) {
ancestor_ignored_changed_map.insert(std::make_pair(node, true));
return;
}
ancestor_ignored_changed_map.insert(std::make_pair(node, false));
}
void AXEventGenerator::PostprocessEvents() {
std::map<AXNode*, bool> ancestor_ignored_changed_map;
auto iter = tree_events_.begin();
while (iter != tree_events_.end()) {
AXNode* node = iter->first;
......@@ -678,17 +735,13 @@ void AXEventGenerator::PostprocessEvents() {
RemoveEvent(&node_events, Event::LIVE_REGION_CHANGED);
}
// If a node toggled its ignored state, don't also fire children-changed
// because platforms likely will do that in response to ignored-changed.
// Suppress name- and description-changed because those can be emitted
// as a side effect of calculating alternative text values for a newly-
// displayed object. Ditto for text attributes such as foreground and
// background colors.
if (HasEvent(node_events, Event::IGNORED_CHANGED)) {
RemoveEvent(&node_events, Event::CHILDREN_CHANGED);
RemoveEvent(&node_events, Event::DESCRIPTION_CHANGED);
RemoveEvent(&node_events, Event::NAME_CHANGED);
RemoveEvent(&node_events, Event::TEXT_ATTRIBUTE_CHANGED);
// If a node toggled its ignored state from show to hide, we only want to
// fire IGNORED_CHANGED event on the top most ancestor where this ignored
// state change takes place and suppress all the descendants's
// IGNORED_CHANGED events.
TrimEventsDueToAncestorIgnoredChanged(node, ancestor_ignored_changed_map);
RemoveEventsDueToIgnoredChanged(&node_events);
}
// When the selected option in an expanded select element changes, the
......
ui/accessibility/ax_event_generator.h
View file @ 4856b612
......@@ -235,6 +235,18 @@ class AX_EXPORT AXEventGenerator : public AXTreeObserver {
void FireActiveDescendantEvents();
void FireRelationSourceEvents(AXTree* tree, AXNode* target_node);
bool ShouldFireLoadEvents(AXNode* node);
// Remove excessive events for a tree update containing node.
// We remove certain events on a node when it changes to IGNORED state and one
// of the node's ancestor has also changed to IGNORED in the same tree update.
// |ancestor_has_ignored_map| contains if a node's ancestor has changed to
// IGNORED state.
// Map's key is: an ax node.
// Map's value is:
// - True if an ancestor of node changed to IGNORED state.
// - False if no ancestor of node changed to IGNORED state.
void TrimEventsDueToAncestorIgnoredChanged(
AXNode* node,
std::map<AXNode*, bool>& ancestor_has_ignored_map);
void PostprocessEvents();
static void GetRestrictionStates(ax::mojom::Restriction restriction,
bool* is_enabled,
......
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