- Revision
- 294186
- Author
- tyle...@apple.com
- Date
- 2022-05-13 21:37:21 -0700 (Fri, 13 May 2022)
Log Message
Infinite recursion caused by call to accessibilityIsIgnored in the midst of AccessibilityObject::ignoredFromModalPresence
https://bugs.webkit.org/show_bug.cgi?id=240365
Reviewed by Chris Fleizach.
Source/WebCore:
We can get infinite recursion when accessibilityIsIgnored is called as
part of computing AccessibilityObject::ignoredFromModalPresence. One
example of such a cycle:
AXObjectCache::currentModalNode() ->
AccessibilityRenderObject::computeAccessibilityIsIgnored() ->
AccessibilityRenderObject::parentObjectUnignored() ->
AccessibilityObject::accessibilityIsIgnored() ->
AccessibilityObject::ignoredFromModalPresence() ->
AXObjectCache::currentModalNode() ->
...repeat...
This patch fixes this by tracking when we start computing the current
modal node in the AXObjectCache. Then, in AccessibilityObject::accessibilityIsIgnored(),
we don't call AccessibilityObject::ignoredFromModalPresence() if this new state is true,
since in this context we only need to know if the object is inherently
ignored (i.e. ignored disregarding modal presence).
Test: accessibility/aria-modal-with-text-crash.html
* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::currentModalNode):
* accessibility/AXObjectCache.h:
Add m_isRetrievingCurrentModalNode.
(WebCore::AXObjectCache::isRetrievingCurrentModalNode): Added.
* accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::accessibilityIsIgnored const):
Don't call ignoredFromModalPresence if we're in the midst of computing the current modal.
LayoutTests:
* accessibility/aria-modal-with-text-crash-expected.txt: Added.
* accessibility/aria-modal-with-text-crash.html: Added.
* platform/glib/TestExpectations: Skip new test.
* platform/ios/TestExpectations: Enable new test.
* platform/win/TestExpectations: Skip new test.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (294185 => 294186)
--- trunk/LayoutTests/ChangeLog 2022-05-14 03:20:28 UTC (rev 294185)
+++ trunk/LayoutTests/ChangeLog 2022-05-14 04:37:21 UTC (rev 294186)
@@ -1,3 +1,16 @@
+2022-05-13 Tyler Wilcock <tyle...@apple.com>
+
+ Infinite recursion caused by call to accessibilityIsIgnored in the midst of AccessibilityObject::ignoredFromModalPresence
+ https://bugs.webkit.org/show_bug.cgi?id=240365
+
+ Reviewed by Chris Fleizach.
+
+ * accessibility/aria-modal-with-text-crash-expected.txt: Added.
+ * accessibility/aria-modal-with-text-crash.html: Added.
+ * platform/glib/TestExpectations: Skip new test.
+ * platform/ios/TestExpectations: Enable new test.
+ * platform/win/TestExpectations: Skip new test.
+
2022-05-13 Tim Nguyen <n...@apple.com>
[css-ui] Remove caret/progress-bar-value/listitem values from appearance property
Added: trunk/LayoutTests/accessibility/aria-modal-with-text-crash-expected.txt (0 => 294186)
--- trunk/LayoutTests/accessibility/aria-modal-with-text-crash-expected.txt (rev 0)
+++ trunk/LayoutTests/accessibility/aria-modal-with-text-crash-expected.txt 2022-05-14 04:37:21 UTC (rev 294186)
@@ -0,0 +1,10 @@
+This test ensures we don't crash when using search to traverse an aria-modal with text.
+
+
+AXRole: AXStaticText
+AXValue: Foo
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Foo
Added: trunk/LayoutTests/accessibility/aria-modal-with-text-crash.html (0 => 294186)
--- trunk/LayoutTests/accessibility/aria-modal-with-text-crash.html (rev 0)
+++ trunk/LayoutTests/accessibility/aria-modal-with-text-crash.html 2022-05-14 04:37:21 UTC (rev 294186)
@@ -0,0 +1,37 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src=""
+<script src=""
+</head>
+<body>
+
+<div id="modal" role="dialog" aria-modal="true">
+ <span>Foo</span>
+</div>
+
+<script>
+ var testOutput = "This test ensures we don't crash when using search to traverse an aria-modal with text.\n\n";
+
+ if (window.accessibilityController) {
+ const modal = accessibilityController.accessibleElementById("modal");
+ let searchResult = null;
+ while (true) {
+ searchResult = modal.uiElementForSearchPredicate(searchResult, true, "AXAnyTypeSearchKey", "", false);
+ if (!searchResult)
+ break;
+ const role = searchResult.role;
+ testOutput += `\n${role}`;
+ if (role.includes("StaticText")) {
+ let textContent = accessibilityController.platformName === "ios" ? searchResult.description : searchResult.stringValue;
+ testOutput += `\n${textContent}`;
+ }
+ testOutput += "\n";
+ }
+ debug(testOutput);
+ }
+</script>
+</body>
+</html>
+
+
Modified: trunk/LayoutTests/platform/glib/TestExpectations (294185 => 294186)
--- trunk/LayoutTests/platform/glib/TestExpectations 2022-05-14 03:20:28 UTC (rev 294185)
+++ trunk/LayoutTests/platform/glib/TestExpectations 2022-05-14 04:37:21 UTC (rev 294186)
@@ -351,6 +351,7 @@
accessibility/text-updates-after-dynamic-change.html [ Skip ]
# Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
+accessibility/aria-modal-with-text-crash.html [ Skip ]
accessibility/display-contents-search-traversal.html [ Skip ]
accessibility/search-traversal-after-role-change.html [ Skip ]
accessibility/table-search-traversal.html [ Skip ]
Modified: trunk/LayoutTests/platform/ios/TestExpectations (294185 => 294186)
--- trunk/LayoutTests/platform/ios/TestExpectations 2022-05-14 03:20:28 UTC (rev 294185)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2022-05-14 04:37:21 UTC (rev 294186)
@@ -2135,6 +2135,7 @@
# Enable "aria-table-attributes" test for iOS
webkit.org/b/150366 accessibility/aria-table-attributes.html [ Pass ]
+accessibility/aria-modal-with-text-crash.html [ Pass ]
accessibility/display-contents-search-traversal.html [ Pass ]
accessibility/search-traversal-after-role-change.html [ Pass ]
accessibility/table-search-traversal.html [ Pass ]
Modified: trunk/LayoutTests/platform/win/TestExpectations (294185 => 294186)
--- trunk/LayoutTests/platform/win/TestExpectations 2022-05-14 03:20:28 UTC (rev 294185)
+++ trunk/LayoutTests/platform/win/TestExpectations 2022-05-14 04:37:21 UTC (rev 294186)
@@ -481,6 +481,7 @@
accessibility/ancestor-computation.html [ Skip ]
# Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
+accessibility/aria-modal-with-text-crash.html [ Skip ]
accessibility/display-contents-search-traversal.html [ Skip ]
accessibility/search-traversal-after-role-change.html [ Skip ]
accessibility/table-search-traversal.html [ Skip ]
Modified: trunk/Source/WebCore/ChangeLog (294185 => 294186)
--- trunk/Source/WebCore/ChangeLog 2022-05-14 03:20:28 UTC (rev 294185)
+++ trunk/Source/WebCore/ChangeLog 2022-05-14 04:37:21 UTC (rev 294186)
@@ -1,3 +1,39 @@
+2022-05-13 Tyler Wilcock <tyle...@apple.com>
+
+ Infinite recursion caused by call to accessibilityIsIgnored in the midst of AccessibilityObject::ignoredFromModalPresence
+ https://bugs.webkit.org/show_bug.cgi?id=240365
+
+ Reviewed by Chris Fleizach.
+
+ We can get infinite recursion when accessibilityIsIgnored is called as
+ part of computing AccessibilityObject::ignoredFromModalPresence. One
+ example of such a cycle:
+
+ AXObjectCache::currentModalNode() ->
+ AccessibilityRenderObject::computeAccessibilityIsIgnored() ->
+ AccessibilityRenderObject::parentObjectUnignored() ->
+ AccessibilityObject::accessibilityIsIgnored() ->
+ AccessibilityObject::ignoredFromModalPresence() ->
+ AXObjectCache::currentModalNode() ->
+ ...repeat...
+
+ This patch fixes this by tracking when we start computing the current
+ modal node in the AXObjectCache. Then, in AccessibilityObject::accessibilityIsIgnored(),
+ we don't call AccessibilityObject::ignoredFromModalPresence() if this new state is true,
+ since in this context we only need to know if the object is inherently
+ ignored (i.e. ignored disregarding modal presence).
+
+ Test: accessibility/aria-modal-with-text-crash.html
+
+ * accessibility/AXObjectCache.cpp:
+ (WebCore::AXObjectCache::currentModalNode):
+ * accessibility/AXObjectCache.h:
+ Add m_isRetrievingCurrentModalNode.
+ (WebCore::AXObjectCache::isRetrievingCurrentModalNode): Added.
+ * accessibility/AccessibilityObject.cpp:
+ (WebCore::AccessibilityObject::accessibilityIsIgnored const):
+ Don't call ignoredFromModalPresence if we're in the midst of computing the current modal.
+
2022-05-13 Brent Fulgham <bfulg...@apple.com>
REGRESSION (r281791): [iOS] WKWebView cannot load local .log file
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.cpp (294185 => 294186)
--- trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2022-05-14 03:20:28 UTC (rev 294185)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.cpp 2022-05-14 04:37:21 UTC (rev 294186)
@@ -306,6 +306,7 @@
return activeModalDialog;
}
+ SetForScope retrievingCurrentModalNode(m_isRetrievingCurrentModalNode, true);
// If any of the modal nodes contains the keyboard focus, we want to pick that one.
// If not, we want to pick the last visible dialog in the DOM.
RefPtr<Element> focusedElement = document().focusedElement();
Modified: trunk/Source/WebCore/accessibility/AXObjectCache.h (294185 => 294186)
--- trunk/Source/WebCore/accessibility/AXObjectCache.h 2022-05-14 03:20:28 UTC (rev 294185)
+++ trunk/Source/WebCore/accessibility/AXObjectCache.h 2022-05-14 04:37:21 UTC (rev 294186)
@@ -199,7 +199,8 @@
void deferNodeAddedOrRemoved(Node*);
void handleScrolledToAnchor(const Node* anchorNode);
void handleScrollbarUpdate(ScrollView*);
-
+
+ bool isRetrievingCurrentModalNode() { return m_isRetrievingCurrentModalNode; }
Node* modalNode();
void deferAttributeChangeIfNeeded(const QualifiedName&, Element*);
@@ -518,6 +519,7 @@
// If that changes to require only one aria-modal we could change this to a WeakHashSet, or discard the set completely.
ListHashSet<Element*> m_modalElementsSet;
bool m_modalNodesInitialized { false };
+ bool m_isRetrievingCurrentModalNode { false };
Timer m_performCacheUpdateTimer;
Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (294185 => 294186)
--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp 2022-05-14 03:20:28 UTC (rev 294185)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp 2022-05-14 04:37:21 UTC (rev 294186)
@@ -3746,7 +3746,10 @@
}
}
- bool ignored = ignoredFromModalPresence();
+ // If we are in the midst of retrieving the current modal node, we only need to consider whether the object
+ // is inherently ignored via computeAccessibilityIsIgnored. Also, calling ignoredFromModalPresence
+ // in this state would cause infinite recursion.
+ bool ignored = cache && cache->isRetrievingCurrentModalNode() ? false : ignoredFromModalPresence();
if (!ignored)
ignored = computeAccessibilityIsIgnored();
