AX: Implement AXStartTextMarkerAttribute and AXEndTextMarkerAttribute…

… off the main-thread https://bugs.webkit.org/show_bug.cgi?id=267944 rdar://problem/121463431 Reviewed by Andres Gonzalez. This patch also reimplements findObjectWithRuns in terms of Accessibility::findMatchingObjects, as the old implementation was implemented in a way that could hit a dead-end despite there being more objects with text runs left to traverse over. This bug is exercised by newly added testcase ax-thread-text-apis/display-contents-end-text-marker.html. * LayoutTests/accessibility/ax-thread-text-apis/display-contents-end-text-marker-expected.txt: Added. * LayoutTests/accessibility/ax-thread-text-apis/display-contents-end-text-marker.html: Added. * Source/WebCore/accessibility/AXCoreObject.h: Add AccessibilitySearchKey::HasTextRuns. * Source/WebCore/accessibility/AXLogger.cpp: (WebCore::operator<<): * Source/WebCore/accessibility/AXTextMarker.cpp: (WebCore::findObjectWithRuns): (WebCore::AXTextMarker::findLast const): Added. (WebCore::AXTextMarker::findMarker const): Remove redundant RELEASE_ASSERTs. * Source/WebCore/accessibility/AXTextMarker.h: * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::Accessibility::isAccessibilityObjectSearchMatchAtIndex): * Source/WebCore/accessibility/AccessibilityObject.h: * Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp: (WebCore::AXIsolatedObject::sibling const): Add missing nullptr check that was causing crashes due to its absence. * Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h: * Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm: (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]): Canonical link: https://commits.webkit.org/273962@main
WebKit · Feb 2, 2024 · dffba3d · dffba3d
1 parent 5825791
commit dffba3d
Show file tree

Hide file tree

