VO interacts with the element containing the app selection instead of…

… with VO's current element.

https://bugs.webkit.org/show_bug.cgi?id=259629
rdar://112193520

Reviewed by Chris Fleizach.

VoiceOver uses the AXTextMarkerRangeForUnorderedTextMarkersAttribute to decide which element to interact with when the user presses VO Shift Down Arrow. This API failed when one of the TextMarker parameters was created off the main thread and thus has a null Node*. This patch fixes the issue by having a single implementation for both AXTextMarkerRangeForUnorderedTextMarkers and AXTextMarkerRangeForTextMarkers that supports unordered TextMarkers.

Added WTR::AccessibilityUIElement::textMarkerRangeForUnorderedMarkers and a test case for this API.

* LayoutTests/accessibility/text-marker/text-marker-range-with-unordered-markers-expected.txt:
* LayoutTests/accessibility/text-marker/text-marker-range-with-unordered-markers.html:
* LayoutTests/platform/glib/accessibility/text-marker/text-marker-range-with-unordered-markers-expected.txt: Added.
* LayoutTests/platform/ios/accessibility/text-marker/text-marker-range-with-unordered-markers-expected.txt: Added.
* LayoutTests/platform/mac-wk1/TestExpectations:
* Source/WebCore/accessibility/AXTextMarker.cpp:
(WebCore::partialOrder):
* Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:forParameter:]):
* Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp:
(WTR::AccessibilityUIElement::textMarkerRangeForUnorderedMarkers):
* Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h:
* Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl:
* Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:
(WTR::AccessibilityUIElement::textMarkerRangeForUnorderedMarkers):

Canonical link: https://commits.webkit.org/266417@main

LayoutTests/accessibility/text-marker/text-marker-range-with-unordered-markers-expected.txt

@@ -6,6 +6,8 @@ PASS: text.stringForTextMarkerRange(range1) === 'This is a paragraph.'
 PASS: text.textMarkerRangeLength(range1) === 20
 PASS: text.stringForTextMarkerRange(range2) === 'para'
 PASS: text.textMarkerRangeLength(range2) === 4
+PASS: text.stringForTextMarkerRange(range3) === 'para'
+PASS: text.textMarkerRangeLength(range3) === 4
 
 PASS successfullyParsed is true
 

LayoutTests/accessibility/text-marker/text-marker-range-with-unordered-markers.html

@@ -32,6 +32,12 @@
     output += expect("text.stringForTextMarkerRange(range2)", "'para'");
     output += expect("text.textMarkerRangeLength(range2)", "4");
 
+    if (accessibilityController.platformName == "mac") {
+        var range3 = text.textMarkerRangeForUnorderedMarkers(end, start);
+        output += expect("text.stringForTextMarkerRange(range3)", "'para'");
+        output += expect("text.textMarkerRangeLength(range3)", "4");
+    }
+
     document.getElementById("text").style.visibility = "hidden";
     debug(output);
 }

LayoutTests/platform/glib/accessibility/text-marker/text-marker-range-with-unordered-markers-expected.txt

@@ -0,0 +1,13 @@
+This test ensures that clietns can build an AXTextMarkerRange from unordered TextMarkers, i.e., start > end.
+
+PASS: text.stringForTextMarkerRange(range) === 'This is a paragraph.'
+PASS: text.textMarkerRangeLength(range) === 20
+PASS: text.stringForTextMarkerRange(range1) === 'This is a paragraph.'
+PASS: text.textMarkerRangeLength(range1) === 20
+PASS: text.stringForTextMarkerRange(range2) === 'para'
+PASS: text.textMarkerRangeLength(range2) === 4
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

LayoutTests/platform/ios/accessibility/text-marker/text-marker-range-with-unordered-markers-expected.txt

@@ -0,0 +1,13 @@
+This test ensures that clietns can build an AXTextMarkerRange from unordered TextMarkers, i.e., start > end.
+
+PASS: text.stringForTextMarkerRange(range) === 'This is a paragraph.'
+PASS: text.textMarkerRangeLength(range) === 20
+PASS: text.stringForTextMarkerRange(range1) === 'This is a paragraph.'
+PASS: text.textMarkerRangeLength(range1) === 20
+PASS: text.stringForTextMarkerRange(range2) === 'para'
+PASS: text.textMarkerRangeLength(range2) === 4
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

