Braille display is blank in contenteditable elements when the field i…

…s followed by another element. https://bugs.webkit.org/show_bug.cgi?id=229713 rdar://82095237 Reviewed by Chris Fleizach. Source/WebCore: Test: accessibility/mac/range-for-line-index.html We were making the length of line ranges in text fields 1 more than the number of characters in the line even when no line break character existed, like in the case of a single line text field. Clients like VoiceOver expect the length of the line ranges in text fields to match the number of charaters in the line including the line break if one exists. * accessibility/AccessibilityRenderObject.cpp: (WebCore::isHardLineBreak): Helper function used in doAXRangeForLine. Determines whether the given VisiblePosition corresponds to a hard line break. (WebCore::AccessibilityRenderObject::doAXRangeForLine const): Returns a PlainTextRange whose length matches the number of characters in the given line, accounting for line break characters. LayoutTests: * accessibility/mac/range-for-line-index-expected.txt: Added. * accessibility/mac/range-for-line-index.html: Added. * platform/mac/accessibility/content-editable-as-textarea-expected.txt: * platform/win/accessibility/content-editable-as-textarea-expected.txt: Canonical link: https://commits.webkit.org/241229@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@281920 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Sep 2, 2021 · 03ae90e · 03ae90e
1 parent a0b8a86
commit 03ae90e
Show file tree

Hide file tree

Showing 7 changed files with 161 additions and 25 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2021-09-02  Andres Gonzalez  <andresg_22@apple.com>
+
+        Braille display is blank in contenteditable elements when the field is followed by another element.
+        https://bugs.webkit.org/show_bug.cgi?id=229713
+        rdar://82095237
+
+        Reviewed by Chris Fleizach.
+
+        * accessibility/mac/range-for-line-index-expected.txt: Added.
+        * accessibility/mac/range-for-line-index.html: Added.
+        * platform/mac/accessibility/content-editable-as-textarea-expected.txt:
+        * platform/win/accessibility/content-editable-as-textarea-expected.txt:
+
 2021-09-02  Philippe Normand  <pnormand@igalia.com>
 
         Unreviewed, GLib gardening

diff --git a/LayoutTests/accessibility/mac/range-for-line-index-expected.txt b/LayoutTests/accessibility/mac/range-for-line-index-expected.txt
@@ -0,0 +1,23 @@
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS contenteditable.rangeForLine(0) is '{0, 15}'
+PASS multilineContenteditable.rangeForLine(0) is '{0, 16}'
+PASS multilineContenteditable.rangeForLine(1) is '{15, 1}'
+PASS multilineContenteditable.rangeForLine(0) is '{0, 16}'
+PASS multilineContenteditable.rangeForLine(1) is '{16, 6}'
+PASS multilineContenteditable.rangeForLine(2) is '{22, 5}'
+PASS textarea.rangeForLine(0) is '{0, 15}'
+PASS multilineTextarea.rangeForLine(0) is '{0, 16}'
+PASS multilineTextarea.rangeForLine(1) is '{16, 6}'
+PASS multilineTextarea.rangeForLine(2) is '{22, 5}'
+PASS textfield.rangeForLine(0) is '{0, 21}'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+this is a test.
+this is a test.
+this is a test.
+hello
+world
+
diff --git a/LayoutTests/accessibility/mac/range-for-line-index.html b/LayoutTests/accessibility/mac/range-for-line-index.html
@@ -0,0 +1,53 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+
+<div id="contenteditable1" contenteditable="true">this is a test.</div>
+<div id="contenteditable2" contenteditable="true">this is a test.<br></div>
+<div id="contenteditable3" contenteditable="true">this is a test.<br>hello<br>
+    <a href="#">world</a>
+</div>
+
+<textarea id="textarea1">this is a test.</textarea>
+<textarea id="textarea2">this is a test.
+hello
+world</textarea>
+
+<input type="text" id="textfield" value="this is a text field."></input>
+
+<script>
+    if (window.accessibilityController) {
+        description("");
+
+        // contenteditable
+        var contenteditable = accessibilityController.accessibleElementById("contenteditable1");
+        shouldBe("contenteditable.rangeForLine(0)", "'{0, 15}'");
+
+        var multilineContenteditable = accessibilityController.accessibleElementById("contenteditable2");
+        shouldBe("multilineContenteditable.rangeForLine(0)", "'{0, 16}'");
+        shouldBe("multilineContenteditable.rangeForLine(1)", "'{15, 1}'");
+
+        multilineContenteditable = accessibilityController.accessibleElementById("contenteditable3");
+        shouldBe("multilineContenteditable.rangeForLine(0)", "'{0, 16}'");
+        shouldBe("multilineContenteditable.rangeForLine(1)", "'{16, 6}'");
+        shouldBe("multilineContenteditable.rangeForLine(2)", "'{22, 5}'");
+
+        // textarea
+        var textarea = accessibilityController.accessibleElementById("textarea1");
+        shouldBe("textarea.rangeForLine(0)", "'{0, 15}'");
+
+        var multilineTextarea = accessibilityController.accessibleElementById("textarea2");
+        shouldBe("multilineTextarea.rangeForLine(0)", "'{0, 16}'");
+        shouldBe("multilineTextarea.rangeForLine(1)", "'{16, 6}'");
+        shouldBe("multilineTextarea.rangeForLine(2)", "'{22, 5}'");
+
+        // input text field
+        var textfield = accessibilityController.accessibleElementById("textfield");
+        shouldBe("textfield.rangeForLine(0)", "'{0, 21}'");
+    }
+</script>
+</body>
+</html>
diff --git a/LayoutTests/platform/mac/accessibility/content-editable-as-textarea-expected.txt b/LayoutTests/platform/mac/accessibility/content-editable-as-textarea-expected.txt
@@ -35,7 +35,7 @@ Attributed string with range: ello
 Line for index(0): 0
 Line for index(7): 1
 Range for line(0): {0, 6}
