Title: [275217] trunk
Revision
275217
Author
andresg...@apple.com
Date
2021-03-30 11:47:17 -0700 (Tue, 30 Mar 2021)

Log Message

Fix for accessibility/textarea-insertion-point-line-number.html.
https://bugs.webkit.org/show_bug.cgi?id=223936
<rdar://problem/76007361>

Reviewed by Chris Fleizach.

Source/WebCore:

Test: accessibility/textarea-insertion-point-line-number.html

Added AXCoreObject::insertionPointLineNumber to support this functionality.
This replaces the previous implementation in the wrapper's accessibilityAttributeValue.
There were several problems with the previous implementation that was
doing an unnecessary and buggy round trip from Ranges to indexes and
back to VisiblePositions.

* accessibility/AccessibilityObject.h:
* accessibility/AccessibilityObjectInterface.h:
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::insertionPointLineNumber const):
* accessibility/AccessibilityRenderObject.h:
* accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::insertionPointLineNumber const):
(WebCore::AXIsolatedObject::selectionStart const): Deleted.
(WebCore::AXIsolatedObject::selectionEnd const): Deleted.
* accessibility/isolatedtree/AXIsolatedObject.h:
* accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
(-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):

LayoutTests:

Updated this test and corrected several issues where the expected
returned values were incorrect.

* accessibility/textarea-insertion-point-line-number-expected.txt:
* accessibility/textarea-insertion-point-line-number.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (275216 => 275217)


--- trunk/LayoutTests/ChangeLog	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/LayoutTests/ChangeLog	2021-03-30 18:47:17 UTC (rev 275217)
@@ -1,3 +1,17 @@
+2021-03-30  Andres Gonzalez  <andresg...@apple.com>
+
+        Fix for accessibility/textarea-insertion-point-line-number.html.
+        https://bugs.webkit.org/show_bug.cgi?id=223936
+        <rdar://problem/76007361>
+
+        Reviewed by Chris Fleizach.
+
+        Updated this test and corrected several issues where the expected
+        returned values were incorrect.
+
+        * accessibility/textarea-insertion-point-line-number-expected.txt:
+        * accessibility/textarea-insertion-point-line-number.html:
+
 2021-03-30  Aditya Keerthi  <akeer...@apple.com>
 
         [iOS] Two taps required to view <select> options on Square Checkout

Modified: trunk/LayoutTests/accessibility/textarea-insertion-point-line-number-expected.txt (275216 => 275217)


--- trunk/LayoutTests/accessibility/textarea-insertion-point-line-number-expected.txt	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/LayoutTests/accessibility/textarea-insertion-point-line-number-expected.txt	2021-03-30 18:47:17 UTC (rev 275217)
@@ -1,20 +1,21 @@
+
+Line1
+
+Line3
 This tests that lineNumberForPosition is reported correctly for textarea and contenteditable elements.
 
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS area1.selectionStart = (0); area1.selectionEnd = (0); area1AXUIElement.insertionPointLineNumber; is currentLine
-PASS area1.selectionStart = (7); area1.selectionEnd = (7); area1AXUIElement.insertionPointLineNumber; is currentLine
-PASS area1.selectionStart = (14); area1.selectionEnd = (14); area1AXUIElement.insertionPointLineNumber; is currentLine
-PASS area2.focus(); area1AXUIElement.insertionPointLineNumber; is -1
-PASS window.getSelection().setBaseAndExtent(contenteditableLine1, 1, contenteditableLine1, 1); contenteditableAXUIElement.insertionPointLineNumber; is 0
-PASS contenteditableLine2.selectionStart = 1; contenteditableLine2.selectionEnd = 1;contenteditableAXUIElement.insertionPointLineNumber; is -1
-PASS window.getSelection().setBaseAndExtent(contenteditableLine3, 1, contenteditableLine3, 1); contenteditableAXUIElement.insertionPointLineNumber; is 2
+PASS area1AXUIElement.insertionPointLineNumber is currentLine
+PASS area1AXUIElement.insertionPointLineNumber is currentLine
+PASS area1AXUIElement.insertionPointLineNumber is currentLine
+PASS area2AXUIElement.insertionPointLineNumber is 0
+PASS contenteditableAXUIElement.insertionPointLineNumber is 0
+PASS contenteditableAXUIElement.insertionPointLineNumber is 1
+PASS contenteditableAXUIElement.insertionPointLineNumber is 2
 PASS textareaAXUIElement.insertionPointLineNumber is 0
 PASS successfullyParsed is true
 
 TEST COMPLETE
 
