Bug 265683 – AX: add Mac API to get selected text range overlapping static text element

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Dominic Mazzoni]

One of the things preventing VoiceOver from being able to use WebKit entirely off the main thread is that it wants to be able to access the range of a static text element that's part of the document selection, if any.

Doing that using text ranges is complex and hard for WebKit to optimize. A new API that provides this information directly can be computed efficiently using the isolated tree.

[Radar WebKit Bug Importer]

<rdar://problem/119042064>

[Dominic Mazzoni]

Created attachment 468835 [details]
Patch

[Dominic Mazzoni]

I can rename to selectedTextRangeIntersectingElement if that sounds more clear to everyone.

[chris fleizach]

Comment on attachment 468835 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=468835&action=review

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1378
> +    Node* node = m_renderer->node();

auto*

> Source/WebCore/accessibility/AccessibilityRenderObject.cpp:1381
> +    VisiblePositionRange intersection = WebCore::intersection(selectedVisiblePositionRange, nodeVisibleRange);

i think we can use auto for all of these

> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1281
> +
> +    AXTextMarkerRange range = tree()->selectedTextMarkerRange();

auto

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:1562
> +        NSMutableString* rangeDescription = [NSMutableString stringWithFormat:@"{%lu, %lu}", static_cast<unsigned long>(range.location), static_cast<unsigned long>(range.length)];

NSMutableString *

also i don't think this needs to be mutable

[Andres Gonzalez]

(In reply to Dominic Mazzoni from comment #2)
> Created attachment 468835 [details]
> Patch

diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
index a47d458c0c23..0ec64c61c922 100644
--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
+++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
@@ -1274,6 +1274,29 @@ bool AXIsolatedObject::isNativeTextControl() const
     return false;
 }

