[iOS] Moving backwards by word granularity does not work if the previous line was...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Apr 2019 22:57:02 +0000 (22:57 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 Apr 2019 22:57:02 +0000 (22:57 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196670

Reviewed by Wenson Hsieh.

Source/WebCore:

The bug was ultimately caused by two reasons:
 1. On iOS, previousWordPositionBoundary would identify a blank line as a word boundary.
 2. SimplifiedBackwardsTextIterator generates a new line character (\n) between two block elements.

When moving backwards by word granularity, therefore, previousBoundary would encounter a new line created by (2)
and then previousWordPositionBoundary would identify it as a word boundary.

Fixed the bug (2) by adding the same check as TextIterator::exitNode has to avoid generating an extra new line
character following an exiting new line character. Also added internals.rangeAsTextUsingBackwardsTextIterator
to make SimplifiedBackwardsTextIterator directly testable in layout tests.

This fix unveiled an unrelated bug when moving backwards with sentence granularity at the beginning of a line.
In this case, WebKit was previously feeding ICU with the previous line's content followed by two new lines,
which constituted a new sentence. However after the fix, ICU no longer detects a new sentence after the end
of the prevous line. This patch, therefore, introduces a new optional argument to previousBoundary which forces
the succeeding paragraph's content (i.e. the content of the line from which we're moving backwards with sentence
granularity) to be fed to ICU. This fixes the bug that we were previously not being able to move backwards
with sentence granularity at the beginning of a line as indicated by the new tests.

Tests: editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity.html
       editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity.html
       editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity.html
       editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity.html
       editing/text-iterator/backwards-text-iterator-basic.html

* accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::previousBoundary): Fixed the bug that moving backwards with sentence granularity at
the beginning of a line does not work like we did in VisibleUnits. See the description below. It's tested by
an existing layout test accessibility/mac/text-marker-sentence-nav.html, which would fail without this fix.
(WebCore::AXObjectCache::startCharacterOffsetOfSentence):
* accessibility/AXObjectCache.h:
(WebCore::CharacterOffset::isEqual const):
* editing/TextIterator.cpp:
(WebCore::SimplifiedBackwardsTextIterator::handleNonTextNode): Fixed the bug that we were generating two line
lines between block elements. This fixes the bug that moving backwards with word granularity at the beginning
of a line fails on iOS.
(WebCore::plainTextUsingBackwardsTextIteratorForTesting): Added.
* editing/TextIterator.h:
* editing/VisibleUnits.cpp:
(WebCore::previousBoundary): Added the code to extract the succeeding paragraph's content as context for ICU.
This fixes the bug that moving backwards with sentence granularity at the beginning of a line fails.
Limit the length of backwards iteration at the current position to avoid traversing backwards beyond
the current position, and fixed a bug that an early return for the text node was not taking the suffix length
into account when deciding whether next position resides in the starting container node or not.
(WebCore::startSentenceBoundary):
(WebCore::startOfSentence):
* testing/Internals.cpp:
(WebCore::Internals::rangeAsTextUsingBackwardsTextIterator): Added.
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

Added a test for moving and extending backwards from the beginning of a line with word & sentence granularities,
and a basic set of tests forSimplifiedBackwardsTextIterator.

* editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt: Added.
* editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity.html: Added.
* editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt: Added.
* editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity.html: Added.
* editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt: Added.
* editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity.html: Added.
* editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt: Added.
* editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity.html: Added.
* editing/text-iterator/backwards-text-iterator-basic-expected.txt: Added.
* editing/text-iterator/backwards-text-iterator-basic.html: Added.

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

