AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Sep 2019 07:06:09 +0000 (07:06 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Sep 2019 07:06:09 +0000 (07:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201518
<rdar://problem/54835122>

Patch by Andres Gonzalez <andresg_22@apple.com> on 2019-09-06
Reviewed by Ryosuke Niwa.

Source/WebCore:

Test: accessibility/set-selected-text-range-after-newline.html

In the case of an empty line, the CharacterIterator range start and end
were not equal, thus we were not advancing the iterator and returning
the iterator range end, which is not correct. With this change we are
always advancing the iterator if its text is just '\n'. This covers all
the cases we fixed before plus empty lines.

* editing/Editing.cpp:
(WebCore::visiblePositionForIndexUsingCharacterIterator):

LayoutTests:

Extended this test to set the selection range passed an empty line.
* accessibility/set-selected-text-range-after-newline-expected.txt:
* accessibility/set-selected-text-range-after-newline.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249565 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/accessibility/set-selected-text-range-after-newline-expected.txt
LayoutTests/accessibility/set-selected-text-range-after-newline.html
Source/WebCore/ChangeLog
Source/WebCore/editing/Editing.cpp

index b607f1a..3bd7813 100644 (file)
@@ -1,3 +1,15 @@
+2019-09-06  Andres Gonzalez  <andresg_22@apple.com>
+
+        AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
+        https://bugs.webkit.org/show_bug.cgi?id=201518
+        <rdar://problem/54835122>
+
+        Reviewed by Ryosuke Niwa.
+
+        Extended this test to set the selection range passed an empty line.
+        * accessibility/set-selected-text-range-after-newline-expected.txt:
+        * accessibility/set-selected-text-range-after-newline.html:
+
 2019-09-05  Chris Dumez  <cdumez@apple.com>
 
         Stop using testRunner.setPrivateBrowsingEnabled_DEPRECATED() in js-test.js
index 10d4527..130f4b4 100644 (file)
@@ -1,8 +1,11 @@
 hello
+
 world
 PASS text.selectedTextRange became '{5, 0}'
 There must be only one [newline] between hello and world: hello[newline]world
 PASS text.selectedTextRange became '{6, 0}'
+There must be two [newline] between hello and world: hello[newline][newline]world
+PASS text.selectedTextRange became '{7, 0}'
 The text after the newline should be world: world
 PASS successfullyParsed is true
 
index 4ffc2e3..bdb42cb 100644 (file)
@@ -18,6 +18,7 @@
         var text = accessibilityController.focusedElement;
         text.setSelectedTextRange(5, 0);
         shouldBecomeEqual("text.selectedTextRange", "'{5, 0}'", function() {
+            // Insert a linebreak between "hello" and "world".
             text.replaceTextInRange("\n", 5, 0);
 
             var t = text.stringForRange(0, 11);
 
             text.setSelectedTextRange(6, 0);
             shouldBecomeEqual("text.selectedTextRange", "'{6, 0}'", function() {
-                var t = text.stringForRange(6, 5);
+                // Insert another linebreak before "world".
+                text.replaceTextInRange("\n", 6, 0);
+
+                var t = text.stringForRange(0, 12);
                 t = t.replace(/(?:\r\n|\r|\n)/g, '[newline]');
-                debug("The text after the newline should be world: " + t);
+                debug("There must be two [newline] between hello and world: " + t);
+
+                text.setSelectedTextRange(7, 0);
+                shouldBecomeEqual("text.selectedTextRange", "'{7, 0}'", function() {
+                    var t = text.stringForRange(7, 5);
+                    t = t.replace(/(?:\r\n|\r|\n)/g, '[newline]');
+                    debug("The text after the newline should be world: " + t);
 
-                finishJSTest();
+                    finishJSTest();
+                });
             });
         });
     }
index da65335..5231120 100644 (file)
@@ -1,3 +1,22 @@
+2019-09-06  Andres Gonzalez  <andresg_22@apple.com>
+
+        AccessibilityRenderObject::setSelectedTextRange fails to set the selection passed an empty line.
+        https://bugs.webkit.org/show_bug.cgi?id=201518
+        <rdar://problem/54835122>
+
+        Reviewed by Ryosuke Niwa.
+
+        Test: accessibility/set-selected-text-range-after-newline.html
+
+        In the case of an empty line, the CharacterIterator range start and end
+        were not equal, thus we were not advancing the iterator and returning
+        the iterator range end, which is not correct. With this change we are
+        always advancing the iterator if its text is just '\n'. This covers all
+        the cases we fixed before plus empty lines.
+
+        * editing/Editing.cpp:
+        (WebCore::visiblePositionForIndexUsingCharacterIterator):
+
 2019-09-05  Fujii Hironori  <Hironori.Fujii@sony.com>
 
         [Win] Add support for MouseEvent.buttons
index 69999c9..ff2df82 100644 (file)
@@ -1122,14 +1122,11 @@ VisiblePosition visiblePositionForIndexUsingCharacterIterator(Node& node, int in
     CharacterIterator it(range.get());
     it.advance(index - 1);
 
-    if (!it.atEnd() && it.text()[0] == '\n') {
+    if (!it.atEnd() && it.text().length() == 1 && it.text()[0] == '\n') {
         // FIXME: workaround for collapsed range (where only start position is correct) emitted for some emitted newlines.
-        auto iteratorRange = it.range();
-        if (iteratorRange->startPosition() == iteratorRange->endPosition()) {
-            it.advance(1);
-            if (!it.atEnd())
-                return VisiblePosition(it.range()->startPosition());
-        }
+        it.advance(1);
+        if (!it.atEnd())
+            return VisiblePosition(it.range()->startPosition());
     }
 
     return { it.atEnd() ? range->endPosition() : it.range()->endPosition(), UPSTREAM };