AX ITM: VoiceOver only reading "link" for some lines when navigating …

…via arrow up and down in contenteditables. https://bugs.webkit.org/show_bug.cgi?id=259310 rdar://112157851 Reviewed by Tyler Wilcock. The problem occurs for any <contenteditable> that contains children, e.g., a link or a span. We were returning nil for the AttributedString corresponding to the range of a line containing a child of the contenteditable. With this patch, if the range is not confined to a single object for which we cache the AttributedString, we fallback to the live objects to extract the attributedString. In addition, this patch adds several TextMarker related methods to WTR::AccessibilityUIElement for testing purpose. * LayoutTests/accessibility/mac/content-editable-attributed-string-expected.txt: Added. * LayoutTests/accessibility/mac/content-editable-attributed-string.html: Added. * LayoutTests/platform/mac-wk1/TestExpectations: * Source/WebCore/accessibility/AXTextMarker.cpp: (WebCore::AXTextMarker::debugDescription const): * Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm: (WebCore::AXIsolatedObject::attributedStringForTextMarkerRange const): * Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp: (WTR::AccessibilityUIElement::rightLineTextMarkerRangeForTextMarker): (WTR::AccessibilityUIElement::leftLineTextMarkerRangeForTextMarker): (WTR::AccessibilityUIElement::previousLineStartTextMarkerForTextMarker): (WTR::AccessibilityUIElement::nextLineEndTextMarkerForTextMarker): * Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h: * Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl: * Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm: (WTR::AccessibilityUIElement::lineTextMarkerRangeForTextMarker): (WTR::AccessibilityUIElement::rightLineTextMarkerRangeForTextMarker): (WTR::AccessibilityUIElement::leftLineTextMarkerRangeForTextMarker): (WTR::AccessibilityUIElement::previousLineStartTextMarkerForTextMarker): (WTR::AccessibilityUIElement::nextLineEndTextMarkerForTextMarker): (WTR::AccessibilityUIElement::previousWordStartTextMarkerForTextMarker): (WTR::AccessibilityUIElement::nextWordEndTextMarkerForTextMarker): Canonical link: https://commits.webkit.org/266138@main
WebKit · Jul 18, 2023 · e9f264d · e9f264d
1 parent 7f4d021
commit e9f264d
Show file tree

Hide file tree

Showing 9 changed files with 167 additions and 8 deletions.
diff --git a/LayoutTests/accessibility/mac/content-editable-attributed-string-expected.txt b/LayoutTests/accessibility/mac/content-editable-attributed-string-expected.txt
@@ -0,0 +1,52 @@
+Tests that AttributedStrings are retrieved properly from line ranges within a contenteditable with children.
+
+All text in the contenteditable: "AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, First line.
+Some text click me more text.
+Another line."
+First line: "AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, First line."
+Second line: "AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, AXFont - {
+    AXFontFamily = Times;
+    AXFontName = "Times-Roman";
+    AXFontSize = 16;
+}, Some text click me more text."
+third line: "AXFont - {
+    AXFontFamily = Times;
+    AXFontItalic = 1;
+    AXFontName = "Times-Italic";
+    AXFontSize = 16;
+}, Another line."
+
+PASS successfullyParsed is true
+
+TEST COMPLETE
+First line.
+Some text click me more text.
+Another line.
diff --git a/LayoutTests/accessibility/mac/content-editable-attributed-string.html b/LayoutTests/accessibility/mac/content-editable-attributed-string.html
@@ -0,0 +1,46 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="../../resources/accessibility-helper.js"></script>
+</head>
+<body>
+
+<div contenteditable id="editable">First line.<br>Some text <a href="#">click me</a> <span>more</span> text.<br><i>Another line.</i></div>
+
+<script>
+if (accessibilityController) {
+    let output = "Tests that AttributedStrings are retrieved properly from line ranges within a contenteditable with children.\n\n";
+
+    let text = accessibilityController.accessibleElementById("editable");
+    let range = text.textMarkerRangeForElement(text);
+    let string = text.attributedStringForTextMarkerRange(range);
+    output += `All text in the contenteditable: "${string}"\n`;
+
+    // Get the range for the first line.
+    let start = text.startTextMarkerForTextMarkerRange(range);
+    range = text.lineTextMarkerRangeForTextMarker(start);
+    string = text.attributedStringForTextMarkerRange(range);
+    output += `First line: "${string}"\n`;
+
+    // Get the range for the second line:
+    let end = text.nextLineEndTextMarkerForTextMarker(start);
+    end = text.nextTextMarker(end);
+    end = text.nextTextMarker(end);
+    range = text.lineTextMarkerRangeForTextMarker(end);
+    string = text.attributedStringForTextMarkerRange(range);
+    output += `Second line: "${string}"\n`;
+
+    // Get the range for the third line:
+    end = text.nextLineEndTextMarkerForTextMarker(end);
+    end = text.nextTextMarker(end);
+    end = text.nextTextMarker(end);
+    range = text.lineTextMarkerRangeForTextMarker(end);
+    string = text.attributedStringForTextMarkerRange(range);
+    output += `third line: "${string}"\n`;
+
+    debug(output);
+}
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac-wk1/TestExpectations b/LayoutTests/platform/mac-wk1/TestExpectations
@@ -827,6 +827,7 @@ accessibility/display-contents/dynamically-added-children.html [ Skip ]
 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 ]
 
 # DOM paste access requests are not implemented in WebKit1.