LayoutTests/platform/mac-wk1/TestExpectations

@@ -829,6 +829,7 @@ accessibility/display-contents/aria-owns.html [ Skip ]
 accessibility/mac/text-field-number-of-characters.html [ Skip ]
 accessibility/mac/content-editable-attributed-string.html [ Skip ]
 accessibility/mac/lazy-spellchecking.html [ Skip ]
+accessibility/text-marker/text-marker-range-with-unordered-markers.html
 
 # DOM paste access requests are not implemented in WebKit1.
 editing/async-clipboard/clipboard-change-data-while-reading.html [ Skip ]

Source/WebCore/accessibility/AXTextMarker.cpp

@@ -294,7 +294,7 @@ std::optional<CharacterRange> AXTextMarkerRange::characterRange() const
 
 std::partial_ordering partialOrder(const AXTextMarker& marker1, const AXTextMarker& marker2)
 {
-    if (marker1.objectID() == marker2.objectID() && marker1.treeID() == marker2.treeID()) {
+    if (marker1.objectID() == marker2.objectID() && LIKELY(marker1.treeID() == marker2.treeID())) {
         if (LIKELY(marker1.m_data.characterOffset < marker2.m_data.characterOffset))
             return std::partial_ordering::less;
         if (marker1.m_data.characterOffset > marker2.m_data.characterOffset)

Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm

@@ -3407,7 +3407,8 @@ - (id)accessibilityAttributeValue:(NSString*)attribute forParameter:(id)paramete
         return nil;
     }
 
-    if ([attribute isEqualToString:AXTextMarkerRangeForTextMarkersAttribute]) {
+    if ([attribute isEqualToString:AXTextMarkerRangeForTextMarkersAttribute]
+        || [attribute isEqualToString:AXTextMarkerRangeForUnorderedTextMarkersAttribute]) {
         if (array.count < 2
             || !AXObjectIsTextMarker([array objectAtIndex:0])
             || !AXObjectIsTextMarker([array objectAtIndex:1]))
@@ -3416,32 +3417,6 @@ - (id)accessibilityAttributeValue:(NSString*)attribute forParameter:(id)paramete
         return AXTextMarkerRange { { (AXTextMarkerRef)[array objectAtIndex:0] }, { (AXTextMarkerRef)[array objectAtIndex:1] } }.platformData().bridgingAutorelease();
     }
 
-    if ([attribute isEqualToString:AXTextMarkerRangeForUnorderedTextMarkersAttribute]) {
-        if (array.count < 2
-            || !AXObjectIsTextMarker([array objectAtIndex:0])
-            || !AXObjectIsTextMarker([array objectAtIndex:1]))
-            return nil;
-
-        AXTextMarkerRef textMarker1 = (AXTextMarkerRef)[array objectAtIndex:0];
-        AXTextMarkerRef textMarker2 = (AXTextMarkerRef)[array objectAtIndex:1];
-
-        return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker1 = retainPtr(textMarker1), textMarker2 = retainPtr(textMarker2), protectedSelf = retainPtr(self)] () -> RetainPtr<id> {
-            auto* backingObject = protectedSelf.get().axBackingObject;
-            if (!backingObject)
-                return nil;
-
-            auto* cache = backingObject->axObjectCache();
-            if (!cache)
-                return nil;
-
-            auto characterOffset1 = characterOffsetForTextMarker(cache, textMarker1.get());
-            auto characterOffset2 = characterOffsetForTextMarker(cache, textMarker2.get());
-            auto range = cache->rangeForUnorderedCharacterOffsets(characterOffset1, characterOffset2);
-
-            return (id)textMarkerRangeFromRange(cache, range);
-        });
-    }
-
     if ([attribute isEqualToString:AXNextTextMarkerForTextMarkerAttribute]) {
         return Accessibility::retrieveAutoreleasedValueFromMainThread<id>([textMarker = retainPtr(textMarker), protectedSelf = retainPtr(self)] () -> RetainPtr<id> {
             auto* backingObject = protectedSelf.get().axBackingObject;

Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp

@@ -110,6 +110,7 @@ RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::leftLineTextMarkerR
 RefPtr<AccessibilityTextMarker> AccessibilityUIElement::previousLineStartTextMarkerForTextMarker(AccessibilityTextMarker*) { return nullptr; }
 RefPtr<AccessibilityTextMarker> AccessibilityUIElement::nextLineEndTextMarkerForTextMarker(AccessibilityTextMarker*) { return nullptr; }
 int AccessibilityUIElement::lineIndexForTextMarker(AccessibilityTextMarker*) const { return -1; }
+RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::textMarkerRangeForUnorderedMarkers(AccessibilityTextMarker*, AccessibilityTextMarker*) { return nullptr; }
 RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::textMarkerRangeForRange(unsigned, unsigned) { return nullptr; }
 RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::selectedTextMarkerRange() { return nullptr; }
 void AccessibilityUIElement::resetSelectedTextMarkerRange() { }

Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h

@@ -338,7 +338,8 @@ class AccessibilityUIElement : public JSWrappable {
     int lineIndexForTextMarker(AccessibilityTextMarker*) const;
     RefPtr<AccessibilityTextMarkerRange> misspellingTextMarkerRange(AccessibilityTextMarkerRange* start, bool forward);
     RefPtr<AccessibilityTextMarkerRange> textMarkerRangeForElement(AccessibilityUIElement*);
-    RefPtr<AccessibilityTextMarkerRange> textMarkerRangeForMarkers(AccessibilityTextMarker* startMarker, AccessibilityTextMarker* endMarker);
+    RefPtr<AccessibilityTextMarkerRange> textMarkerRangeForMarkers(AccessibilityTextMarker*, AccessibilityTextMarker*);
+    RefPtr<AccessibilityTextMarkerRange> textMarkerRangeForUnorderedMarkers(AccessibilityTextMarker*, AccessibilityTextMarker*);
     RefPtr<AccessibilityTextMarkerRange> textMarkerRangeForRange(unsigned location, unsigned length);
     RefPtr<AccessibilityTextMarkerRange> selectedTextMarkerRange();
     void resetSelectedTextMarkerRange();

Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl

@@ -241,6 +241,7 @@
     AccessibilityTextMarkerRange misspellingTextMarkerRange(AccessibilityTextMarkerRange start, boolean forward);
     AccessibilityTextMarkerRange textMarkerRangeForElement(AccessibilityUIElement element);
     AccessibilityTextMarkerRange textMarkerRangeForMarkers(AccessibilityTextMarker startMarker, AccessibilityTextMarker endMarker);
+    AccessibilityTextMarkerRange textMarkerRangeForUnorderedMarkers(AccessibilityTextMarker startMarker, AccessibilityTextMarker endMarker);
     AccessibilityTextMarkerRange textMarkerRangeForRange(unsigned long location, unsigned long length);
     AccessibilityTextMarkerRange selectedTextMarkerRange();
     undefined resetSelectedTextMarkerRange();

Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm

@@ -2041,6 +2041,17 @@ void setAttributeValue(id element, NSString* attribute, id value, bool synchrono
     return nullptr;
 }
 
+RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::textMarkerRangeForUnorderedMarkers(AccessibilityTextMarker* startMarker, AccessibilityTextMarker* endMarker)
+{
+    BEGIN_AX_OBJC_EXCEPTIONS
+    NSArray *textMarkers = @[startMarker->platformTextMarker(), endMarker->platformTextMarker()];
+    auto textMarkerRange = attributeValueForParameter(@"AXTextMarkerRangeForUnorderedTextMarkers", textMarkers);
+    return AccessibilityTextMarkerRange::create(textMarkerRange.get());
+    END_AX_OBJC_EXCEPTIONS
+
+    return nullptr;
+}
+
 RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::textMarkerRangeForRange(unsigned location, unsigned length)
 {
     BEGIN_AX_OBJC_EXCEPTIONS