20 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity.html [new file with mode: 0644]
LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity.html [new file with mode: 0644]
LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity.html [new file with mode: 0644]
LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity.html [new file with mode: 0644]
LayoutTests/editing/text-iterator/backwards-text-iterator-basic-expected.txt [new file with mode: 0644]
LayoutTests/editing/text-iterator/backwards-text-iterator-basic.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/accessibility/AXObjectCache.cpp
Source/WebCore/accessibility/AXObjectCache.h
Source/WebCore/editing/TextIterator.cpp
Source/WebCore/editing/TextIterator.h
Source/WebCore/editing/VisibleUnits.cpp
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index c5fa31e..962782f 100644 (file)
@@ -1,3 +1,24 @@
+2019-04-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] Moving backwards by word granularity does not work if the previous line was inside another block element
+        https://bugs.webkit.org/show_bug.cgi?id=196670
+
+        Reviewed by Wenson Hsieh.
+
+        Added a test for moving and extending backwards from the beginning of a line with word & sentence granularities,
+        and a basic set of tests forSimplifiedBackwardsTextIterator.
+
+        * editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt: Added.
+        * editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity.html: Added.
+        * editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt: Added.
+        * editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity.html: Added.
+        * editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt: Added.
+        * editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity.html: Added.
+        * editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt: Added.
+        * editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity.html: Added.
+        * editing/text-iterator/backwards-text-iterator-basic-expected.txt: Added.
+        * editing/text-iterator/backwards-text-iterator-basic.html: Added.
+
 2019-04-11  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Allow the MediaSource API to be enabled via website policy
