AX VoiceOver: some content on web pages is not displayed on the brail…

…le display. https://bugs.webkit.org/show_bug.cgi?id=259066 rdar://110758833 Reviewed by Tyler Wilcock. AX clients like VoiceOver may request the AttributedString of a TextMarkerRange using the WebArea object. Since we don't cache the AttributedString for the WebArea, this causes a hit to the main thread. Furthermore, since the TextMarkerRange for a given object is created off the main thread and expects to be accessed off the main thread, it may not return the correct AttributedString when used on the main thread. This change ensures that the cached AttributedString for a given object is returned even when it is requested using the WebArea or any other object instead the object that owns the range. Canonical link: https://commits.webkit.org/265957@main
WebKit · Jul 11, 2023 · 27b8435 · 27b8435
1 parent f438e85
commit 27b8435
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 4 deletions.
diff --git a/...ests/accessibility/mac/attributed-string-for-text-marker-range-using-webarea-expected.txt b/...ests/accessibility/mac/attributed-string-for-text-marker-range-using-webarea-expected.txt
@@ -0,0 +1,28 @@
+Tests that using the WebArea to retrieve AttributedStrings from element TextMarkerRanges works properly.
+
+"AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, This is a test."
+"AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, This is a test.
+
+Second"
+"AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, This is a test.
+
+Second paragraph."
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+This is a test.
+
+Second paragraph.
diff --git a/LayoutTests/accessibility/mac/attributed-string-for-text-marker-range-using-webarea.html b/LayoutTests/accessibility/mac/attributed-string-for-text-marker-range-using-webarea.html
@@ -0,0 +1,38 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="../../resources/accessibility-helper.js"></script>
+</head>
+<body>
+
+<p id="p1">This is a test.</p>
+<p id="p2">Second paragraph.</p>
+
+<script>
+if (accessibilityController) {
+    let output = "Tests that using the WebArea to retrieve AttributedStrings from element TextMarkerRanges works properly.\n\n";
+
+    let webArea = accessibilityController.rootElement.childAtIndex(0);
+    let text = accessibilityController.accessibleElementById("p1").childAtIndex(0);
+    let range1 = webArea.textMarkerRangeForElement(text);
+    output += `"${webArea.attributedStringForTextMarkerRange(range1)}"\n`;
+
+    // Include the word Second in p2.
+    let end = webArea.endTextMarkerForTextMarkerRange(range1);
+    for (let i = 0; i < 7; ++i)
+        end = webArea.nextTextMarker(end);
+    let range = webArea.textMarkerRangeForMarkers(webArea.startTextMarkerForTextMarkerRange(range1), end);
+    output += `"${webArea.attributedStringForTextMarkerRange(range)}"\n`;
+
+    // Get the string for both paragraphs together.
+    text = accessibilityController.accessibleElementById("p2").childAtIndex(0);
+    let range2 = webArea.textMarkerRangeForElement(text);
+    range = webArea.textMarkerRangeForMarkers(webArea.startTextMarkerForTextMarkerRange(range1), webArea.endTextMarkerForTextMarkerRange(range2));
+    output += `"${webArea.attributedStringForTextMarkerRange(range)}"\n`;
+
+    debug(output);
+}
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/accessibility/AXTextMarker.cpp b/Source/WebCore/accessibility/AXTextMarker.cpp
@@ -298,4 +298,11 @@ std::partial_ordering partialOrder(const AXTextMarker& marker1, const AXTextMark
     return result;
 }
 
+bool AXTextMarkerRange::isConfinedTo(AXID objectID) const
+{
+    return m_start.objectID() == objectID
+        && m_end.objectID() == objectID
+        && LIKELY(m_start.treeID() == m_end.treeID());
+}
+
 } // namespace WebCore
diff --git a/Source/WebCore/accessibility/AXTextMarker.h b/Source/WebCore/accessibility/AXTextMarker.h
@@ -133,9 +133,9 @@ class AXTextMarker {
     String debugDescription() const;
 #endif
 
-private:
-    // Sets m_data.node when the marker is being created with a PlatformTextMarkerData that lacks the node pointer because it was created off the main thread.
+    // Sets m_data.node when the marker was created with a PlatformTextMarkerData that lacks the node pointer because it was created off the main thread.
     void setNodeIfNeeded() const;
+private:
     TextMarkerData m_data;
 };
 
@@ -167,7 +167,7 @@ class AXTextMarkerRange {
 
     AXTextMarker start() const { return m_start; }
     AXTextMarker end() const { return m_end; }
-
+    bool isConfinedTo(AXID) const;
 private:
     AXTextMarker m_start;
     AXTextMarker m_end;

diff --git a/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm b/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm
@@ -177,14 +177,25 @@
     if (!markerRange)
         return nil;
 
+    if (markerRange.start().objectID() != objectID() && markerRange.isConfinedTo(markerRange.start().objectID())) {
+        // markerRange is confined to an object different from this. That is the case when clients use the webarea to request AttributedStrings for a range obtained from an inner descendant.
+        // Delegate to the inner object in this case.
+        if (RefPtr object = markerRange.start().object())
+            return object->attributedStringForTextMarkerRange(WTFMove(markerRange), spellCheck);
+    }
+
     // At the moment we are only handling ranges that are contained in a single object, and for which we cached the AttributeString.
     // FIXME: Extend to cases where the range expands multiple objects.
 
     auto attributedText = propertyValue<RetainPtr<NSAttributedString>>(AXPropertyName::AttributedText);
     if (!attributedText) {
         return Accessibility::retrieveValueFromMainThread<RetainPtr<NSAttributedString>>([markerRange = WTFMove(markerRange), &spellCheck, this] () mutable -> RetainPtr<NSAttributedString> {
-            if (RefPtr axObject = associatedAXObject())
+            if (RefPtr axObject = associatedAXObject()) {
+                // Ensure that the TextMarkers have valid Node references, in case the range was created on the AX thread.
+                markerRange.start().setNodeIfNeeded();
+                markerRange.end().setNodeIfNeeded();
                 return axObject->attributedStringForTextMarkerRange(WTFMove(markerRange), spellCheck);
+            }
             return { };
         });
     }