-Range for line(1): {6, 6}
+Range for line(1): {6, 5}
 Bounds for range: {{-1.000000, -1.000000}, {32.000000, 36.000000}}
 Selected text range: {0, 3}
 Selected text: hel

diff --git a/LayoutTests/platform/win/accessibility/content-editable-as-textarea-expected.txt b/LayoutTests/platform/win/accessibility/content-editable-as-textarea-expected.txt
@@ -34,7 +34,7 @@ Attributed string with range: ello
 Line for index(0): 0
 Line for index(7): 1
 Range for line(0): {0, 6}
-Range for line(1): {6, 6}
+Range for line(1): {6, 5}
 Bounds for range: {{-1.000000, -1.000000}, {31.000000, 36.000000}}
 Selected text range: {0, 3}
 Selected text: hel

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,28 @@
+2021-09-02  Andres Gonzalez  <andresg_22@apple.com>
+
+        Braille display is blank in contenteditable elements when the field is followed by another element.
+        https://bugs.webkit.org/show_bug.cgi?id=229713
+        rdar://82095237
+
+        Reviewed by Chris Fleizach.
+
+        Test: accessibility/mac/range-for-line-index.html
+
+        We were making the length of line ranges in text fields 1 more than the
+        number of characters in the line even when no line break character
+        existed, like in the case of a single line text field.
+        Clients like VoiceOver expect the length of the line ranges in text
+        fields to match the number of charaters in the line including the line
+        break if one exists.
+
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::isHardLineBreak): Helper function used in doAXRangeForLine.
+        Determines whether the given VisiblePosition corresponds to a hard line
+        break.
+        (WebCore::AccessibilityRenderObject::doAXRangeForLine const):
+        Returns a PlainTextRange whose length matches the number of characters
+        in the given line, accounting for line break characters.
+
 2021-09-02  Rob Buis  <rbuis@igalia.com>
 
         Absolutely positioned and negative z-index div with canvas child gets drawn with wrong stacking order

diff --git a/Source/WebCore/accessibility/AccessibilityRenderObject.cpp b/Source/WebCore/accessibility/AccessibilityRenderObject.cpp
@@ -51,6 +51,7 @@
 #include "GeometryUtilities.h"
 #include "HTMLAreaElement.h"
 #include "HTMLAudioElement.h"
+#include "HTMLBRElement.h"
 #include "HTMLDetailsElement.h"
 #include "HTMLFormElement.h"
 #include "HTMLFrameElementBase.h"
@@ -2443,38 +2444,59 @@ void AccessibilityRenderObject::lineBreaks(Vector<int>& lineBreaks) const
     }
 }
 
+static bool isHardLineBreak(const VisiblePosition& position)
+{
+    if (!isEndOfLine(position))
+        return false;
+
+    auto next = position.next();
+
+    auto lineBreakRange = makeSimpleRange(position, next);
+    if (!lineBreakRange)
+        return false;
+
+    TextIterator it(*lineBreakRange);
+    if (it.atEnd())
+        return false;
+
+    if (is<HTMLBRElement>(it.node()))
+        return true;
+
+    if (it.node() != position.deepEquivalent().anchorNode())
+        return false;
+
+    return it.text().length() == 1 && it.text()[0] == '\n';
+}
+
 // Given a line number, the range of characters of the text associated with this accessibility
 // object that contains the line number.
 PlainTextRange AccessibilityRenderObject::doAXRangeForLine(unsigned lineNumber) const
 {
     if (!isTextControl())
-        return PlainTextRange();
-
-    // iterate to the specified line
-    VisiblePosition visiblePos = visiblePositionForIndex(0);
-    VisiblePosition savedVisiblePos;
-    for (unsigned lineCount = lineNumber; lineCount; lineCount -= 1) {
-        savedVisiblePos = visiblePos;
-        visiblePos = nextLinePosition(visiblePos, 0);
-        if (visiblePos.isNull() || visiblePos == savedVisiblePos)
-            return PlainTextRange();
+        return { };
+
+    // Iterate to the specified line.
+    auto lineStart = visiblePositionForIndex(0);
+    for (unsigned lineCount = lineNumber; lineCount; --lineCount) {
+        auto nextLineStart = nextLinePosition(lineStart, 0);
+        if (nextLineStart.isNull() || nextLineStart == lineStart)
+            return { };
+        lineStart = nextLineStart;
     }
 
     // Get the end of the line based on the starting position.
-    VisiblePosition endPosition = endOfLine(visiblePos);
+    auto lineEnd = endOfLine(lineStart);
 
-    int index1 = indexForVisiblePosition(visiblePos);
-    int index2 = indexForVisiblePosition(endPosition);
-
-    // add one to the end index for a line break not caused by soft line wrap (to match AppKit)
-    if (endPosition.affinity() == Affinity::Downstream && endPosition.next().isNotNull())
-        index2 += 1;
-
-    // return nil rather than an zero-length range (to match AppKit)
-    if (index1 == index2)
-        return PlainTextRange();
-
-    return PlainTextRange(index1, index2 - index1);
+    int index1 = indexForVisiblePosition(lineStart);
+    int index2 = indexForVisiblePosition(lineEnd);
+
+    if (isHardLineBreak(lineEnd))
+        ++index2;
+
+    if (index1 < 0 || index2 < 0 || index2 <= index1)
+        return { };
+
+    return { static_cast<unsigned>(index1), static_cast<unsigned>(index2 - index1) };
 }
 
 // The composed character range in the text associated with this accessibility object that