Moving word boundaries backwards fails when there is a text node starting with an...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Apr 2013 19:50:50 +0000 (19:50 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Apr 2013 19:50:50 +0000 (19:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=115070

Reviewed by Alexey Proskuryakov.

Source/WebCore:

The bug was caused by previousBoundary erroneously assuming that we don't need any more context if a word
boundary is found at the beginning of a string. For example, when "I'll" is split into two text nodes,
"I" and "'ll", there is a word boundary between "'" and "ll" in "'ll" so we need to examine the whole "I'll".

Fixed the bug by obtaining more context when the character starts exactly at offset 1 in a text node to
work around this bug. In the long term, we probably need to provide Foundation of the entire context since in
languages like Hebrew and some of European languages, there could be many accents and combining characters
between split into multiple text nodes as one variant is seen in the newly added test case.

Test: editing/selection/previous-word-boundary-across-text-nodes.html

* editing/VisibleUnits.cpp:
(WebCore::previousBoundary):

LayoutTests:

Added a test case for moving bacwkards with a word granurality across multiple text nodes.
Some test cases still fail since this patch only implements a work around.

* editing/selection/previous-word-boundary-across-text-nodes-expected.txt: Added.
* editing/selection/previous-word-boundary-across-text-nodes.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/previous-word-boundary-across-text-nodes-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/previous-word-boundary-across-text-nodes.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/VisibleUnits.cpp

index 7fa52b22d243003e45047100f673c186104461f1..01a54ab6b3b4a97ac5c7079667c38ba569914e4f 100644 (file)
@@ -1,3 +1,16 @@
+2013-04-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Moving word boundaries backwards fails when there is a text node starting with an apostrophe
+        https://bugs.webkit.org/show_bug.cgi?id=115070
+
+        Reviewed by Alexey Proskuryakov.
+
+        Added a test case for moving bacwkards with a word granurality across multiple text nodes.
+        Some test cases still fail since this patch only implements a work around.
+
+        * editing/selection/previous-word-boundary-across-text-nodes-expected.txt: Added.
+        * editing/selection/previous-word-boundary-across-text-nodes.html: Added.
+
 2013-04-24  Chris Fleizach  <cfleizach@apple.com>
 
         AX: WAI-ARIA landmarks no longer speak type of landmark on iOS
diff --git a/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes-expected.txt b/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes-expected.txt
new file mode 100644 (file)
index 0000000..ffc30f7
--- /dev/null
@@ -0,0 +1,22 @@
+PASS selectWordBackward(container); /* <span>I</span>'ll */ getSelection().toString(); is "I'll"
+PASS selectWordBackward(container); /* <span>I'</span>ll */ getSelection().toString(); is "I'll"
+PASS selectWordBackward(container); /* <span>I'l</span>l */ getSelection().toString(); is "I'll"
+PASS selectWordBackward(container); /* I'<span>ll</span> */ getSelection().toString(); is "I'll"
+PASS selectWordBackward(container); /* I<span>'l</span>l */ getSelection().toString(); is "I'll"
+FAIL selectWordBackward(container); /* <span>e</span>́'ll */ getSelection().toString(); should be é'll. Was ll.
+PASS selectWordBackward(container); /* <span>é</span>'ll */ getSelection().toString(); is "é'll"
+PASS selectWordBackward(container); /* <span>é'</span>ll */ getSelection().toString(); is "é'll"
+PASS selectWordBackward(container); /* <span>é'l</span>l */ getSelection().toString(); is "é'll"
+FAIL selectWordBackward(container); /* e<span>́</span>'ll */ getSelection().toString(); should be é'll. Was ll.
+FAIL selectWordBackward(container); /* e<span>́'</span>ll */ getSelection().toString(); should be é'll. Was ll.
+FAIL selectWordBackward(container); /* e<span>́'l</span>l */ getSelection().toString(); should be é'll. Was ll.
+FAIL selectWordBackward(container); /* e<span>́'ll</span> */ getSelection().toString(); should be é'll. Was ll.
+PASS selectWordBackward(container); /* é<span>'</span>ll */ getSelection().toString(); is "é'll"
+PASS selectWordBackward(container); /* é<span>'l</span>l */ getSelection().toString(); is "é'll"
+PASS selectWordBackward(container); /* é<span>'ll</span> */ getSelection().toString(); is "é'll"
+PASS selectWordBackward(container); /* é<span>'</span>ll */ getSelection().toString(); is "é'll"
+PASS selectWordBackward(container); /* é<span>'l</span>l */ getSelection().toString(); is "é'll"
+PASS selectWordBackward(container); /* é<span>'ll</span> */ getSelection().toString(); is "é'll"
+PASS selectWordBackward(container); /* é'<span>l</span>l */ getSelection().toString(); is "é'll"
+This test checks moving to the previous word boundary across multiple text nodes.
+For example, when "I" and "'ll" are put in a separate text node, we should not erroneously report the previous word boundary to be between "'" and "ll".
diff --git a/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes.html b/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes.html
new file mode 100644 (file)
index 0000000..1a458c3
--- /dev/null
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This test checks moving to the previous word boundary across multiple text nodes.<br>
+For example, when "I" and "'ll" are put in a separate text node, we should not erroneously report the previous word boundary to be between "'" and "ll".</p>
+<div id="tests" style="font-size: 20px;" >
+<div contenteditable><span>I</span>'ll</div>
+<div contenteditable><span>I'</span>ll</div>
+<div contenteditable><span>I'l</span>l</div>
+<div contenteditable>I'<span>ll</span></div>
+<div contenteditable>I<span>'l</span>l</div>
+<div contenteditable><span>e</span>&#x301;'ll</div>
+<div contenteditable><span>e&#x301;</span>'ll</div>
+<div contenteditable><span>e&#x301;'</span>ll</div>
+<div contenteditable><span>e&#x301;'l</span>l</div>
+<div contenteditable>e<span>&#x301;</span>'ll</div>
+<div contenteditable>e<span>&#x301;'</span>ll</div>
+<div contenteditable>e<span>&#x301;'l</span>l</div>
+<div contenteditable>e<span>&#x301;'ll</span></div>
+<div contenteditable>e&#x301;<span>'</span>ll</div>
+<div contenteditable>e&#x301;<span>'l</span>l</div>
+<div contenteditable>e&#x301;<span>'ll</span></div>
+<div contenteditable>e&#x301;<span>'</span>ll</div>
+<div contenteditable>e&#x301;<span>'l</span>l</div>
+<div contenteditable>e&#x301;<span>'ll</span></div>
+<div contenteditable>e&#x301;'<span>l</span>l</div>
+</div>
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+<script>
+
+function selectWordBackward(container) {
+    getSelection().collapse(container, container.childNodes.length);
+    getSelection().modify('extend', 'backward', 'word');
+}
+
+var tests = document.getElementById('tests').children;
+for (var i = 0; i < tests.length; i++) {
+    var container = tests[i];
+    shouldBeEqualToString("selectWordBackward(container); /* " + container.innerHTML + " */ getSelection().toString();", container.textContent);
+}
+document.getElementById('tests').style.display = 'none';
+
+</script>
+</body>
+</html>
index cc536c99218c1f791c9a248fbe1421d76f2f8c3f..d5934df4afd119bfe33d86267951070711e0715a 100644 (file)
@@ -1,3 +1,24 @@
+2013-04-23  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Moving word boundaries backwards fails when there is a text node starting with an apostrophe
+        https://bugs.webkit.org/show_bug.cgi?id=115070
+
+        Reviewed by Alexey Proskuryakov.
+
+        The bug was caused by previousBoundary erroneously assuming that we don't need any more context if a word
+        boundary is found at the beginning of a string. For example, when "I'll" is split into two text nodes,
+        "I" and "'ll", there is a word boundary between "'" and "ll" in "'ll" so we need to examine the whole "I'll".
+
+        Fixed the bug by obtaining more context when the character starts exactly at offset 1 in a text node to
+        work around this bug. In the long term, we probably need to provide Foundation of the entire context since in
+        languages like Hebrew and some of European languages, there could be many accents and combining characters
+        between split into multiple text nodes as one variant is seen in the newly added test case.
+
+        Test: editing/selection/previous-word-boundary-across-text-nodes.html
+
+        * editing/VisibleUnits.cpp:
+        (WebCore::previousBoundary):
+
 2013-04-24  Benjamin Poulain  <bpoulain@apple.com>
 
         Do not use static string in DiagnosticLoggingKeys
index 17c2b335368898d713e4f2324da9d6bb3dd7de93..c27cf451781234f35e66fab961695496df3704a5 100644 (file)
@@ -499,7 +499,7 @@ static VisiblePosition previousBoundary(const VisiblePosition& c, BoundarySearch
             string.prepend(iteratorString.characters(), iteratorString.length());
         }
         next = searchFunction(string.data(), string.size(), string.size() - suffixLength, MayHaveMoreContext, needMoreContext);
-        if (next)
+        if (next > 1) // FIXME: This is a work around for https://webkit.org/b/115070. We need to provide more contexts in general case.
             break;
         it.advance();
     }