Attachment 459274 Details for Bug 240365 – Patch

[patch] Patch

bug-240365-20220512212055.patch (text/plain), 9.93 KB, created by Tyler Wilcock on 2022-05-12 21:20:56 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tyler Wilcock

Created: 2022-05-12 21:20:56 PDT

Size: 9.93 KB

patch

obsolete

>Subversion Revision: 294114
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ea889c48a3671f4928c5ba98b022e800c78c8dac..5f3aa2b316091e27201057014754901fe4a39084 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,39 @@
>+2022-05-12  Tyler Wilcock  <tyler_w@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 NOBODY (OOPS!).
>+
>+        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-11  Kate Cheney  <katherine_cheney@apple.com>
> 
>         Mail compose: right clicking an image attachment selects it
>diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp
>index 29a123ce91d40021f701664432e5b6d6374b26e7..92d6ab8ab8c54ff64cb45cbf508b00b6a9d0374f 100644
>--- a/Source/WebCore/accessibility/AXObjectCache.cpp
>+++ b/Source/WebCore/accessibility/AXObjectCache.cpp
>@@ -306,6 +306,7 @@ Element* AXObjectCache::currentModalNode()
>         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();
>diff --git a/Source/WebCore/accessibility/AXObjectCache.h b/Source/WebCore/accessibility/AXObjectCache.h
>index 22eefbd1325e7a19e386cfcba6bec6a9bba05acf..ae62dfc6140fca97a3d1c91afa70f0c12f53dc12 100644
>--- a/Source/WebCore/accessibility/AXObjectCache.h
>+++ b/Source/WebCore/accessibility/AXObjectCache.h
>@@ -199,7 +199,8 @@ public:
>     void deferNodeAddedOrRemoved(Node*);
>     void handleScrolledToAnchor(const Node* anchorNode);
>     void handleScrollbarUpdate(ScrollView*);
>-    
>+
>+    bool isRetrievingCurrentModalNode() { return m_isRetrievingCurrentModalNode; }
>     Node* modalNode();
> 
>     void deferAttributeChangeIfNeeded(const QualifiedName&, Element*);
>@@ -516,6 +517,7 @@ private:
>     // 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;
> 
>diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
>index d74d5f83f286bc526b19f3fd5cc15eea22526769..a8108919d8da21dfc741f44a999985695662b771 100644
>--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
>+++ b/Source/WebCore/accessibility/AccessibilityObject.cpp
>@@ -3746,7 +3746,10 @@ bool AccessibilityObject::accessibilityIsIgnored() const
>         }
>     }
> 
>-    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();
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 5a3d765befed93e6348ffd3d21bf030f60aa00c1..5ad3bcf27d857cc5687e1433d0bdab88c8d7ca6b 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,16 @@
>+2022-05-12  Tyler Wilcock  <tyler_w@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 NOBODY (OOPS!).
>+
>+        * 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-12  Karl Rackler  <rackler@apple.com>
> 
>         [ iOS ] imported/w3c/web-platform-tests/webrtc/RTCPeerConnection-videoDetectorTest.html is a consistent failure
>diff --git a/LayoutTests/accessibility/aria-modal-with-text-crash-expected.txt b/LayoutTests/accessibility/aria-modal-with-text-crash-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..e02b916e5189e3171f48786dc2bd21b3542a44ca
>--- /dev/null
>+++ b/LayoutTests/accessibility/aria-modal-with-text-crash-expected.txt
>@@ -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
>diff --git a/LayoutTests/accessibility/aria-modal-with-text-crash.html b/LayoutTests/accessibility/aria-modal-with-text-crash.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..d03ed198a37738539219b15ebf71cc8ac3fb7c74
>--- /dev/null
>+++ b/LayoutTests/accessibility/aria-modal-with-text-crash.html
>@@ -0,0 +1,37 @@
>+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
>+<html>
>+<head>
>+<script src="../resources/accessibility-helper.js"></script>
>+<script src="../resources/js-test.js"></script>
>+</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>
>+
>+
>diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
>index ec0ca0e0f4a8de7276d8fb598f24ecfa34557fbf..ccb520fad62aac4e5b51de59a5f160812b0f92e5 100644
>--- a/LayoutTests/platform/glib/TestExpectations
>+++ b/LayoutTests/platform/glib/TestExpectations
>@@ -351,6 +351,7 @@ accessibility/aria-busy-updates-after-dynamic-change.html [ Skip ]
> 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 ]
>diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
>index d8c914309a27fc0616c06769bf4bd3dc6fa36727..b36aa7713f001383fd7213db3aa47b0c5fbb1e1b 100644
>--- a/LayoutTests/platform/ios/TestExpectations
>+++ b/LayoutTests/platform/ios/TestExpectations
>@@ -2135,6 +2135,7 @@ accessibility/aria-sort-changed-notification.html [ Pass ]
> # 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 ]
>diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
>index 6b58a464e33bea26a975819f07fdf24679f8c00b..39a7b2aa82305652e33228946793de6f9fa09a57 100644
>--- a/LayoutTests/platform/win/TestExpectations
>+++ b/LayoutTests/platform/win/TestExpectations
>@@ -481,6 +481,7 @@ accessibility/visible-character-range-width-changes.html [ Skip ]
> 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 ]

Actions: View | Formatted Diff | Diff

Attachments on bug 240365: 459266 | 459274