+std::optional<CharacterRange> AXIsolatedObject::selectedStaticTextRange() const
+{
+    ASSERT(isStaticText());
+
+    AXTextMarkerRange range = tree()->selectedTextMarkerRange();
+    if (range.start().treeID() != tree()->treeID() || range.end().treeID() != tree()->treeID())
+        return std::nullopt;
+
+    if (range.start().objectID() == range.end().objectID()) {
+        if (range.start().objectID() != objectID())
+            return std::nullopt;
+
+        std::make_optional(CharacterRange(range.start().characterOffset(), range.end().characterOffset()));

AG: missing return ?

+    }
+
+    return Accessibility::retrieveValueFromMainThread<std::optional<CharacterRange>>([this] () -> std::optional<CharacterRange> {
+        if (auto* axObject = associatedAXObject())
+            return axObject->selectedStaticTextRange();
+
+        return std::nullopt;
+    });
+}
+
 int AXIsolatedObject::insertionPointLineNumber() const
 {
     return Accessibility::retrieveValueFromMainThread<int>([this] () -> int {

[Andres Gonzalez]

(In reply to Dominic Mazzoni from comment #3)
> I can rename to selectedTextRangeIntersectingElement if that sounds more
> clear to everyone.

Maybe something like AXIntersectionWithSelectionRange ?

[Joshua Hoffman]

Comment on attachment 468835 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=468835&action=review

> LayoutTests/accessibility/mac/selected-static-text-range.html:10
> +<!--
> +  Note: run this test with --accessibility-isolated-tree
> +-->

This comment is probably not necessary.

> LayoutTests/accessibility/mac/selected-static-text-range.html:59
> +        dumpAccessibilityTree(accessibilityController.rootElement, null, 0, true, false, true);

Is it necessary to dump the accessibility tree here?

[Dominic Mazzoni]

(In reply to Joshua Hoffman from comment #7)
> > LayoutTests/accessibility/mac/selected-static-text-range.html:59
> > +        dumpAccessibilityTree(accessibilityController.rootElement, null, 0, true, false, true);
> 
> Is it necessary to dump the accessibility tree here?

Oddly, yes. If I don't do this, the isolated objects don't exist.

Is this a known issue, is there a better or more clear workaround?

[Dominic Mazzoni]

Created attachment 468838 [details]
Patch

[Dominic Mazzoni]

Created attachment 468874 [details]
Patch

[Dominic Mazzoni]

Just rebasing. I'm going to try Andres's idea of using the same public API but implementing using text marker ranges.

[Dominic Mazzoni]

Created attachment 468884 [details]
Patch

[chris fleizach]

Comment on attachment 468884 [details]
Patch

a few test failures to cleanup, but otherwise lgtm

[Andres Gonzalez]

(In reply to Dominic Mazzoni from comment #12)
> Created attachment 468884 [details]
> Patch

Thanks for doing this, looks good overall, just a few more comments.

diff --git a/Source/WebCore/accessibility/AXTextMarker.cpp b/Source/WebCore/accessibility/AXTextMarker.cpp
index d75920ae9a14..69022fc47757 100644
--- a/Source/WebCore/accessibility/AXTextMarker.cpp
+++ b/Source/WebCore/accessibility/AXTextMarker.cpp
@@ -299,6 +299,38 @@ std::optional<CharacterRange> AXTextMarkerRange::characterRange() const
     return { { m_start.m_data.characterOffset, m_end.m_data.characterOffset - m_start.m_data.characterOffset } };
 }

+std::optional<AXTextMarkerRange> AXTextMarkerRange::intersectionWith(const AXTextMarkerRange& other) const
+{
+    if (m_start.m_data.treeID != m_end.m_data.treeID
+        || other.m_start.m_data.treeID != other.m_end.m_data.treeID
+        || m_start.m_data.treeID != other.m_start.m_data.treeID) {

AG: you may want to enclose this bool expression in UNLIKELY(...).

+        ASSERT_NOT_REACHED();
+        return std::nullopt;
+    }
+
+    // Fast path: both ranges span one object
+    if (m_start.m_data.objectID == m_end.m_data.objectID
+        && other.m_start.m_data.objectID == other.m_end.m_data.objectID) {
+        if (m_start.m_data.objectID != other.m_start.m_data.objectID)
+            return std::nullopt;
+
+        unsigned startOffset = std::max(m_start.m_data.characterOffset, other.m_start.m_data.characterOffset);
+        unsigned endOffset = std::min(m_end.m_data.characterOffset, other.m_end.m_data.characterOffset);
+
+        auto startMarker = AXTextMarker({ (AXID)m_start.m_data.treeID, (AXID)m_start.m_data.objectID, nullptr, startOffset, Position::PositionIsOffsetInAnchor, Affinity::Downstream, 0, startOffset });

AG: instead of casting (AXID), you can do m_start.m_data.treeID() and .objectID().

+        auto endMarker = AXTextMarker({ (AXID)m_start.m_data.treeID, (AXID)m_start.m_data.objectID, nullptr, endOffset, Position::PositionIsOffsetInAnchor, Affinity::Downstream, 0, endOffset });

AG: same here.

+        return { { startMarker, endMarker } };

AG: do we need to check whether startOffset <= endOffset? Otherwise I think we can get a range when there is no intersection.

+    }
+
+    return Accessibility::retrieveValueFromMainThread<std::optional<AXTextMarkerRange>>([this, &other] () -> std::optional<AXTextMarkerRange> {
+        auto intersection = WebCore::intersection(*this, other);
+        if (intersection.isNull())
+            return std::nullopt;
+
+        return { AXTextMarkerRange(intersection) };
+    });
+}
+
 String AXTextMarkerRange::debugDescription() const
 {
     return makeString("start: {", m_start.debugDescription(), "}\nend: {", m_end.debugDescription(), "}");
diff --git a/Source/WebCore/accessibility/AXTextMarker.h b/Source/WebCore/accessibility/AXTextMarker.h
index 2ffaeec026f0..2136c4fae872 100644
--- a/Source/WebCore/accessibility/AXTextMarker.h
+++ b/Source/WebCore/accessibility/AXTextMarker.h
@@ -156,6 +156,8 @@ public:
     std::optional<SimpleRange> simpleRange() const;
     std::optional<CharacterRange> characterRange() const;

+    std::optional<AXTextMarkerRange> intersectionWith(const AXTextMarkerRange& other) const;
+
 #if PLATFORM(MAC)
     RetainPtr<AXTextMarkerRangeRef> platformData() const;
     operator AXTextMarkerRangeRef() const { return platformData().autorelease(); }
diff --git a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
index f603c3f035b7..426333dc1f08 100644
--- a/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
+++ b/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm
@@ -292,6 +292,10 @@ using namespace WebCore;
 #define NSAccessibilitySelectTextWithCriteriaParameterizedAttribute @"AXSelectTextWithCriteria"
 #endif

+#ifndef NSAccessibilityIntersectionWithSelectionRangeAttribute
+#define NSAccessibilityIntersectionWithSelectionRangeAttribute @"AXIntersectionWithSelectionRange"
+#endif
+
 // Text search

 #ifndef NSAccessibilitySearchTextWithCriteriaParameterizedAttribute
@@ -1159,6 +1163,11 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
         [tempArray addObject:NSAccessibilityURLAttribute];
         return tempArray;
     }();
+    static NeverDestroyed staticTextAttrs = [] {
+        auto tempArray = adoptNS([[NSMutableArray alloc] initWithArray:attributes.get().get()]);
+        [tempArray addObject:NSAccessibilityIntersectionWithSelectionRangeAttribute];
+        return tempArray;
+    }();

     NSArray *objectAttributes = attributes.get().get();

@@ -1166,6 +1175,8 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
         objectAttributes = secureFieldAttributes.get().get();
     else if (backingObject->isWebArea())
         objectAttributes = webAreaAttrs.get().get();
+    else if (backingObject->isStaticText())
+        objectAttributes = staticTextAttrs.get().get();
     else if (backingObject->isTextControl())
         objectAttributes = textAttrs.get().get();
     else if (backingObject->isLink())
@@ -1626,6 +1637,11 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
         }
     }

+    if (backingObject->isStaticText()) {
+        if ([attributeName isEqualToString:NSAccessibilityIntersectionWithSelectionRangeAttribute])
+            return [self intersectionWithSelectionRange];
+    }
+
     if ([attributeName isEqualToString:NSAccessibilityVisibleCharacterRangeAttribute]) {
         if (backingObject->isSecureField())
             return nil;
@@ -2235,6 +2251,25 @@ ALLOW_DEPRECATED_IMPLEMENTATIONS_END
     return nil;
 }

+- (NSValue *)intersectionWithSelectionRange
+{
+    RefPtr<AXCoreObject> backingObject = self.updateObjectBackingStore;
+    if (!backingObject)
+        return nil;
+
+    auto objectRange = backingObject->textMarkerRange();
+    auto selectionRange = backingObject->selectedTextMarkerRange();
+
+    auto intersection = selectionRange.intersectionWith(objectRange);
+    if (intersection.has_value()) {
+        auto intersectionCharacterRange = intersection->characterRange();
+        if (intersectionCharacterRange.has_value())
+            return [NSValue valueWithRange:intersectionCharacterRange.value()];
+    }
+
+    return nil;
+}
+
 - (NSString *)accessibilityPlatformMathSubscriptKey
 {
     return NSAccessibilityMathSubscriptAttribute;
diff --git a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h b/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h
index c682f1d19750..0f38df2383c3 100644
--- a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h
+++ b/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h
@@ -178,6 +178,7 @@ public:
     unsigned numberOfCharacters() const;
     int insertionPointLineNumber();
     JSRetainPtr<JSStringRef> selectedTextRange();
+    JSRetainPtr<JSStringRef> intersectionWithSelectionRange();
     JSRetainPtr<JSStringRef> textInputMarkedRange() const;
     bool isAtomicLiveRegion() const;
     bool isBusy() const;
diff --git a/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl b/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
index dc7bfc9d1c12..a5102e5f7a69 100644
--- a/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
+++ b/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
@@ -60,6 +60,7 @@
     readonly attribute unsigned long numberOfCharacters;
     readonly attribute long insertionPointLineNumber;
     readonly attribute DOMString selectedTextRange;
+  readonly attribute DOMString intersectionWithSelectionRange;
     readonly attribute DOMString textInputMarkedRange;

     DOMString stringDescriptionOfAttributeValue(DOMString attr);
diff --git a/Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityUIElementAtspi.cpp b/Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityUIElementAtspi.cpp
index 4efe8d005d39..089243e47f62 100644
--- a/Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityUIElementAtspi.cpp
+++ b/Tools/WebKitTestRunner/InjectedBundle/atspi/AccessibilityUIElementAtspi.cpp
@@ -1426,6 +1426,11 @@ JSRetainPtr<JSStringRef> AccessibilityUIElement::selectedTextRange()
     return OpaqueJSString::tryCreate(range).leakRef();
 }

+JSRetainPtr<JSStringRef> AccessibilityUIElement::intersectionWithSelectionRange()
+{
+    return nullptr;
+}
+
 bool AccessibilityUIElement::setSelectedTextRange(unsigned location, unsigned length)
 {
     if (!m_element->interfaces().contains(WebCore::AccessibilityObjectAtspi::Interface::Text))
diff --git a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm b/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
index 2d566783d910..b708005c52cc 100644
--- a/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
+++ b/Tools/WebKitTestRunner/InjectedBundle/ios/AccessibilityUIElementIOS.mm
@@ -986,6 +986,11 @@ JSRetainPtr<JSStringRef> AccessibilityUIElement::selectedTextRange()
     return [rangeDescription createJSStringRef];
 }

+JSRetainPtr<JSStringRef> AccessibilityUIElement::intersectionWithSelectionRange()
+{
+    return nullptr;
+}
+
 bool AccessibilityUIElement::setSelectedTextMarkerRange(AccessibilityTextMarkerRange*)
 {
     return false;
diff --git a/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm b/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm
index b5532f4af0ba..f44996db29ef 100644
--- a/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm
+++ b/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm
@@ -89,6 +89,10 @@
 #define NSAccessibilityTextInputMarkedTextMarkerRangeAttribute @"AXTextInputMarkedTextMarkerRange"
 #endif

+#ifndef NSAccessibilityIntersectionWithSelectionRangeAttribute
+#define NSAccessibilityIntersectionWithSelectionRangeAttribute @"AXIntersectionWithSelectionRange"
+#endif
+
 typedef void (*AXPostedNotificationCallback)(id element, NSString* notification, void* context);

 @interface NSObject (WebKitAccessibilityAdditions)
@@ -134,7 +138,15 @@ bool AccessibilityUIElement::isEqual(AccessibilityUIElement* otherElement)
 #if ENABLE(ACCESSIBILITY_ISOLATED_TREE)
 bool AccessibilityUIElement::isIsolatedObject() const
 {
-    return [m_element isIsolatedObject];
+    BOOL value;
+
+    BEGIN_AX_OBJC_EXCEPTIONS
+    s_controller->executeOnAXThreadAndWait([this, &value] {
+        value = [m_element isIsolatedObject];
+    });
+    END_AX_OBJC_EXCEPTIONS
+
+    return value;
 }
 #endif

@@ -1542,13 +1554,27 @@ JSRetainPtr<JSStringRef> AccessibilityUIElement::selectedTextRange()
     auto indexRange = attributeValue(NSAccessibilitySelectedTextRangeAttribute);
     if (indexRange)
         range = [indexRange rangeValue];
-    NSMutableString* rangeDescription = [NSMutableString stringWithFormat:@"{%lu, %lu}", static_cast<unsigned long>(range.location), static_cast<unsigned long>(range.length)];
+    NSString *rangeDescription = [NSString stringWithFormat:@"{%lu, %lu}", static_cast<unsigned long>(range.location), static_cast<unsigned long>(range.length)];
     return [rangeDescription createJSStringRef];
     END_AX_OBJC_EXCEPTIONS

     return nullptr;
 }

+JSRetainPtr<JSStringRef> AccessibilityUIElement::intersectionWithSelectionRange()
+{
+    BEGIN_AX_OBJC_EXCEPTIONS
+    auto rangeAttribute = attributeValue(NSAccessibilityIntersectionWithSelectionRangeAttribute);
+    if (rangeAttribute) {
+        NSRange range = [rangeAttribute rangeValue];
+        NSMutableString* rangeDescription = [NSMutableString stringWithFormat:@"{%lu, %lu}", static_cast<unsigned long>(range.location), static_cast<unsigned long>(range.length)];

AG: NSMutableString* rangeDescription -> NSString *rangeDescription

+        return [rangeDescription createJSStringRef];
+    }
+    END_AX_OBJC_EXCEPTIONS
+
+    return nullptr;
+}
+
 bool AccessibilityUIElement::setSelectedTextRange(unsigned location, unsigned length)
 {
     NSRange textRange = NSMakeRange(location, length);
diff --git a/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityUIElementWin.cpp b/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityUIElementWin.cpp
index ff11fb7be199..b22e62ee8d02 100644
--- a/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityUIElementWin.cpp
+++ b/Tools/WebKitTestRunner/InjectedBundle/win/AccessibilityUIElementWin.cpp
@@ -658,6 +658,12 @@ JSRetainPtr<JSStringRef> AccessibilityUIElement::selectedTextRange()
     return nullptr;
 }

+JSRetainPtr<JSStringRef> AccessibilityUIElement::intersectionWithSelectionRange()
+{
+    notImplemented();
+    return nullptr;
+}
+
 bool AccessibilityUIElement::setSelectedTextRange(unsigned, unsigned)
 {
     notImplemented();
diff --git a/LayoutTests/accessibility/mac/intersection-with-selection-range-expected.txt b/LayoutTests/accessibility/mac/intersection-with-selection-range-expected.txt
new file mode 100644
index 000000000000..214287c6c542
--- /dev/null
+++ b/LayoutTests/accessibility/mac/intersection-with-selection-range-expected.txt
@@ -0,0 +1,46 @@
+This tests the intersectionWithSelectionRange api, which returns the range of characters in a static text node that are part of the document selection, if any.
+
+Trying range: text1:0 - text1:5 : Alpha
+PASS: window.getSelection().toString() === expectedText
+PASS: axStaticText.role === "AXRole: AXStaticText"
+PASS: axStaticText.intersectionWithSelectionRange === expectedAccessibleRange
+
+Trying range: text1:6 - text1:11 : Bravo
+PASS: window.getSelection().toString() === expectedText
+PASS: axStaticText.role === "AXRole: AXStaticText"
+PASS: axStaticText.intersectionWithSelectionRange === expectedAccessibleRange
+
+Trying range: text2:2 - text2:9 : Charlie
+PASS: window.getSelection().toString() === expectedText
+PASS: axStaticText.role === "AXRole: AXStaticText"
+PASS: axStaticText.intersectionWithSelectionRange === expectedAccessibleRange
+
+Trying range: text2:11 - text2:16 : Delta
+PASS: window.getSelection().toString() === expectedText
+PASS: axStaticText.role === "AXRole: AXStaticText"
+PASS: axStaticText.intersectionWithSelectionRange === expectedAccessibleRange
+
+Trying range: text1:6 - text2:9 : Bravo
+Charlie
+PASS: window.getSelection().toString() === expectedText
+PASS: axStaticText.role === "AXRole: AXStaticText"
+PASS: axStaticText.intersectionWithSelectionRange === expectedAccessibleRange
+
+Trying range: text1:0 - text1:5 : Alpha
+PASS: window.getSelection().toString() === expectedText
+PASS: axStaticText.role === "AXRole: AXStaticText"
+PASS: axStaticText.intersectionWithSelectionRange === expectedAccessibleRange
+
+Trying range: text1:6 - text2:9 : Bravo
+Charlie
+PASS: window.getSelection().toString() === expectedText
+PASS: axStaticText.role === "AXRole: AXStaticText"
+PASS: axStaticText.intersectionWithSelectionRange === expectedAccessibleRange
+
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Alpha Bravo
+Charlie Delta
+
diff --git a/LayoutTests/accessibility/mac/intersection-with-selection-range.html b/LayoutTests/accessibility/mac/intersection-with-selection-range.html
new file mode 100644
index 000000000000..db826a965400
--- /dev/null
+++ b/LayoutTests/accessibility/mac/intersection-with-selection-range.html
@@ -0,0 +1,64 @@
+<html>
+<head>
+<script src="../../resources/accessibility-helper.js"></script>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+
+<div id="text1">Alpha Bravo</div>
+<div id="text2">  Charlie  Delta  </div>
+
+<pre id="tree"></pre>
+
+<script>
+    var output = "This tests the intersectionWithSelectionRange api, which returns the range of characters in a static text node that are part of the document selection, if any.\n\n";
+
+    async function selectNodeIdRange(nodeId0, offset0, nodeId1, offset1) {
+        let root = accessibilityController.rootElement;
+        let previousAXSelection = root.stringForTextMarkerRange(root.selectedTextMarkerRange())
+        let sel = window.getSelection();
+        let range = document.createRange();
+        range.setStart(document.getElementById(nodeId0).firstChild, offset0);
+        range.setEnd(document.getElementById(nodeId1).firstChild, offset1);
+        sel.removeAllRanges();
+        sel.addRange(range);
+        if (root.isIsolatedObject) {

AG: we shouldn't need this, isIsolatedObject is a leftover that should be removed along with the test that is using it. Client shouldn't need to know whether it is isolated or not.

+            await waitFor(() => previousAXSelection != root.stringForTextMarkerRange(root.selectedTextMarkerRange()));
+        }
+    }
+
+    async function runTest(nodeId0, offset0, nodeId1, offset1,
+                     expectedText,
+                     accessibleElementId, expectedAccessibleRange) {
+        output += `Trying range: ${nodeId0}:${offset0} - ${nodeId1}:${offset1} : ${expectedText}\n`;
+        await selectNodeIdRange(nodeId0, offset0, nodeId1, offset1);
+        window.expectedText = expectedText;
+        output += expect('window.getSelection().toString()', 'expectedText');
+
+        axElement = accessibilityController.accessibleElementById(accessibleElementId);
+        axStaticText = axElement.childAtIndex(0);
+        output += expect('axStaticText.role', '"AXRole: AXStaticText"');
+        window.expectedAccessibleRange = expectedAccessibleRange
+        output += expect('axStaticText.intersectionWithSelectionRange', 'expectedAccessibleRange');
+
+        output += '\n';
+    }
+
+    if (window.accessibilityController) {
+        window.jsTestIsAsync = true;
+        setTimeout(async () => {
+            //dumpAccessibilityTree(accessibilityController.rootElement, null, 0, true, false, true);

AG: remove commented line.

+            await runTest('text1', 0, 'text1', 5, 'Alpha', 'text1', '{0, 5}');
+            await runTest('text1', 6, 'text1', 11, 'Bravo', 'text1', '{6, 5}');
+            await runTest('text2', 2, 'text2', 9, 'Charlie', 'text2', '{0, 7}');
+            await runTest('text2', 11, 'text2', 16, 'Delta', 'text2', '{8, 5}');
+            await runTest('text1', 6, 'text2', 9, 'Bravo\nCharlie', 'text1', '{6, 5}');
+            await runTest('text1', 0, 'text1', 5, 'Alpha', 'text2', null);
+            await runTest('text1', 6, 'text2', 9, 'Bravo\nCharlie', 'text2', '{0, 7}');
+            debug(output);
+            finishJSTest();
+        }, 0);
+    }
+</script>
+</body>
+</html>

[Dominic Mazzoni]

Created attachment 468896 [details]
Patch

[chris fleizach]

Comment on attachment 468896 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=468896&action=review

> Source/WebCore/accessibility/AXTextMarker.h:159
> +    std::optional<AXTextMarkerRange> intersectionWith(const AXTextMarkerRange& other) const;

std::optional<AXTextMarkerRange> intersectionWith(const AXTextMarkerRange&) const;

> Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl:63
> +  readonly attribute DOMString intersectionWithSelectionRange;

indentation

> Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm:1568
> +    if (rangeAttribute) {

if (auto *rangeAttribute = attributeValue(NSAccessibilityIntersectionWithSelectionRangeAttribute)) {

[Dominic Mazzoni]

Created attachment 468901 [details]
Patch

[Dominic Mazzoni]

Created attachment 468902 [details]
Patch

[Dominic Mazzoni]

Thanks for the reviews! I believe I addressed all of the feedback now and figured out how to get rid of the isIsolatedObject workaround.

[Andres Gonzalez]

(In reply to Dominic Mazzoni from comment #19)
> Thanks for the reviews! I believe I addressed all of the feedback now and
> figured out how to get rid of the isIsolatedObject workaround.

Thanks! If you have two ranges r1 = (start = = 1, end = 2) and r2 = (start = 3, end = 4), and you do

+        unsigned startOffset = std::max(m_start.m_data.characterOffset, other.m_start.m_data.characterOffset);
+        unsigned endOffset = std::min(m_end.m_data.characterOffset, other.m_end.m_data.characterOffset);
+
+        auto startMarker = AXTextMarker({ m_start.treeID(), m_start.objectID(), nullptr, startOffset, Position::PositionIsOffsetInAnchor, Affinity::Downstream, 0, startOffset });
+        auto endMarker = AXTextMarker({ m_start.treeID(), m_start.objectID(), nullptr, endOffset, Position::PositionIsOffsetInAnchor, Affinity::Downstream, 0, endOffset });
+        return { { startMarker, endMarker } };

I think we return a range (start = 3, end = 2) which behaves the same as (2, 3), but we should return null or an empty range in that case.

Minor style nit:

+        NSString* rangeDescription = [NSString stringWithFormat:@"{%lu, %lu}", static_cast<unsigned long>(range.location), static_cast<unsigned long>(range.length)];

WebKit coding standard calls for the * on the other side of the space for ObjC types, e.g., NSString *rangeDescription;

[Dominic Mazzoni]

Created attachment 468903 [details]
Patch

[Dominic Mazzoni]

Thanks. Fixed so that it returns an empty range in that case, and fixed the style nit.

[EWS]

Committed 271621@main (dbf11589ce2f): <https://commits.webkit.org/271621@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 468903 [details].