-Line1
-
-Line3

Modified: trunk/LayoutTests/accessibility/textarea-insertion-point-line-number.html (275216 => 275217)


--- trunk/LayoutTests/accessibility/textarea-insertion-point-line-number.html	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/LayoutTests/accessibility/textarea-insertion-point-line-number.html	2021-03-30 18:47:17 UTC (rev 275217)
@@ -1,61 +1,74 @@
 <html>
-<script src=""
+<head>
+<script src=""
+</head>
 <body>
-    <div id="console"></div>
-    <textarea name="area1" id="area1" rows="5" cols="40">
+
+<textarea name="area1" id="area1" rows="5" cols="40">
 line 1
 line 2
 line 3
-    </textarea>
+</textarea>
 
-    <textarea name="area2" id="area2" rows="5" cols="40"></textarea>
+<textarea name="area2" id="area2" rows="5" cols="40"></textarea>
 
+<div id="contenteditable-div" role="textbox" contenteditable="true" tabindex="0">
+    <div id="contenteditable-line1">Line1</div>
+    <textarea id="contenteditable-line2" rows="1" cols="40">Line2</textarea>
+    <div id="contenteditable-line3">Line3</div>
+</div>
 
-    <div id="contenteditable-div" role="textbox" contenteditable="true" tabindex="0">
-      <div id="contenteditable-line1">Line1</div>
-      <textarea id="contenteditable-line2" rows="1" cols="40">Line2</textarea>
-      <div id="contenteditable-line3">Line3</div>
-    </div>
-    <script>
-        description("This tests that lineNumberForPosition is reported correctly for textarea and contenteditable elements.");
+<p id="description"></p>
+<div id="console"></div>
 