Showing 13 changed files with 184 additions and 39 deletions.
diff --git a/LayoutTests/accessibility/ax-thread-text-apis/display-contents-end-text-marker-expected.txt b/LayoutTests/accessibility/ax-thread-text-apis/display-contents-end-text-marker-expected.txt
@@ -0,0 +1,15 @@
+This test ensures that computing text markers includes all display:contents text content.
+
+Web area string for start-to-end text marker range:
+
+Foo text
+This is a table caption
+Author Title Year
+Stephen Hawking A Brief History of Time 1988
+Carl Sagan Cosmos 1980
+Will Gater The Mysteries of the Universe 2020
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/accessibility/ax-thread-text-apis/display-contents-end-text-marker.html b/LayoutTests/accessibility/ax-thread-text-apis/display-contents-end-text-marker.html
@@ -0,0 +1,65 @@
+<!DOCTYPE HTML><!-- webkit-test-runner [ runSingly=true AccessibilityThreadTextApisEnabled=true ] -->
+<!-- Remove this in favor of display-contents/end-text-marker.html after AX_THREAD_TEXT_APIS enabled is the default. -->
+<html>
+<head>
+<script src="../../resources/accessibility-helper.js"></script>
+<script src="../../resources/js-test.js"></script>
+<style>
+td, th { display: contents; }
+</style>
+</head>
+<body id="body" role="group">
+
+<div id="test-contents">
+    Foo text
+
+    <table>
+    <caption>This is a table caption</caption>
+    <thead>
+        <tr>
+            <th>Author</th>
+            <th>Title</th>
+            <th>Year</th>
+        </tr>
+    </thead>
+    <tbody>
+        <tr>
+            <td>Stephen Hawking</td>
+            <td>A Brief History of Time</td>
+            <td>1988</td>
+        </tr>
+        <tr>
+            <td>Carl Sagan</td>
+            <td>Cosmos</td>
+            <td>1980</td>
+        </tr>
+        <tr>
+            <td>Will Gater</td>
+            <td>The Mysteries of the Universe</td>
+            <td>2020</td>
+        </tr>
+    </tbody>
+    </table>
+</div>
+
+<script>
+var output = "This test ensures that computing text markers includes all display:contents text content.\n\n";
+
+if (window.accessibilityController) {
+    const webArea = accessibilityController.rootElement.childAtIndex(0);
+
+    const startMarker = webArea.startTextMarker;
+    const endMarker = webArea.endTextMarker;
+    const textMarkerRange = webArea.textMarkerRangeForMarkers(startMarker, endMarker);
+
+    // Fails because:
+    //  1. We don't include spaces between cells
+    //  2. We don't include newline characters between rows
+    //  3. We miss the table caption text entirely (it is rendered and selectable text, so we should be including it)
+    output += `Web area string for start-to-end text marker range:\n\n${webArea.stringForTextMarkerRange(textMarkerRange)}\n`;
+    document.getElementById("test-contents").style.display = "none";
+    debug(output);
+}
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/accessibility/AXCoreObject.h b/Source/WebCore/accessibility/AXCoreObject.h
@@ -544,6 +544,7 @@ enum class AccessibilityDetachmentType { CacheDestroyed, ElementDestroyed, Eleme
 
 enum class AccessibilityConversionSpace { Screen, Page };
 
+// FIXME: This should be replaced by AXDirection (or vice versa).
 enum class AccessibilitySearchDirection {
     Next = 1,
     Previous,
@@ -577,6 +578,9 @@ enum class AccessibilitySearchKey {
     FontColorChange,
     Frame,
     Graphic,
+#if ENABLE(AX_THREAD_TEXT_APIS)
+    HasTextRuns,
+#endif
     HeadingLevel1,
     HeadingLevel2,
     HeadingLevel3,
@@ -1108,6 +1112,9 @@ class AXCoreObject : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AXCo
     virtual String description() const = 0;
 
     virtual std::optional<String> textContent() const = 0;
+#if ENABLE(AX_THREAD_TEXT_APIS)
+    virtual bool hasTextRuns() = 0;
+#endif
 
     // Methods for determining accessibility text.
     virtual String stringValue() const = 0;

diff --git a/Source/WebCore/accessibility/AXLogger.cpp b/Source/WebCore/accessibility/AXLogger.cpp
@@ -294,6 +294,11 @@ TextStream& operator<<(TextStream& stream, AccessibilitySearchKey searchKey)
     case AccessibilitySearchKey::Graphic:
         stream << "Graphic";
         break;
+#if ENABLE(AX_THREAD_TEXT_APIS)
+    case AccessibilitySearchKey::HasTextRuns:
+        stream << "HasTextRuns";
+        break;
+#endif
     case AccessibilitySearchKey::HeadingLevel1:
         stream << "HeadingLevel1";
         break;

diff --git a/Source/WebCore/accessibility/AXTextMarker.cpp b/Source/WebCore/accessibility/AXTextMarker.cpp
@@ -390,43 +390,22 @@ static RefPtr<AXIsolatedObject> findObjectWithRuns(AXIsolatedObject& start, AXDi
             return &start;
     }
 
-    RefPtr current = &start;
-    while (current) {
-        if (current != &start) {
-            auto* runs = current->textRuns();
-            if (runs && runs->size())
-                return current;
-        }
-
-        // FIXME: aria-owns breaks this traversal, as aria-owns causes the AX tree to be changed, affecting
-        // our iteration below, but it doesn't actually change text position on the page. So we need to ignore aria-owns
-        // tree changes here in order to behave correctly.
-        // We also probably need to do something about text within aria-hidden containers, which affects the AX tree.
-
-        const auto& children = current->children();
-        if (children.size()) {
-            size_t childIndex = direction == AXDirection::Next ? 0 : children.size() - 1;
-            RELEASE_ASSERT(children[childIndex]);
-            current = dynamicDowncast<AXIsolatedObject>(children[childIndex].get());
-            continue;
-        }
-
-        // `current` has no children, meaning it's a leaf node (e.g. it's text, which cannot have children).
-        // Check `current`s siblings.
-        if (auto* sibling = current->sibling(direction)) {
-            current = dynamicDowncast<AXIsolatedObject>(sibling);
-            continue;
-        }
-
-        // We have no children, and no next/previous sibling. Try our parent's sibling.
-        if (auto* parent = current->parentObjectUnignored()) {
-            current = dynamicDowncast<AXIsolatedObject>(parent->sibling(direction));
-            continue;
-        }
+    // FIXME: aria-owns breaks this function, as aria-owns causes the AX tree to be changed, affecting
+    // our search below, but it doesn't actually change text position on the page. So we need to ignore
+    // aria-owns tree changes here in order to behave correctly. We also probably need to do something
+    // about text within aria-hidden containers, which affects the AX tree.
+
+    AccessibilitySearchCriteria criteria { &start, direction == AXDirection::Next ? AccessibilitySearchDirection::Next : AccessibilitySearchDirection::Previous, emptyString(), 1, false, false };
+    RefPtr tree = std::get<RefPtr<AXIsolatedTree>>(axTreeForID(start.treeID()));
+    RefPtr root = tree ? tree->rootNode() : nullptr;
+    if (!root)
+        return nullptr;
+    criteria.anchorObject = root.get();
+    criteria.searchKeys = { AccessibilitySearchKey::HasTextRuns };
 
-        break;
-    }
-    return nullptr;
+    AXCoreObject::AccessibilityChildrenVector results;
+    Accessibility::findMatchingObjects(criteria, results);
+    return results.isEmpty() ? nullptr : dynamicDowncast<AXIsolatedObject>(results[0]);
 }
 
 unsigned AXTextMarker::offsetFromRoot() const
@@ -472,6 +451,31 @@ AXTextMarker AXTextMarker::nextMarkerFromOffset(unsigned offset) const
     return marker;
 }
 
+AXTextMarker AXTextMarker::findLast() const
+{
+    RELEASE_ASSERT(!isMainThread());
+
+    if (!isValid())
+        return { };
+    if (!isInTextLeaf()) {
+        auto textLeafMarker = toTextLeafMarker();
+        // We couldn't turn this non-text-leaf marker into a marker pointing to actual text, e.g. because
+        // this marker points at an empty container / group at the end of the document. In this case, we
+        // call ourselves the last marker.
+        if (!textLeafMarker.isValid())
+            return *this;
+        return textLeafMarker.findLast();
+    }
+
+    AXTextMarker marker;
+    auto newMarker = *this;
+    while (newMarker.isValid()) {
+        marker = WTFMove(newMarker);
+        newMarker = marker.findMarker(AXDirection::Next);
+    }
+    return marker;
+}
+
 String AXTextMarkerRange::toString() const
 {
     RELEASE_ASSERT(!isMainThread());
@@ -517,7 +521,6 @@ AXTextMarker AXTextMarker::findMarker(AXDirection direction) const
         return { };
     if (!isInTextLeaf())
         return toTextLeafMarker().findMarker(direction);
-    RELEASE_ASSERT(isInTextLeaf());
 
     size_t runIndex = runs()->indexForOffset(offset());
     RELEASE_ASSERT(runIndex != notFound);
@@ -549,7 +552,6 @@ AXTextMarker AXTextMarker::findMarker(AXDirection direction, AXTextUnit textUnit
         return { };
     if (!isInTextLeaf())
         return toTextLeafMarker().findMarker(direction, textUnit, boundary);
-    RELEASE_ASSERT(isInTextLeaf());
 
     if (textUnit == AXTextUnit::Line) {
         size_t runIndex = runs()->indexForOffset(offset());

diff --git a/Source/WebCore/accessibility/AXTextMarker.h b/Source/WebCore/accessibility/AXTextMarker.h
@@ -173,6 +173,8 @@ class AXTextMarker {
     AXTextMarker nextMarkerFromOffset(unsigned) const;
     // Returns the number of intermediate text markers between this and the root.
     unsigned offsetFromRoot() const;
+    // Starting from this marker, navigate to the last marker in the entire page.
+    AXTextMarker findLast() const;
 #endif // ENABLE(AX_THREAD_TEXT_APIS)
 
 private:

diff --git a/Source/WebCore/accessibility/AccessibilityObject.cpp b/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -4466,6 +4466,10 @@ static bool isAccessibilityObjectSearchMatchAtIndex(RefPtr<AXCoreObject> axObjec
         return axObject->isWebArea();
     case AccessibilitySearchKey::Graphic:
         return axObject->isImage();
+#if ENABLE(AX_THREAD_TEXT_APIS)
+    case AccessibilitySearchKey::HasTextRuns:
+        return axObject->hasTextRuns();
+#endif
     case AccessibilitySearchKey::HeadingLevel1:
         return axObject->headingLevel() == 1;
     case AccessibilitySearchKey::HeadingLevel2:

diff --git a/Source/WebCore/accessibility/AccessibilityObject.h b/Source/WebCore/accessibility/AccessibilityObject.h
@@ -387,6 +387,7 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
     unsigned textLength() const final;
 #if ENABLE(AX_THREAD_TEXT_APIS)
     virtual AXTextRuns textRuns() { return { }; }
+    bool hasTextRuns() final { return textRuns().size(); }
 #endif
 #if PLATFORM(COCOA)
     // Returns an array of strings and AXObject wrappers corresponding to the

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
@@ -567,6 +567,8 @@ void AXIsolatedObject::setSelectedChildren(const AccessibilityChildrenVector& se
 AXCoreObject* AXIsolatedObject::sibling(AXDirection direction) const
 {
     RefPtr parent = parentObjectUnignored();
+    if (!parent)
+        return nullptr;
     const auto& siblings = parent->children();
     size_t indexOfThis = siblings.find(this);
     if (indexOfThis == notFound)

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h
@@ -72,7 +72,12 @@ class AXIsolatedObject final : public AXCoreObject {
 
 #if ENABLE(AX_THREAD_TEXT_APIS)
     const AXTextRuns* textRuns() const;
-#endif
+    bool hasTextRuns() final
+    {
+        const auto* runs = textRuns();
+        return runs && runs->size();
+    }
+#endif // ENABLE(AX_THREAD_TEXT_APIS)
 
 private:
     void detachRemoteParts(AccessibilityDetachmentType) final;

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
@@ -1317,6 +1317,26 @@ void AXIsolatedTree::processQueuedNodeUpdates()
     queueRemovalsAndUnresolvedChanges({ });
 }
 
+#if ENABLE(AX_THREAD_TEXT_APIS)
+AXTextMarker AXIsolatedTree::firstMarker()
+{
+    RefPtr root = rootNode();
+    return root ? AXTextMarker { root->treeID(), root->objectID(), 0 } : AXTextMarker();
+}
+
+AXTextMarker AXIsolatedTree::lastMarker()
+{
+    RefPtr root = rootNode();
+    if (!root)
+        return { };
+
+    const auto& children = root->children();
+    // Start the `findLast` traversal from the last child of the root to reduce the amount of traversal done.
+    RefPtr endObject = children.isEmpty() ? root : dynamicDowncast<AXIsolatedObject>(children[children.size() - 1].get());
+    return AXTextMarker { endObject->treeID(), endObject->objectID(), 0 }.findLast();
+}
+#endif // ENABLE(AX_THREAD_TEXT_APIS)
+
 } // namespace WebCore
 
 #endif // ENABLE(ACCESSIBILITY_ISOLATED_TREE)
diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
@@ -387,6 +387,11 @@ class AXIsolatedTree : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AX
     void queueNodeUpdate(AXCoreObject&, const NodeUpdateOptions&);
     void processQueuedNodeUpdates();
 
+#if ENABLE(AX_THREAD_TEXT_APIS)
+    AXTextMarker firstMarker();
+    AXTextMarker lastMarker();
+#endif
+
 private:
     AXIsolatedTree(AXObjectCache&);
     static void storeTree(AXObjectCache&, const Ref<AXIsolatedTree>&);

diff --git a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
@@ -1936,6 +1936,12 @@ - (id)accessibilityAttributeValue:(NSString*)attributeName
         return (id)[self selectedTextMarkerRange];
 
     if ([attributeName isEqualToString:AXStartTextMarkerAttribute]) {
+#if ENABLE(AX_THREAD_TEXT_APIS)
+        if (AXObjectCache::useAXThreadTextApis()) {
+            if (RefPtr tree = std::get<RefPtr<AXIsolatedTree>>(axTreeForID(backingObject->treeID())))
+                return tree->firstMarker().platformData().bridgingAutorelease();
+        }
+#endif // ENABLE(AX_THREAD_TEXT_APIS)
         return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = retainPtr(self)] () -> RetainPtr<id> {
             auto* backingObject = protectedSelf.get().axBackingObject;
             if (!backingObject)
@@ -1946,6 +1952,12 @@ - (id)accessibilityAttributeValue:(NSString*)attributeName
     }
 
     if ([attributeName isEqualToString:AXEndTextMarkerAttribute]) {
+#if ENABLE(AX_THREAD_TEXT_APIS)
+        if (AXObjectCache::useAXThreadTextApis()) {
+            if (RefPtr tree = std::get<RefPtr<AXIsolatedTree>>(axTreeForID(backingObject->treeID())))
+                return tree->lastMarker().platformData().bridgingAutorelease();
+        }
+#endif // ENABLE(AX_THREAD_TEXT_APIS)
         return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([protectedSelf = retainPtr(self)] () -> RetainPtr<id> {
             auto* backingObject = protectedSelf.get().axBackingObject;
             if (!backingObject)