Changeset 281920 in webkit

Timestamp:

Sep 2, 2021 5:45:25 AM (11 months ago)

Author:

Andres Gonzalez

Message:

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.

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/accessibility/mac/range-for-line-index-expected.txt (added)
• LayoutTests/accessibility/mac/range-for-line-index.html (added)
• LayoutTests/platform/mac/accessibility/content-editable-as-textarea-expected.txt (modified) (1 diff)
• LayoutTests/platform/win/accessibility/content-editable-as-textarea-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/accessibility/AccessibilityRenderObject.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/platform/mac/accessibility/content-editable-as-textarea-expected.txt

@@ -36 +36 @@
 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}

trunk/LayoutTests/platform/win/accessibility/content-editable-as-textarea-expected.txt

@@ -35 +35 @@
 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}

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp

@@ -52 +52 @@
 #include "HTMLAreaElement.h"
 #include "HTMLAudioElement.h"
+#include "HTMLBRElement.h"
 #include "HTMLDetailsElement.h"
 #include "HTMLFormElement.h"
@@ -2444 +2445 @@
 }
 
+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.
@@ -2449 +2474 @@
 {
     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);
-
-    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);
+    auto lineEnd = endOfLine(lineStart);
+
+    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) };
 }