-        if (window.accessibilityController) {
-            var console = document.getElementById("console");
+<script>
+    description("This tests that lineNumberForPosition is reported correctly for textarea and contenteditable elements.");
 
-            var area1 = document.getElementById("area1");
-            area1.focus();
-            var area1AXUIElement = accessibilityController.focusedElement;
-            var lineNumber = -2;
+    if (window.accessibilityController) {
+        var area1 = document.getElementById("area1");
+        area1.focus();
+        var area1AXUIElement = accessibilityController.focusedElement;
+        var lineNumber = -2;
 
-            for (var currentLine = 0; currentLine < 3; currentLine++ ) {
-                shouldBe("area1.selectionStart = (" + (currentLine * 7) + "); " +
-                         "area1.selectionEnd = (" + (currentLine * 7) +"); " +
-                         "area1AXUIElement.insertionPointLineNumber;", "currentLine");
-            }
+        // Set the insertion point to the beginning of each line in area1 and
+        // check that accessibility returns the correct line number.
+        for (var currentLine = 0; currentLine < 3; currentLine++) {
+            area1.selectionStart = currentLine * 7;
+            area1.selectionEnd = currentLine * 7;
+            shouldBe("area1AXUIElement.insertionPointLineNumber", "currentLine");
+        }
 
-            var area2 = document.getElementById("area2");
-            shouldBe("area2.focus(); area1AXUIElement.insertionPointLineNumber;", "-1");
+        // area2 is an empty textarea so the insertion point line number should be 0.
+        var area2 = document.getElementById("area2");
+        area2.focus();
+        var area2AXUIElement = accessibilityController.focusedElement;
+        shouldBe("area2AXUIElement.insertionPointLineNumber", "0");
 
-            var contenteditableDiv = document.getElementById("contenteditable-div");
-            contenteditableDiv.focus();
-            var contenteditableAXUIElement = accessibilityController.focusedElement;
+        // Set focus to the contenteditable-div element and set the insertion
+        // point in each of the children lines.
+        var contenteditableDiv = document.getElementById("contenteditable-div");
+        contenteditableDiv.focus();
+        var contenteditableAXUIElement = accessibilityController.focusedElement;
 
-            var contenteditableLine1 = document.getElementById("contenteditable-line1");
-            shouldBe("window.getSelection().setBaseAndExtent(contenteditableLine1, 1, contenteditableLine1, 1); " +
-                     "contenteditableAXUIElement.insertionPointLineNumber;", "0");
+        var contenteditableLine1 = document.getElementById("contenteditable-line1");
+        window.getSelection().setBaseAndExtent(contenteditableLine1, 1, contenteditableLine1, 1);
+        shouldBe("contenteditableAXUIElement.insertionPointLineNumber", "0");
 
-            var contenteditableLine2 = document.getElementById("contenteditable-line2");
-            shouldBe("contenteditableLine2.selectionStart = 1; contenteditableLine2.selectionEnd = 1;" +
-                     "contenteditableAXUIElement.insertionPointLineNumber;", "-1");
+        var contenteditableLine2 = document.getElementById("contenteditable-line2");
+        window.getSelection().setBaseAndExtent(contenteditableLine2, 1, contenteditableLine2, 1);
+        shouldBe("contenteditableAXUIElement.insertionPointLineNumber", "1");
 
-            var contenteditableLine3 = document.getElementById("contenteditable-line3");
-            shouldBe("window.getSelection().setBaseAndExtent(contenteditableLine3, 1, contenteditableLine3, 1); " +
-                     "contenteditableAXUIElement.insertionPointLineNumber;", "2");
+        var contenteditableLine3 = document.getElementById("contenteditable-line3");
+        window.getSelection().setBaseAndExtent(contenteditableLine3, 1, contenteditableLine3, 1);
+        shouldBe("contenteditableAXUIElement.insertionPointLineNumber", "2");
 
-            contenteditableLine2.focus();
-            var textareaAXUIElement = accessibilityController.focusedElement;
-            shouldBe("textareaAXUIElement.insertionPointLineNumber", "0");
-        }
-    </script>
+        // Set focus to line 2 and get the insertion point line number relative
+        // to the nested textarea, which should be 0.
+        contenteditableLine2.focus();
+        var textareaAXUIElement = accessibilityController.focusedElement;
+        shouldBe("textareaAXUIElement.insertionPointLineNumber", "0");
+    }
+</script>
+<script src=""
 </body>
 </html>

Modified: trunk/Source/WebCore/ChangeLog (275216 => 275217)


--- trunk/Source/WebCore/ChangeLog	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/Source/WebCore/ChangeLog	2021-03-30 18:47:17 UTC (rev 275217)
@@ -1,3 +1,32 @@
+2021-03-30  Andres Gonzalez  <andresg...@apple.com>
+
+        Fix for accessibility/textarea-insertion-point-line-number.html.
+        https://bugs.webkit.org/show_bug.cgi?id=223936
+        <rdar://problem/76007361>
+
+        Reviewed by Chris Fleizach.
+
+        Test: accessibility/textarea-insertion-point-line-number.html
+
+        Added AXCoreObject::insertionPointLineNumber to support this functionality.
+        This replaces the previous implementation in the wrapper's accessibilityAttributeValue.
+        There were several problems with the previous implementation that was
+        doing an unnecessary and buggy round trip from Ranges to indexes and
+        back to VisiblePositions.
+
+        * accessibility/AccessibilityObject.h:
+        * accessibility/AccessibilityObjectInterface.h:
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::insertionPointLineNumber const):
+        * accessibility/AccessibilityRenderObject.h:
+        * accessibility/isolatedtree/AXIsolatedObject.cpp:
+        (WebCore::AXIsolatedObject::insertionPointLineNumber const):
+        (WebCore::AXIsolatedObject::selectionStart const): Deleted.
+        (WebCore::AXIsolatedObject::selectionEnd const): Deleted.
+        * accessibility/isolatedtree/AXIsolatedObject.h:
+        * accessibility/mac/WebAccessibilityObjectWrapperMac.mm:
+        (-[WebAccessibilityObjectWrapper accessibilityAttributeValue:]):
+
 2021-03-29  Simon Fraser  <simon.fra...@apple.com>
 
         Allow non-60fps display updates to be driven by DisplayRefreshMonitor

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.h (275216 => 275217)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.h	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.h	2021-03-30 18:47:17 UTC (rev 275217)
@@ -431,9 +431,8 @@
     bool supportsPath() const override { return false; }
 
     TextIteratorBehavior textIteratorBehaviorForTextRange() const override;