diff --git a/Source/WebCore/accessibility/AXTextMarker.cpp b/Source/WebCore/accessibility/AXTextMarker.cpp
@@ -204,9 +204,11 @@ RefPtr<AXCoreObject> AXTextMarker::object() const
 String AXTextMarker::debugDescription() const
 {
     auto separator = ", ";
+    RefPtr object = this->object();
     return makeString(
         "treeID ", treeID().loggingString()
         , separator, "objectID ", objectID().loggingString()
+        , separator, "role ", object ? accessibilityRoleToString(object->roleValue()) : String("no object"_s)
         , separator, isMainThread() ? node()->debugDescription()
             : makeString("node 0x", hex(reinterpret_cast<uintptr_t>(m_data.node)))
         , separator, "offset ", m_data.offset

diff --git a/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm b/Source/WebCore/accessibility/isolatedtree/mac/AXIsolatedObjectMac.mm
@@ -177,18 +177,19 @@
     if (!markerRange)
         return nil;
 
-    if (markerRange.start().objectID() != objectID() && markerRange.isConfinedTo(markerRange.start().objectID())) {
+    // At the moment we are only handling ranges that are confined to a single object, and for which we cached the AttributeString.
+    // FIXME: Extend to cases where the range expands multiple objects.
+
+    bool isConfined = markerRange.isConfinedTo(markerRange.start().objectID());
+    if (isConfined && markerRange.start().objectID() != 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) {
+    if (!isConfined || !attributedText) {
         return Accessibility::retrieveValueFromMainThread<RetainPtr<NSAttributedString>>([markerRange = WTFMove(markerRange), &spellCheck, this] () mutable -> RetainPtr<NSAttributedString> {
             if (RefPtr axObject = associatedAXObject()) {
                 // Ensure that the TextMarkers have valid Node references, in case the range was created on the AX thread.

diff --git a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp b/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.cpp
@@ -105,6 +105,10 @@ JSRetainPtr<JSStringRef> AccessibilityUIElement::sentenceAtOffset(int) { return
 
 #if !PLATFORM(MAC) || !ENABLE(ACCESSIBILITY)
 bool AccessibilityUIElement::isTextMarkerNull(AccessibilityTextMarker* marker) { return !isTextMarkerValid(marker); }
+RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::rightLineTextMarkerRangeForTextMarker(AccessibilityTextMarker*) { return nullptr; }
+RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::leftLineTextMarkerRangeForTextMarker(AccessibilityTextMarker*) { return nullptr; }
+RefPtr<AccessibilityTextMarker> AccessibilityUIElement::previousLineStartTextMarkerForTextMarker(AccessibilityTextMarker*) { return nullptr; }
+RefPtr<AccessibilityTextMarker> AccessibilityUIElement::nextLineEndTextMarkerForTextMarker(AccessibilityTextMarker*) { return nullptr; }
 int AccessibilityUIElement::lineIndexForTextMarker(AccessibilityTextMarker*) const { return -1; }
 RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::textMarkerRangeForRange(unsigned, unsigned) { return nullptr; }
 RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::selectedTextMarkerRange() { return nullptr; }

diff --git a/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h b/Tools/WebKitTestRunner/InjectedBundle/AccessibilityUIElement.h
@@ -328,9 +328,13 @@ class AccessibilityUIElement : public JSWrappable {
     void scrollToMakeVisible();
     void scrollToGlobalPoint(int x, int y);
     void scrollToMakeVisibleWithSubFocus(int x, int y, int width, int height);
-    
+
     // Text markers.
     RefPtr<AccessibilityTextMarkerRange> lineTextMarkerRangeForTextMarker(AccessibilityTextMarker*);
+    RefPtr<AccessibilityTextMarkerRange> rightLineTextMarkerRangeForTextMarker(AccessibilityTextMarker*);
+    RefPtr<AccessibilityTextMarkerRange> leftLineTextMarkerRangeForTextMarker(AccessibilityTextMarker*);
+    RefPtr<AccessibilityTextMarker> previousLineStartTextMarkerForTextMarker(AccessibilityTextMarker*);
+    RefPtr<AccessibilityTextMarker> nextLineEndTextMarkerForTextMarker(AccessibilityTextMarker*);
     int lineIndexForTextMarker(AccessibilityTextMarker*) const;
     RefPtr<AccessibilityTextMarkerRange> misspellingTextMarkerRange(AccessibilityTextMarkerRange* start, bool forward);
     RefPtr<AccessibilityTextMarkerRange> textMarkerRangeForElement(AccessibilityUIElement*);

diff --git a/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl b/Tools/WebKitTestRunner/InjectedBundle/Bindings/AccessibilityUIElement.idl
@@ -233,6 +233,10 @@
 
     // Text markers.
     AccessibilityTextMarkerRange lineTextMarkerRangeForTextMarker(AccessibilityTextMarker textMarker);
+    AccessibilityTextMarkerRange rightLineTextMarkerRangeForTextMarker(AccessibilityTextMarker textMarker);
+    AccessibilityTextMarkerRange leftLineTextMarkerRangeForTextMarker(AccessibilityTextMarker textMarker);
+    AccessibilityTextMarker previousLineStartTextMarkerForTextMarker(AccessibilityTextMarker textMarker);
+    AccessibilityTextMarker nextLineEndTextMarkerForTextMarker(AccessibilityTextMarker textMarker);
     long lineIndexForTextMarker(AccessibilityTextMarker textMarker);
     AccessibilityTextMarkerRange misspellingTextMarkerRange(AccessibilityTextMarkerRange start, boolean forward);
     AccessibilityTextMarkerRange textMarkerRangeForElement(AccessibilityUIElement element);

diff --git a/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm b/Tools/WebKitTestRunner/InjectedBundle/mac/AccessibilityUIElementMac.mm
@@ -1888,7 +1888,54 @@ void setAttributeValue(id element, NSString* attribute, id value, bool synchrono
     auto textMarkerRange = attributeValueForParameter(@"AXLineTextMarkerRangeForTextMarker", textMarker->platformTextMarker());
     return AccessibilityTextMarkerRange::create(textMarkerRange.get());
     END_AX_OBJC_EXCEPTIONS
+    return nullptr;
+}
+
+RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::rightLineTextMarkerRangeForTextMarker(AccessibilityTextMarker* textMarker)
+{
+    if (!textMarker)
+        return nullptr;
+
+    BEGIN_AX_OBJC_EXCEPTIONS
+    auto textMarkerRange = attributeValueForParameter(@"AXRightLineTextMarkerRangeForTextMarker", textMarker->platformTextMarker());
+    return AccessibilityTextMarkerRange::create(textMarkerRange.get());
+    END_AX_OBJC_EXCEPTIONS
+    return nullptr;
+}
+
+RefPtr<AccessibilityTextMarkerRange> AccessibilityUIElement::leftLineTextMarkerRangeForTextMarker(AccessibilityTextMarker* textMarker)
+{
+    if (!textMarker)
+        return nullptr;
 
+    BEGIN_AX_OBJC_EXCEPTIONS
+    auto textMarkerRange = attributeValueForParameter(@"AXLeftLineTextMarkerRangeForTextMarker", textMarker->platformTextMarker());
+    return AccessibilityTextMarkerRange::create(textMarkerRange.get());
+    END_AX_OBJC_EXCEPTIONS
+    return nullptr;
+}
+
+RefPtr<AccessibilityTextMarker> AccessibilityUIElement::previousLineStartTextMarkerForTextMarker(AccessibilityTextMarker* textMarker)
+{
+    if (!textMarker)
+        return nullptr;
+
+    BEGIN_AX_OBJC_EXCEPTIONS
+    auto marker = attributeValueForParameter(@"AXPreviousLineStartTextMarkerForTextMarker", textMarker->platformTextMarker());
+    return AccessibilityTextMarker::create(marker.get());
+    END_AX_OBJC_EXCEPTIONS
+    return nullptr;
+}
+
+RefPtr<AccessibilityTextMarker> AccessibilityUIElement::nextLineEndTextMarkerForTextMarker(AccessibilityTextMarker* textMarker)
+{
+    if (!textMarker)
+        return nullptr;
+
+    BEGIN_AX_OBJC_EXCEPTIONS
+    auto marker = attributeValueForParameter(@"AXNextLineEndTextMarkerForTextMarker", textMarker->platformTextMarker());
+    return AccessibilityTextMarker::create(marker.get());
+    END_AX_OBJC_EXCEPTIONS
     return nullptr;
 }
 
@@ -2309,7 +2356,6 @@ void setAttributeValue(id element, NSString* attribute, id value, bool synchrono
     auto previousWordStartMarker = attributeValueForParameter(@"AXPreviousWordStartTextMarkerForTextMarker", textMarker->platformTextMarker());
     return AccessibilityTextMarker::create(previousWordStartMarker.get());
     END_AX_OBJC_EXCEPTIONS
-
     return nullptr;
 }
 
@@ -2322,7 +2368,6 @@ void setAttributeValue(id element, NSString* attribute, id value, bool synchrono
     auto nextWordEndMarker = attributeValueForParameter(@"AXNextWordEndTextMarkerForTextMarker", textMarker->platformTextMarker());
     return AccessibilityTextMarker::create(nextWordEndMarker.get());
     END_AX_OBJC_EXCEPTIONS
-
     return nullptr;
 }