AX: Dynamic changes to subtrees of AccessibilityNodeObjects can cause…

… missing or stale content https://bugs.webkit.org/show_bug.cgi?id=245212 rdar://problem/99956378 Reviewed by Andres Gonzalez. AccessibilityNodeObject is missing a few key method implementations that can cause incorrect AX tree construction (either missing or stale content) after dynamic page updates. These methods are: - updateChildrenIfNecessary - setNeedsToUpdateChildren - needsToUpdateChildren - setNeedsToUpdateSubtree In this patch, we move these method implementations from AccessibilityRenderObject to AccessibilityNodeObject, ensuring both types of objects share this logic. * LayoutTests/accessibility/display-contents-dynamically-added-children-expected.txt: Added. * LayoutTests/accessibility/display-contents-dynamically-added-children.html: Added. * LayoutTests/platform/glib/TestExpectations: Skip new test. * LayoutTests/platform/ios/TestExpectations: * LayoutTests/platform/ios/accessibility/display-contents-dynamically-added-children-expected.txt: Added. * LayoutTests/platform/mac-wk1/TestExpectations: Skip new test. * LayoutTests/platform/win/TestExpectations: Skip new test. * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::updateChildrenIfNecessary): * Source/WebCore/accessibility/AccessibilityNodeObject.h: * Source/WebCore/accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::updateChildrenIfNecessary): Moved to AccessibilityNodeObject. * Source/WebCore/accessibility/AccessibilityRenderObject.h: Canonical link: https://commits.webkit.org/254534@main
WebKit · Sep 15, 2022 · c6e7fc8 · c6e7fc8
1 parent b0895ea
commit c6e7fc8
Show file tree

Hide file tree

Showing 11 changed files with 128 additions and 16 deletions.
diff --git a/LayoutTests/accessibility/display-contents-dynamically-added-children-expected.txt b/LayoutTests/accessibility/display-contents-dynamically-added-children-expected.txt
@@ -0,0 +1,27 @@
+This test ensures that we include dynamically added content in the AX tree when said content is added as a subtree of a display:contents element.
+
+Traversing page based on initial state.
+
+AXRole: AXWebArea
+
+AXRole: AXGroup
+
+Adding list elements to #div-to-insert-into and performing another page traversal.
+
+AXRole: AXWebArea
+
+AXRole: AXGroup
+
+AXRole: AXList
+
+AXRole: AXGroup
+
+AXRole: AXListMarker
+
+AXRole: AXStaticText
+AXValue: List item one
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+List item one
diff --git a/LayoutTests/accessibility/display-contents-dynamically-added-children.html b/LayoutTests/accessibility/display-contents-dynamically-added-children.html
@@ -0,0 +1,59 @@
+<!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="display-contents-div" aria-label="Div with display:contents" style="display:contents">
+    <div id="div-to-insert-into">
+    </div>
+</div>
+
+<script>
+    var testOutput = "This test ensures that we include dynamically added content in the AX tree when said content is added as a subtree of a display:contents element.\n\n";
+
+    var axRoot = accessibilityController.rootElement;
+    function traversePage() {
+        let searchResult = null;
+        while (true) {
+            searchResult = axRoot.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";
+        }
+    }
+
+    if (window.accessibilityController) {
+        window.jsTestIsAsync = true;
+
+        testOutput += "Traversing page based on initial state.\n";
+        traversePage();
+
+        testOutput += "\nAdding list elements to #div-to-insert-into and performing another page traversal.\n";
+
+        const ul = document.createElement("ul");
+        const li = document.createElement("li");
+        li.innerHTML = "List item one";
+        ul.appendChild(li);
+        document.getElementById("div-to-insert-into").appendChild(ul);
+        setTimeout(async function() {
+            await waitFor(() => axRoot.uiElementForSearchPredicate(axRoot, true, "AXListSearchKey", "", false));
+            traversePage();
+
+            debug(testOutput);
+            finishJSTest();
+        }, 0);
+    }
+</script>
+</body>
+</html>
+
+
diff --git a/LayoutTests/platform/glib/TestExpectations b/LayoutTests/platform/glib/TestExpectations
@@ -372,6 +372,7 @@ accessibility/dynamically-ignored-canvas.html [ Skip ]
 
 # Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
 accessibility/aria-modal-with-text-crash.html [ Skip ]
+accessibility/display-contents-dynamically-added-children.html [ Skip ]
 accessibility/display-contents-object-ordering.html [ Skip ]
 accessibility/display-contents-search-traversal.html [ Skip ]
 accessibility/search-traversal-after-role-change.html [ Skip ]

diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
@@ -2131,6 +2131,7 @@ accessibility/aria-required-updates-after-dynamic-change.html [ Pass ]
 accessibility/aria-roles-unignored.html [ Pass ]
 accessibility/aria-slider-value.html [ Pass ]
 accessibility/ax-object-destroyed-on-reload.html [ Pass ]
+accessibility/display-contents-dynamically-added-children.html [ Pass ]
 accessibility/display-contents-element-roles.html [ Pass ]
 accessibility/display-contents-object-ordering.html [ Pass ]
 accessibility/dynamically-ignored-canvas.html [ Pass ]