-    PlainTextRange selectedTextRange() const override { return PlainTextRange(); }
-    unsigned selectionStart() const override { return selectedTextRange().start; }
-    unsigned selectionEnd() const override { return selectedTextRange().length; }
+    PlainTextRange selectedTextRange() const override { return { }; }
+    int insertionPointLineNumber() const override { return -1; }
 
     URL url() const override { return URL(); }
     VisibleSelection selection() const override { return VisibleSelection(); }

Modified: trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h (275216 => 275217)


--- trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/Source/WebCore/accessibility/AccessibilityObjectInterface.h	2021-03-30 18:47:17 UTC (rev 275217)
@@ -1174,9 +1174,7 @@
 
     virtual TextIteratorBehavior textIteratorBehaviorForTextRange() const = 0;
     virtual PlainTextRange selectedTextRange() const = 0;
-    // FIXME: why do we need the following two methods if we already have selectedTextRange?
-    virtual unsigned selectionStart() const = 0;
-    virtual unsigned selectionEnd() const = 0;
+    virtual int insertionPointLineNumber() const = 0;
 
     virtual URL url() const = 0;
     virtual VisibleSelection selection() const = 0;

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (275216 => 275217)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2021-03-30 18:47:17 UTC (rev 275217)
@@ -1665,6 +1665,36 @@
     return documentBasedSelectedTextRange();
 }
 
+int AccessibilityRenderObject::insertionPointLineNumber() const
+{
+    ASSERT(isTextControl());
+
+    // Use the text control native range if it's a native object.
+    if (isNativeTextControl()) {
+        auto& textControl = downcast<RenderTextControl>(*m_renderer).textFormControlElement();
+        int start = textControl.selectionStart();
+        int end = textControl.selectionEnd();
+
+        // If the selection range is not a collapsed range, we don't know whether the insertion point is the start or the end, thus return -1.
+        // FIXME: for non-collapsed selection, determine the insertion point based on the TextFieldSelectionDirection.
+        if (start != end)
+            return -1;
+
+        return lineForPosition(textControl.visiblePositionForIndex(start));
+    }
+
+    auto* frame = this->frame();
+    if (!frame)
+        return -1;
+
+    auto selectedTextRange = frame->selection().selection().firstRange();
+    // If the selection range is not a collapsed range, we don't know whether the insertion point is the start or the end, thus return -1.
+    if (!selectedTextRange || !selectedTextRange->collapsed())
+        return -1;
+
+    return lineForPosition(makeDeprecatedLegacyPosition(selectedTextRange->start));
+}
+
 static void setTextSelectionIntent(AXObjectCache* cache, AXTextStateChangeType type)
 {
     if (!cache)

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h (275216 => 275217)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.h	2021-03-30 18:47:17 UTC (rev 275217)
@@ -127,6 +127,7 @@
     
     URL url() const override;
     PlainTextRange selectedTextRange() const override;
+    int insertionPointLineNumber() const override;
     VisibleSelection selection() const override;
     String stringValue() const override;
     String helpText() const override;

Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp (275216 => 275217)


--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp	2021-03-30 18:47:17 UTC (rev 275217)
@@ -1340,6 +1340,15 @@
     });
 }
 