diff --git a/LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt b/LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt
new file mode 100644 (file)
index 0000000..b2706c4
--- /dev/null
@@ -0,0 +1,13 @@
+This tests extending the selection backwards at the beginning of a line by sentence granularity. The first sentence should be selected.
+| "
+"
+| <div>
+|   "<#selection-focus>This is the first sentence"
+| "
+"
+| <div>
+|   id="target"
+|   <#selection-anchor>
+|   "Here is another one."
+| "
+"
diff --git a/LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity.html b/LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity.html
new file mode 100644 (file)
index 0000000..bab4228
--- /dev/null
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="editor" contenteditable>
+<div>This is the first sentence</div>
+<div id="target">Here is another one.</div>
+</div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description('This tests extending the selection backwards at the beginning of a line by sentence granularity. The first sentence should be selected.');
+
+editor.focus();
+getSelection().setPosition(target, 0);
+getSelection().modify('extend', 'backward', 'sentence');
+
+Markup.dump(editor);
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt b/LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt
new file mode 100644 (file)
index 0000000..13ca91d
--- /dev/null
@@ -0,0 +1,7 @@
+This tests moving the selection backwards at the beginning of a line by word granularity
+| <div>
+|   "<#selection-focus>first"
+| <div>
+|   id="target"
+|   <#selection-anchor>
+|   "second"
diff --git a/LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity.html b/LayoutTests/editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity.html
new file mode 100644 (file)
index 0000000..a5abd70
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="editor" contenteditable><div>first</div><div id="target">second</div></div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description('This tests moving the selection backwards at the beginning of a line by word granularity');
+
+editor.focus();
+getSelection().setPosition(target, 0);
+getSelection().modify('extend', 'backward', 'word');
+
+Markup.dump(editor);
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt b/LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity-expected.txt
new file mode 100644 (file)
index 0000000..d87025d
--- /dev/null
@@ -0,0 +1,11 @@
+This tests moving the selection backwards at the beginning of a line by sentence granularity. The caret should move to the first line.
+| "
+<#selection-caret>This is"
+| <br>
+| "
+"
+| <div>
+|   id="target"
+|   "one sentence."
+| "
+"
diff --git a/LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity.html b/LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity.html
new file mode 100644 (file)
index 0000000..0ef57e4
--- /dev/null
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="editor" contenteditable>
+This is<br>
+<div id="target">one sentence.</div>
+</div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description('This tests moving the selection backwards at the beginning of a line by sentence granularity. The caret should move to the first line.');
+
+editor.focus();
+getSelection().setPosition(target, 0);
+getSelection().modify('move', 'backward', 'sentence');
+
+Markup.dump(editor);
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt b/LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity-expected.txt
new file mode 100644 (file)
index 0000000..ba38590
--- /dev/null
@@ -0,0 +1,6 @@
+This tests moving the selection backwards at the beginning of a line by word granularity
+| <div>
+|   "<#selection-caret>first"
+| <div>
+|   id="target"
+|   "second"
diff --git a/LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity.html b/LayoutTests/editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity.html
new file mode 100644 (file)
index 0000000..35429d5
--- /dev/null
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="editor" contenteditable><div>first</div><div id="target">second</div></div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description('This tests moving the selection backwards at the beginning of a line by word granularity');
+
+editor.focus();
+getSelection().setPosition(target, 0);
+getSelection().modify('move', 'backward', 'word');
+
+Markup.dump(editor);
+
+</script>
+</body>
+</html>
diff --git a/LayoutTests/editing/text-iterator/backwards-text-iterator-basic-expected.txt b/LayoutTests/editing/text-iterator/backwards-text-iterator-basic-expected.txt
new file mode 100644 (file)
index 0000000..ad33a5f
--- /dev/null
@@ -0,0 +1,26 @@
+Unit tests for SimplifiedBackwardsTextIterator. It currently shows a bug that it generates an extra new line at the beginning.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS plainTextWithTextIterator("<div>hello</div>world") is "hello\nworld"
+PASS plainTextWithBackwardsTextIterator("<div>hello</div>world") is "\nhello\nworld"
+
+PASS plainTextWithTextIterator("<div>hello</div><div>world</div>") is "hello\nworld"
+PASS plainTextWithBackwardsTextIterator("<div>hello</div><div>world</div>") is "\nhello\nworld"
+
+PASS plainTextWithTextIterator("<div><b>h</b>ello <span>world</span></div>WebKit") is "hello world\nWebKit"
+PASS plainTextWithBackwardsTextIterator("<div><b>h</b>ello <span>world</span></div>WebKit") is "\nhello world\nWebKit"
+
+PASS plainTextWithTextIterator("hello<br>world<br>") is "hello\nworld\n"
+PASS plainTextWithBackwardsTextIterator("hello<br>world<br>") is "hello\nworld"
+
+PASS plainTextWithTextIterator("<ul><li>hello<li>world</ul>WebKit") is "hello\nworld\nWebKit"
+PASS plainTextWithBackwardsTextIterator("<ul><li>hello<li>world</ul>WebKit") is "\n\nhello\nworld\nWebKit"
+
+PASS plainTextWithTextIterator("<table><tr><td>hello</td><td>world</td></table>") is "hello    world"
+PASS plainTextWithBackwardsTextIterator("<table><tr><td>hello</td><td>world</td></table>") is "\n\nhello\nworld"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/text-iterator/backwards-text-iterator-basic.html b/LayoutTests/editing/text-iterator/backwards-text-iterator-basic.html
new file mode 100644 (file)
index 0000000..b80e30c
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="container"></div>
+<script src="../../resources/js-test.js"></script>
+<script>
+
+description('Unit tests for SimplifiedBackwardsTextIterator. It currently shows a bug that it generates an extra new line at the beginning.');
+
+function rangeForMarkup(markup) {
+    container.innerHTML = markup;
+    const range = new Range;
+    range.selectNodeContents(container);
+    return range;
+}
+
+function plainTextWithTextIterator(markup) {
+    return internals.rangeAsText(rangeForMarkup(markup));
+}
+
+function plainTextWithBackwardsTextIterator(markup) {
+    return internals.rangeAsTextUsingBackwardsTextIterator(rangeForMarkup(markup));
+}
+
+shouldBe('plainTextWithTextIterator("<div>hello</div>world")', '"hello\\nworld"');
+shouldBe('plainTextWithBackwardsTextIterator("<div>hello</div>world")', '"\\nhello\\nworld"');
+
+debug('');
+
+shouldBe('plainTextWithTextIterator("<div>hello</div><div>world</div>")', '"hello\\nworld"');
+shouldBe('plainTextWithBackwardsTextIterator("<div>hello</div><div>world</div>")', '"\\nhello\\nworld"');
+
+debug('');
+
+shouldBe('plainTextWithTextIterator("<div><b>h</b>ello <span>world</span></div>WebKit")', '"hello world\\nWebKit"');
+shouldBe('plainTextWithBackwardsTextIterator("<div><b>h</b>ello <span>world</span></div>WebKit")', '"\\nhello world\\nWebKit"');
+
+debug('');
+
+shouldBe('plainTextWithTextIterator("hello<br>world<br>")', '"hello\\nworld\\n"');
+shouldBe('plainTextWithBackwardsTextIterator("hello<br>world<br>")', '"hello\\nworld"');
+
+debug('');
+
+shouldBe('plainTextWithTextIterator("<ul><li>hello<li>world</ul>WebKit")', '"hello\\nworld\\nWebKit"');
+shouldBe('plainTextWithBackwardsTextIterator("<ul><li>hello<li>world</ul>WebKit")', '"\\n\\nhello\\nworld\\nWebKit"');
+
+debug('');
+
+shouldBe('plainTextWithTextIterator("<table><tr><td>hello</td><td>world</td></table>")', '"hello\tworld"');
+shouldBe('plainTextWithBackwardsTextIterator("<table><tr><td>hello</td><td>world</td></table>")', '"\\n\\nhello\\nworld"');
+
+container.innerHTML = '';
+
+successfullyParsed = true;
+</script>
+</body>
+</html>
index fd5c8f9..f67d0d2 100644 (file)
@@ -1,3 +1,61 @@
+2019-04-09  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] Moving backwards by word granularity does not work if the previous line was inside another block element
+        https://bugs.webkit.org/show_bug.cgi?id=196670
+
+        Reviewed by Wenson Hsieh.
+
+        The bug was ultimately caused by two reasons:
+         1. On iOS, previousWordPositionBoundary would identify a blank line as a word boundary.
+         2. SimplifiedBackwardsTextIterator generates a new line character (\n) between two block elements.
+
+        When moving backwards by word granularity, therefore, previousBoundary would encounter a new line created by (2)
+        and then previousWordPositionBoundary would identify it as a word boundary.
+
+        Fixed the bug (2) by adding the same check as TextIterator::exitNode has to avoid generating an extra new line
+        character following an exiting new line character. Also added internals.rangeAsTextUsingBackwardsTextIterator
+        to make SimplifiedBackwardsTextIterator directly testable in layout tests.
+
+        This fix unveiled an unrelated bug when moving backwards with sentence granularity at the beginning of a line.
+        In this case, WebKit was previously feeding ICU with the previous line's content followed by two new lines,
+        which constituted a new sentence. However after the fix, ICU no longer detects a new sentence after the end
+        of the prevous line. This patch, therefore, introduces a new optional argument to previousBoundary which forces
+        the succeeding paragraph's content (i.e. the content of the line from which we're moving backwards with sentence
+        granularity) to be fed to ICU. This fixes the bug that we were previously not being able to move backwards
+        with sentence granularity at the beginning of a line as indicated by the new tests.
+
+        Tests: editing/selection/extend-selection-backward-at-beginning-of-line-by-sentence-granularity.html
+               editing/selection/extend-selection-backward-at-beginning-of-line-by-word-granularity.html
+               editing/selection/move-selection-backward-at-beginning-of-line-by-sentence-granularity.html
+               editing/selection/move-selection-backward-at-beginning-of-line-by-word-granularity.html
+               editing/text-iterator/backwards-text-iterator-basic.html
+
+        * accessibility/AXObjectCache.cpp:
+        (WebCore::AXObjectCache::previousBoundary): Fixed the bug that moving backwards with sentence granularity at
+        the beginning of a line does not work like we did in VisibleUnits. See the description below. It's tested by
+        an existing layout test accessibility/mac/text-marker-sentence-nav.html, which would fail without this fix.
+        (WebCore::AXObjectCache::startCharacterOffsetOfSentence):
+        * accessibility/AXObjectCache.h:
+        (WebCore::CharacterOffset::isEqual const):
+        * editing/TextIterator.cpp:
+        (WebCore::SimplifiedBackwardsTextIterator::handleNonTextNode): Fixed the bug that we were generating two line
+        lines between block elements. This fixes the bug that moving backwards with word granularity at the beginning
+        of a line fails on iOS.
+        (WebCore::plainTextUsingBackwardsTextIteratorForTesting): Added.
+        * editing/TextIterator.h:
+        * editing/VisibleUnits.cpp:
+        (WebCore::previousBoundary): Added the code to extract the succeeding paragraph's content as context for ICU.
+        This fixes the bug that moving backwards with sentence granularity at the beginning of a line fails.
+        Limit the length of backwards iteration at the current position to avoid traversing backwards beyond
+        the current position, and fixed a bug that an early return for the text node was not taking the suffix length
+        into account when deciding whether next position resides in the starting container node or not.
+        (WebCore::startSentenceBoundary):
+        (WebCore::startOfSentence):
+        * testing/Internals.cpp:
+        (WebCore::Internals::rangeAsTextUsingBackwardsTextIterator): Added.
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2019-04-11  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Allow the MediaSource API to be enabled via website policy
index 196bc28..b809586 100644 (file)
@@ -2473,7 +2473,8 @@ CharacterOffset AXObjectCache::nextBoundary(const CharacterOffset& characterOffs
     return characterOffset;
 }
 