diff --git a/...Tests/platform/ios/accessibility/display-contents-dynamically-added-children-expected.txt b/...Tests/platform/ios/accessibility/display-contents-dynamically-added-children-expected.txt
@@ -0,0 +1,19 @@
+This test ensures that we include dynamically added content in the AX tree when said content is added as a subtree of a display:contents element.
+
+Traversing page based on initial state.
+
+WebArea
+
+Div
+
+Adding list elements to #div-to-insert-into and performing another page traversal.
+
+ListMarker
+
+StaticText
+AXLabel: List item one
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+List item one
diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -318,6 +318,8 @@ accessibility/custom-elements/role.html [ Skip ]
 # Missing necessary testing logic (AccessibilityController::{get,set}RetainedElement)
 accessibility/ax-object-destroyed-on-reload.html [ Skip ]
 
+accessibility/display-contents-dynamically-added-children.html [ Skip ]
+
 # DOM paste access requests are not implemented in WebKit1.
 editing/async-clipboard/clipboard-change-data-while-reading.html [ Skip ]
 editing/async-clipboard/clipboard-do-not-read-text-from-platform-if-text-changes.html [ Skip ]

diff --git a/LayoutTests/platform/win/TestExpectations b/LayoutTests/platform/win/TestExpectations
@@ -528,6 +528,7 @@ accessibility/ancestor-computation.html [ Skip ]
 
 # Missing AccessibilityUIElement::uiElementForSearchPredicate implementation.
 accessibility/aria-modal-with-text-crash.html [ Skip ]
+accessibility/display-contents-dynamically-added-children.html [ Skip ]
 accessibility/display-contents-object-ordering.html [ Skip ]
 accessibility/display-contents-search-traversal.html [ Skip ]
 accessibility/search-traversal-after-role-change.html [ Skip ]

diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.cpp b/Source/WebCore/accessibility/AccessibilityNodeObject.cpp
@@ -447,6 +447,14 @@ bool AccessibilityNodeObject::isDescendantOfElementType(const HashSet<QualifiedN
     return false;
 }
 
+void AccessibilityNodeObject::updateChildrenIfNecessary()
+{
+    if (needsToUpdateChildren())
+        clearChildren();
+
+    AccessibilityObject::updateChildrenIfNecessary();
+}
+
 void AccessibilityNodeObject::addChildren()
 {
     // If the need to add more children in addition to existing children arises, 

diff --git a/Source/WebCore/accessibility/AccessibilityNodeObject.h b/Source/WebCore/accessibility/AccessibilityNodeObject.h
@@ -159,13 +159,15 @@ class AccessibilityNodeObject : public AccessibilityObject {
         Yes
     };
     AccessibilityRole determineAccessibilityRoleFromNode(TreatStyleFormatGroupAsInline = TreatStyleFormatGroupAsInline::No) const;
-    void addChildren() override;
-
-    bool canHaveChildren() const override;
     AccessibilityRole ariaRoleAttribute() const override;
     virtual AccessibilityRole determineAriaRoleAttribute() const;
     AccessibilityRole remapAriaRoleDueToParent(AccessibilityRole) const;
+
+    void addChildren() override;
+    void updateChildrenIfNecessary() override;
+    bool canHaveChildren() const override;
     bool isDescendantOfBarrenParent() const override;
+
     enum class StepAction : bool { Decrement, Increment };
     void alterRangeValue(StepAction);
     void changeValueByStep(StepAction);
@@ -211,7 +213,11 @@ class AccessibilityNodeObject : public AccessibilityObject {
     void setNodeValue(StepAction, float);
     bool performDismissAction() final;
     bool hasTextAlternative() const;
-
+
+    void setNeedsToUpdateChildren() override { m_childrenDirty = true; }
+    bool needsToUpdateChildren() const override { return m_childrenDirty; }
+    void setNeedsToUpdateSubtree() override { m_subtreeDirty = true; }
+
     bool isDescendantOfElementType(const HashSet<QualifiedName>&) const;
 
     Node* m_node;

diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
@@ -2933,14 +2933,6 @@ void AccessibilityRenderObject::addImageMapChildren()
     }
 }
 
-void AccessibilityRenderObject::updateChildrenIfNecessary()
-{
-    if (needsToUpdateChildren())
-        clearChildren();
-
-    AccessibilityObject::updateChildrenIfNecessary();
-}
-
 void AccessibilityRenderObject::addTextFieldChildren()
 {
     Node* node = this->node();

diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.h b/Source/WebCore/accessibility/AccessibilityRenderObject.h
@@ -134,7 +134,6 @@ class AccessibilityRenderObject : public AccessibilityNodeObject {
     FrameView* documentFrameView() const override;
 
     void clearChildren() override;
-    void updateChildrenIfNecessary() override;
 
     void setFocused(bool) override;
     void setSelectedTextRange(const PlainTextRange&) override;
@@ -214,9 +213,6 @@ class AccessibilityRenderObject : public AccessibilityNodeObject {
     PlainTextRange documentBasedSelectedTextRange() const;
     Element* rootEditableElementForPosition(const Position&) const;
     bool nodeIsTextControl(const Node*) const;
-    void setNeedsToUpdateChildren() override { m_childrenDirty = true; }
-    bool needsToUpdateChildren() const override { return m_childrenDirty; }
-    void setNeedsToUpdateSubtree() override { m_subtreeDirty = true; }
     Path elementPath() const override;
 
     bool isTabItemSelected() const;