+int AXIsolatedObject::insertionPointLineNumber() const
+{
+    return Accessibility::retrieveValueFromMainThread<int>([this] () -> int {
+        if (auto* axObject = associatedAXObject())
+            return axObject->insertionPointLineNumber();
+        return -1;
+    });
+}
+
 PlainTextRange AXIsolatedObject::doAXRangeForLine(unsigned lineIndex) const
 {
     return Accessibility::retrieveValueFromMainThread<PlainTextRange>([&lineIndex, this] () -> PlainTextRange {
@@ -1436,24 +1445,6 @@
     return axObject ? axObject->elementRange() : WTF::nullopt;
 }
 
-unsigned AXIsolatedObject::selectionStart() const
-{
-    return Accessibility::retrieveValueFromMainThread<unsigned>([this] () -> unsigned {
-        if (auto* object = associatedAXObject())
-            return object->selectionStart();
-        return 0;
-    });
-}
-
-unsigned AXIsolatedObject::selectionEnd() const
-{
-    return Accessibility::retrieveValueFromMainThread<unsigned>([this] () -> unsigned {
-        if (auto* object = associatedAXObject())
-            return object->selectionEnd();
-        return 0;
-    });
-}
-
 String AXIsolatedObject::selectedText() const
 {
     return Accessibility::retrieveValueFromMainThread<String>([this] () -> String {

Modified: trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h (275216 => 275217)


--- trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h	2021-03-30 18:47:17 UTC (rev 275217)
@@ -373,6 +373,7 @@
 
     // PlainTextRange support.
     PlainTextRange selectedTextRange() const override;
+    int insertionPointLineNumber() const override;
     PlainTextRange doAXRangeForLine(unsigned) const override;
     String doAXStringForRange(const PlainTextRange&) const override;
     PlainTextRange doAXRangeForPosition(const IntPoint&) const override;
@@ -386,8 +387,6 @@
     void setSelectedVisiblePositionRange(const VisiblePositionRange&) const override;
 
     // TODO: Text ranges and selection.
-    unsigned selectionStart() const override;
-    unsigned selectionEnd() const override;
     String selectedText() const override;
     VisiblePositionRange visiblePositionRange() const override;
     VisiblePositionRange visiblePositionRangeForLine(unsigned) const override;

Modified: trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm (275216 => 275217)


--- trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm	2021-03-30 18:32:20 UTC (rev 275216)
+++ trunk/Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm	2021-03-30 18:47:17 UTC (rev 275217)
@@ -2240,23 +2240,7 @@
             return backingObject->isPasswordField() ? nil : [NSValue valueWithRange: NSMakeRange(0, backingObject->textLength())];
 
         if ([attributeName isEqualToString:NSAccessibilityInsertionPointLineNumberAttribute]) {
-            // if selectionEnd > 0, then there is selected text and this question should not be answered
-            if (backingObject->isPasswordField() || backingObject->selectionEnd() > 0)
-                return nil;
-
-            auto* focusedObject = backingObject->focusedUIElement();
-            if (focusedObject != backingObject)
-                return nil;
-
-            int lineNumber = Accessibility::retrieveValueFromMainThread<int>([protectedSelf = retainPtr(self)] () -> int {
-                auto* backingObject = protectedSelf.get().axBackingObject;
-                if (!backingObject)
-                    return -1;
-
-                auto focusedPosition = backingObject->visiblePositionForIndex(backingObject->selectionStart(), true);
-                return backingObject->lineForPosition(focusedPosition);
-            });
-
+            int lineNumber = backingObject->insertionPointLineNumber();
             return lineNumber >= 0 ? @(lineNumber) : nil;
         }
     }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to