-CharacterOffset AXObjectCache::previousBoundary(const CharacterOffset& characterOffset, BoundarySearchFunction searchFunction)
+// FIXME: Share code with the one in VisibleUnits.cpp.
+CharacterOffset AXObjectCache::previousBoundary(const CharacterOffset& characterOffset, BoundarySearchFunction searchFunction, NeedsContextAtParagraphStart needsContextAtParagraphStart)
 {
     if (characterOffset.isNull())
         return CharacterOffset();
@@ -2485,8 +2486,18 @@ CharacterOffset AXObjectCache::previousBoundary(const CharacterOffset& character
     RefPtr<Range> searchRange = rangeForNodeContents(boundary);
     Vector<UChar, 1024> string;
     unsigned suffixLength = 0;
-    
-    if (requiresContextForWordBoundary(characterBefore(characterOffset))) {
+
+    if (needsContextAtParagraphStart == NeedsContextAtParagraphStart::Yes && startCharacterOffsetOfParagraph(characterOffset).isEqual(characterOffset)) {
+        auto forwardsScanRange = boundary->document().createRange();
+        auto endOfCurrentParagraph = endCharacterOffsetOfParagraph(characterOffset);
+        if (!setRangeStartOrEndWithCharacterOffset(forwardsScanRange, characterOffset, true))
+            return { };
+        if (!setRangeStartOrEndWithCharacterOffset(forwardsScanRange, endOfCurrentParagraph, false))
+            return { };
+        for (TextIterator forwardsIterator(forwardsScanRange.ptr()); !forwardsIterator.atEnd(); forwardsIterator.advance())
+            append(string, forwardsIterator.text());
+        suffixLength = string.size();
+    } else if (requiresContextForWordBoundary(characterBefore(characterOffset))) {
         auto forwardsScanRange = boundary->document().createRange();
         if (forwardsScanRange->setEndAfter(*boundary).hasException())
             return { };
@@ -2513,15 +2524,17 @@ CharacterOffset AXObjectCache::previousBoundary(const CharacterOffset& character
     Node* nextSibling = node.nextSibling();
     if (&node != characterOffset.node && AccessibilityObject::replacedNodeNeedsCharacter(nextSibling))
         return startOrEndCharacterOffsetForRange(rangeForNodeContents(nextSibling), false);
-    
-    if ((node.isTextNode() && static_cast<int>(next) <= node.maxCharacterOffset()) || (node.renderer() && node.renderer()->isBR() && !next)) {
+
+    if ((!suffixLength && node.isTextNode() && static_cast<int>(next) <= node.maxCharacterOffset()) || (node.renderer() && node.renderer()->isBR() && !next)) {
         // The next variable contains a usable index into a text node
         if (node.isTextNode())
             return traverseToOffsetInRange(rangeForNodeContents(&node), next, TraverseOptionValidateOffset);
         return characterOffsetForNodeAndOffset(node, next, TraverseOptionIncludeStart);
     }
     
-    int characterCount = characterOffset.offset - (string.size() - suffixLength - next);
+    int characterCount = characterOffset.offset;
+    if (next < string.size() - suffixLength)
+        characterCount -= string.size() - suffixLength - next;
     // We don't want to go to the previous node if the node is at the start of a new line.
     if (characterCount < 0 && (characterOffsetNodeIsBR(characterOffset) || string[string.size() - suffixLength - 1] == '\n'))
         characterCount = 0;
@@ -2611,7 +2624,7 @@ CharacterOffset AXObjectCache::previousParagraphStartCharacterOffset(const Chara
 
 CharacterOffset AXObjectCache::startCharacterOffsetOfSentence(const CharacterOffset& characterOffset)
 {
-    return previousBoundary(characterOffset, startSentenceBoundary);
+    return previousBoundary(characterOffset, startSentenceBoundary, NeedsContextAtParagraphStart::Yes);
 }
 
 CharacterOffset AXObjectCache::endCharacterOffsetOfSentence(const CharacterOffset& characterOffset)
index 6472114..878478d 100644 (file)
@@ -86,7 +86,7 @@ struct CharacterOffset {
     
     int remaining() const { return remainingOffset; }
     bool isNull() const { return !node; }
-    bool isEqual(CharacterOffset& other) const
+    bool isEqual(const CharacterOffset& other) const
     {
         if (isNull() || other.isNull())
             return false;
@@ -391,7 +391,9 @@ protected:
     UChar32 characterAfter(const CharacterOffset&);
     UChar32 characterBefore(const CharacterOffset&);
     CharacterOffset characterOffsetForNodeAndOffset(Node&, int, TraverseOption = TraverseOptionDefault);
-    CharacterOffset previousBoundary(const CharacterOffset&, BoundarySearchFunction);
+
+    enum class NeedsContextAtParagraphStart { Yes, No };
+    CharacterOffset previousBoundary(const CharacterOffset&, BoundarySearchFunction, NeedsContextAtParagraphStart = NeedsContextAtParagraphStart::No);
     CharacterOffset nextBoundary(const CharacterOffset&, BoundarySearchFunction);
     CharacterOffset startCharacterOffsetOfWord(const CharacterOffset&, EWordSide = RightWordIfOnBoundary);
     CharacterOffset endCharacterOffsetOfWord(const CharacterOffset&, EWordSide = RightWordIfOnBoundary);
index 7e7a1f4..23414a2 100644 (file)
@@ -1479,10 +1479,13 @@ bool SimplifiedBackwardsTextIterator::handleNonTextNode()
     // We can use a linefeed in place of a tab because this simple iterator is only used to
     // find boundaries, not actual content. A linefeed breaks words, sentences, and paragraphs.
     if (shouldEmitNewlineForNode(m_node, m_behavior & TextIteratorEmitsOriginalText) || shouldEmitNewlineAfterNode(*m_node) || shouldEmitTabBeforeNode(*m_node)) {
-        unsigned index = m_node->computeNodeIndex();
-        // The start of this emitted range is wrong. Ensuring correctness would require
-        // VisiblePositions and so would be slow. previousBoundary expects this.
-        emitCharacter('\n', *m_node->parentNode(), index + 1, index + 1);
+        if (m_lastCharacter != '\n') {
+            // Corresponds to the same check in TextIterator::exitNode.
+            unsigned index = m_node->computeNodeIndex();
+            // The start of this emitted range is wrong. Ensuring correctness would require
+            // VisiblePositions and so would be slow. previousBoundary expects this.
+            emitCharacter('\n', *m_node->parentNode(), index + 1, index + 1);
+        }
     }
     return true;
 }
@@ -2700,6 +2703,14 @@ String plainText(const Range* range, TextIteratorBehavior defaultBehavior, bool
     return plainText(range->startPosition(), range->endPosition(), defaultBehavior, isDisplayString);
 }
 
+String plainTextUsingBackwardsTextIteratorForTesting(const Range& range)
+{
+    String result;
+    for (SimplifiedBackwardsTextIterator backwardsIterator(range); !backwardsIterator.atEnd(); backwardsIterator.advance())
+        result.insert(backwardsIterator.text().toString(), 0);
+    return result;
+}
+
 String plainTextReplacingNoBreakSpace(const Range* range, TextIteratorBehavior defaultBehavior, bool isDisplayString)
 {
     return plainText(range, defaultBehavior, isDisplayString).replace(noBreakSpace, ' ');
index 5917a30..08b1fb2 100644 (file)
@@ -48,6 +48,8 @@ WEBCORE_EXPORT String plainTextReplacingNoBreakSpace(Position start, Position en
 
 WEBCORE_EXPORT String plainText(const Range*, TextIteratorBehavior = TextIteratorDefaultBehavior, bool isDisplayString = false);
 WEBCORE_EXPORT String plainTextReplacingNoBreakSpace(const Range*, TextIteratorBehavior = TextIteratorDefaultBehavior, bool isDisplayString = false);
+WEBCORE_EXPORT String plainTextUsingBackwardsTextIteratorForTesting(const Range&);
+
 Ref<Range> findPlainText(const Range&, const String&, FindOptions);
 WEBCORE_EXPORT Ref<Range> findClosestPlainText(const Range&, const String&, FindOptions, unsigned);
 WEBCORE_EXPORT bool hasAnyPlainText(const Range&, TextIteratorBehavior = TextIteratorDefaultBehavior);
index 75feb55..1be227e 100644 (file)
@@ -578,7 +578,9 @@ unsigned forwardSearchForBoundaryWithTextIterator(TextIterator& it, Vector<UChar
     return next;
 }
 
-static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearchFunction searchFunction)
+enum class NeedsContextAtParagraphStart { Yes, No };
+static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearchFunction searchFunction,
+    NeedsContextAtParagraphStart needsContextAtParagraphStart = NeedsContextAtParagraphStart::No)
 {
     Position pos = c.deepEquivalent();
     Node* boundary = pos.parentEditingBoundary();
@@ -597,7 +599,19 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch
     Vector<UChar, 1024> string;
     unsigned suffixLength = 0;
 
-    if (requiresContextForWordBoundary(c.characterBefore())) {
+    if (needsContextAtParagraphStart == NeedsContextAtParagraphStart::Yes && isStartOfParagraph(c)) {
+        auto forwardsScanRange = boundaryDocument.createRange();
+        auto endOfCurrentParagraph = endOfParagraph(c);
+        auto result = forwardsScanRange->setEnd(endOfCurrentParagraph.deepEquivalent());
+        if (result.hasException())
+            return { };
+        result = forwardsScanRange->setStart(start);
+        if (result.hasException())
+            return { };
+        for (TextIterator forwardsIterator(forwardsScanRange.ptr()); !forwardsIterator.atEnd(); forwardsIterator.advance())
+            append(string, forwardsIterator.text());
+        suffixLength = string.size();
+    } else if (requiresContextForWordBoundary(c.characterBefore())) {
         auto forwardsScanRange = boundaryDocument.createRange();
         auto result = forwardsScanRange->setEndAfter(*boundary);
         if (result.hasException())
@@ -622,14 +636,15 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch
         return VisiblePosition(it.atEnd() ? searchRange->startPosition() : pos, DOWNSTREAM);
 
     Node& node = it.atEnd() ? searchRange->startContainer() : it.range()->startContainer();
-    if ((node.isTextNode() && static_cast<int>(next) <= node.maxCharacterOffset()) || (node.renderer() && node.renderer()->isBR() && !next)) {
+    if ((!suffixLength && node.isTextNode() && static_cast<int>(next) <= node.maxCharacterOffset()) || (node.renderer() && node.renderer()->isBR() && !next)) {
         // The next variable contains a usable index into a text node
         return VisiblePosition(createLegacyEditingPosition(&node, next), DOWNSTREAM);
     }
 
     // Use the character iterator to translate the next value into a DOM position.
     BackwardsCharacterIterator charIt(searchRange);
-    charIt.advance(string.size() - suffixLength - next);
+    if (next < string.size() - suffixLength)
+        charIt.advance(string.size() - suffixLength - next);
     // FIXME: charIt can get out of shadow host.
     return VisiblePosition(charIt.range()->endPosition(), DOWNSTREAM);
 }
@@ -1132,7 +1147,7 @@ unsigned startSentenceBoundary(StringView text, unsigned, BoundarySearchContextA
 
 VisiblePosition startOfSentence(const VisiblePosition& position)
 {
-    return previousBoundary(position, startSentenceBoundary);
+    return previousBoundary(position, startSentenceBoundary, NeedsContextAtParagraphStart::Yes);
 }
 
 unsigned endSentenceBoundary(StringView text, unsigned, BoundarySearchContextAvailability, bool&)
index 4da430d..33c7520 100644 (file)
@@ -1925,6 +1925,11 @@ String Internals::rangeAsText(const Range& range)
     return range.text();
 }
 
+String Internals::rangeAsTextUsingBackwardsTextIterator(const Range& range)
+{
+    return plainTextUsingBackwardsTextIteratorForTesting(range);
+}
+
 Ref<Range> Internals::subrange(Range& range, int rangeLocation, int rangeLength)
 {
     return TextIterator::subrange(range, rangeLocation, rangeLength);
index 56ce6f1..cd2aed9 100644 (file)
@@ -274,6 +274,7 @@ public:
     unsigned locationFromRange(Element& scope, const Range&);
     unsigned lengthFromRange(Element& scope, const Range&);
     String rangeAsText(const Range&);
+    String rangeAsTextUsingBackwardsTextIterator(const Range&);
     Ref<Range> subrange(Range&, int rangeLocation, int rangeLength);
     ExceptionOr<RefPtr<Range>> rangeForDictionaryLookupAtLocation(int x, int y);
     RefPtr<Range> rangeOfStringNearLocation(const Range&, const String&, unsigned);
index fe878f5..4e44430 100644 (file)
@@ -293,6 +293,7 @@ enum CompositingPolicy {
     unsigned long locationFromRange(Element scope, Range range);
     unsigned long lengthFromRange(Element scope, Range range);
     DOMString rangeAsText(Range range);
+    DOMString rangeAsTextUsingBackwardsTextIterator(Range range);
     Range subrange(Range range, long rangeLocation, long rangeLength);
     [MayThrowException] Range? rangeForDictionaryLookupAtLocation(long x, long y);
     Range? rangeOfStringNearLocation(Range range, DOMString text, long targetOffset);