Skip to content

GitLab

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

Reland "Fire WIN HIDE event only on ignored ancestor" 

This is a reland of 4856b612
The original changed was reverted due to 'win-asan' bot flaky test
for IA2_EVENT_TEXT_INSERTED/REMOVED and EVENT_OBJECT_REORDER for
my newly added test 'aria-hidden-descendants-already-ignored.html'.
It was only flaky for 'win-asan' bot and not flaky on normal win10
bot. This reland suppresses these flaky events since the tests do
not require them.

Original change's description:
> 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: Dominic Mazzoni <dmazzoni@chromium.org>
> Reviewed-by: Adam Ettenberger <Adam.Ettenberger@microsoft.com>
> Cr-Commit-Position: refs/heads/master@{#795017}

Bug: 1019420
Change-Id: Iae15521bb69645eef9e3fc4aad4b6bd64345c0f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2340282
Commit-Queue: Aaron Leventhal <aleventhal@chromium.org>
Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796380}
parent 0c437d0a
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 685 additions and 10 deletions
content/browser/accessibility/dump_accessibility_events_browsertest.cc
View file @ a2e28ea8
...@@ -544,6 +544,22 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest, ...@@ -544,6 +544,22 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
RunEventTest(FILE_PATH_LITERAL("css-display.html")); 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, IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsCSSFlexTextUpdate) { AccessibilityEventsCSSFlexTextUpdate) {
RunEventTest(FILE_PATH_LITERAL("css-flex-text-update.html")); RunEventTest(FILE_PATH_LITERAL("css-flex-text-update.html"));
...@@ -554,6 +570,11 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest, ...@@ -554,6 +570,11 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
RunEventTest(FILE_PATH_LITERAL("css-visibility.html")); 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, IN_PROC_BROWSER_TEST_P(DumpAccessibilityEventsTest,
AccessibilityEventsCSSCollapse) { AccessibilityEventsCSSCollapse) {
RunEventTest(FILE_PATH_LITERAL("css-visibility-collapse.html")); 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 @ a2e28ea8
EVENT_OBJECT_HIDE on <div#heading-root.a> role=ROLE_SYSTEM_GROUPING name="Heading" INVISIBLE level=2
content/test/data/accessibility/event/aria-hidden-descendants-already-ignored.html 0 → 100644
View file @ a2e28ea8
<!--
@WIN-DENY:IA2_EVENT_TEXT_INSERTED*
@WIN-DENY:IA2_EVENT_TEXT_REMOVED*
@WIN-DENY:EVENT_OBJECT_REORDER*
-->
<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 @ a2e28ea8
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"
content/test/data/accessibility/event/aria-hidden-descendants.html 0 → 100644
View file @ a2e28ea8
<!--
@WIN-DENY:IA2_EVENT_TEXT_INSERTED*
@WIN-DENY:IA2_EVENT_TEXT_REMOVED*
-->
<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 @ a2e28ea8
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"
content/test/data/accessibility/event/css-display-descendants.html 0 → 100644
View file @ a2e28ea8
<!--
@WIN-DENY:IA2_EVENT_TEXT_INSERTED*
@WIN-DENY:IA2_EVENT_TEXT_REMOVED*
-->
<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 @ a2e28ea8
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"
content/test/data/accessibility/event/css-visibility-descendants.html 0 → 100644
View file @ a2e28ea8
<!--
@WIN-DENY:IA2_EVENT_TEXT_INSERTED*
@WIN-DENY:IA2_EVENT_TEXT_REMOVED*
-->
<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 @ a2e28ea8
...@@ -47,6 +47,19 @@ void RemoveEvent(std::set<AXEventGenerator::EventParams>* node_events, ...@@ -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 } // namespace
AXEventGenerator::EventParams::EventParams( AXEventGenerator::EventParams::EventParams(
...@@ -665,7 +678,51 @@ bool AXEventGenerator::ShouldFireLoadEvents(AXNode* node) { ...@@ -665,7 +678,51 @@ bool AXEventGenerator::ShouldFireLoadEvents(AXNode* node) {
data.relative_bounds.bounds.height(); 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() { void AXEventGenerator::PostprocessEvents() {
std::map<AXNode*, bool> ancestor_ignored_changed_map;
auto iter = tree_events_.begin(); auto iter = tree_events_.begin();
while (iter != tree_events_.end()) { while (iter != tree_events_.end()) {
AXNode* node = iter->first; AXNode* node = iter->first;
...@@ -678,17 +735,13 @@ void AXEventGenerator::PostprocessEvents() { ...@@ -678,17 +735,13 @@ void AXEventGenerator::PostprocessEvents() {
RemoveEvent(&node_events, Event::LIVE_REGION_CHANGED); 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)) { if (HasEvent(node_events, Event::IGNORED_CHANGED)) {
RemoveEvent(&node_events, Event::CHILDREN_CHANGED); // If a node toggled its ignored state from show to hide, we only want to
RemoveEvent(&node_events, Event::DESCRIPTION_CHANGED); // fire IGNORED_CHANGED event on the top most ancestor where this ignored
RemoveEvent(&node_events, Event::NAME_CHANGED); // state change takes place and suppress all the descendants's
RemoveEvent(&node_events, Event::TEXT_ATTRIBUTE_CHANGED); // IGNORED_CHANGED events.
TrimEventsDueToAncestorIgnoredChanged(node, ancestor_ignored_changed_map);
RemoveEventsDueToIgnoredChanged(&node_events);
} }
// When the selected option in an expanded select element changes, the // When the selected option in an expanded select element changes, the
......
ui/accessibility/ax_event_generator.h
View file @ a2e28ea8
...@@ -235,6 +235,18 @@ class AX_EXPORT AXEventGenerator : public AXTreeObserver { ...@@ -235,6 +235,18 @@ class AX_EXPORT AXEventGenerator : public AXTreeObserver {
void FireActiveDescendantEvents(); void FireActiveDescendantEvents();
void FireRelationSourceEvents(AXTree* tree, AXNode* target_node); void FireRelationSourceEvents(AXTree* tree, AXNode* target_node);
bool ShouldFireLoadEvents(AXNode* 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(); void PostprocessEvents();
static void GetRestrictionStates(ax::mojom::Restriction restriction, static void GetRestrictionStates(ax::mojom::Restriction restriction,
bool* is_enabled, 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