Title: [149058] trunk
- Revision
- 149058
- Author
- [email protected]
- Date
- 2013-04-24 12:50:50 -0700 (Wed, 24 Apr 2013)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (149057 => 149058)
--- trunk/LayoutTests/ChangeLog 2013-04-24 19:46:44 UTC (rev 149057)
+++ trunk/LayoutTests/ChangeLog 2013-04-24 19:50:50 UTC (rev 149058)
@@ -1,3 +1,16 @@
+2013-04-23 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
AX: WAI-ARIA landmarks no longer speak type of landmark on iOS
Added: trunk/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes-expected.txt (0 => 149058)
--- trunk/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes-expected.txt 2013-04-24 19:50:50 UTC (rev 149058)
@@ -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".
Added: trunk/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes.html (0 => 149058)
--- trunk/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes.html (rev 0)
+++ trunk/LayoutTests/editing/selection/previous-word-boundary-across-text-nodes.html 2013-04-24 19:50:50 UTC (rev 149058)
@@ -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>́'ll</div>
+<div contenteditable><span>é</span>'ll</div>
+<div contenteditable><span>é'</span>ll</div>
+<div contenteditable><span>é'l</span>l</div>
+<div contenteditable>e<span>́</span>'ll</div>
+<div contenteditable>e<span>́'</span>ll</div>
+<div contenteditable>e<span>́'l</span>l</div>
+<div contenteditable>e<span>́'ll</span></div>
+<div contenteditable>é<span>'</span>ll</div>
+<div contenteditable>é<span>'l</span>l</div>
+<div contenteditable>é<span>'ll</span></div>
+<div contenteditable>é<span>'</span>ll</div>
+<div contenteditable>é<span>'l</span>l</div>
+<div contenteditable>é<span>'ll</span></div>
+<div contenteditable>é'<span>l</span>l</div>
+</div>
+<script src=""
+<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>
Modified: trunk/Source/WebCore/ChangeLog (149057 => 149058)
--- trunk/Source/WebCore/ChangeLog 2013-04-24 19:46:44 UTC (rev 149057)
+++ trunk/Source/WebCore/ChangeLog 2013-04-24 19:50:50 UTC (rev 149058)
@@ -1,3 +1,24 @@
+2013-04-23 Ryosuke Niwa <[email protected]>
+
+ 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 <[email protected]>
Do not use static string in DiagnosticLoggingKeys
Modified: trunk/Source/WebCore/editing/VisibleUnits.cpp (149057 => 149058)
--- trunk/Source/WebCore/editing/VisibleUnits.cpp 2013-04-24 19:46:44 UTC (rev 149057)
+++ trunk/Source/WebCore/editing/VisibleUnits.cpp 2013-04-24 19:50:50 UTC (rev 149058)
@@ -499,7 +499,7 @@
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();
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
