Log Message
Moving caret up or down skips lines when there's a non-editable line https://bugs.webkit.org/show_bug.cgi?id=81490
Reviewed by Eric Seidel. Source/WebCore: The bug was caused by previousRootInlineBoxCandidatePosition and nextRootInlineBoxCandidatePosition skipping leaf nodes that constitute a new line and belong to the same editable region because block elements that separate lines are not editable so it looked as if all editable lines belong to a single line as far as those two functions are concerned. Fixed the bug by using the first leaf node that belongs to the same editable region but does not belong in the same as the start node. This patch is based on a patch authored by Yi Shen (Nokia). Test: editing/selection/move-between-lines-of-different-editabilities.html * editing/visible_units.cpp: (WebCore::previousRootInlineBoxCandidatePosition): (WebCore::nextRootInlineBoxCandidatePosition): LayoutTests: Added a regression test. * editing/selection/move-between-lines-of-different-editabilities.html: Added. * editing/selection/move-by-word-visually-mac-expected.txt: Rebaselined a test case. It failed before this change and still fails after this change.
Modified Paths
- trunk/LayoutTests/ChangeLog
- trunk/LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt
- trunk/Source/WebCore/ChangeLog
- trunk/Source/WebCore/editing/visible_units.cpp
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (117391 => 117392)
--- trunk/LayoutTests/ChangeLog 2012-05-17 03:58:23 UTC (rev 117391)
+++ trunk/LayoutTests/ChangeLog 2012-05-17 04:09:47 UTC (rev 117392)
@@ -1,3 +1,16 @@
+2012-05-16 Ryosuke Niwa <rn...@webkit.org>
+
+ Moving caret up or down skips lines when there's a non-editable line
+ https://bugs.webkit.org/show_bug.cgi?id=81490
+
+ Reviewed by Eric Seidel.
+
+ Added a regression test.
+
+ * editing/selection/move-between-lines-of-different-editabilities.html: Added.
+ * editing/selection/move-by-word-visually-mac-expected.txt: Rebaselined a test case. It failed
+ before this change and still fails after this change.
+
2012-05-16 Luke Macpherson <macpher...@chromium.org>
Add tests for CSS Variables.
Added: trunk/LayoutTests/editing/selection/move-between-lines-of-different-editabilities-expected.txt (0 => 117392)
--- trunk/LayoutTests/editing/selection/move-between-lines-of-different-editabilities-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/move-between-lines-of-different-editabilities-expected.txt 2012-05-17 04:09:47 UTC (rev 117392)
@@ -0,0 +1,61 @@
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
+This test moves caret between lines of different editabilities.
+
+Initial condition:
+| "
+Line 1<#selection-caret> editable"
+| <div>
+| contenteditable="false"
+| "Uneditable div 1"
+| "
+Line 2 editable"
+| <div>
+| contenteditable="false"
+| "Uneditable div 2"
+| "
+Line 3 editable"
+
+After moving forward by line:
+| "
+Line 1 editable"
+| <div>
+| contenteditable="false"
+| "Uneditable div 1"
+| "
+Line 2<#selection-caret> editable"
+| <div>
+| contenteditable="false"
+| "Uneditable div 2"
+| "
+Line 3 editable"
+
+After moving forward by line:
+| "
+Line 1 editable"
+| <div>
+| contenteditable="false"
+| "Uneditable div 1"
+| "
+Line 2 editable"
+| <div>
+| contenteditable="false"
+| "Uneditable div 2"
+| "
+Line 3<#selection-caret> editable"
+
+After moving forward by line:
+| "
+Line 1 editable"
+| <div>
+| contenteditable="false"
+| "Uneditable div 1"
+| "
+Line 2 editable"
+| <div>
+| contenteditable="false"
+| "Uneditable div 2"
+| "
+Line 3 editable<#selection-caret>"
Added: trunk/LayoutTests/editing/selection/move-between-lines-of-different-editabilities.html (0 => 117392)
--- trunk/LayoutTests/editing/selection/move-between-lines-of-different-editabilities.html (rev 0)
+++ trunk/LayoutTests/editing/selection/move-between-lines-of-different-editabilities.html 2012-05-17 04:09:47 UTC (rev 117392)
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<body>
+<style>
+
+div[contenteditable=false] {
+ background: red;
+}
+
+</style>
+<div id="test" contenteditable=true>
+Line 1 editable<div contenteditable=false>Uneditable div 1</div>
+Line 2 editable<div contenteditable=false>Uneditable div 2</div>
+Line 3 editable</div>
+<script src=""
+<script src=""
+<script>
+
+Markup.description('This test moves caret between lines of different editabilities.')
+
+document.getElementById('test').focus();
+
+for (var i = 0; i < 6; i++)
+ execMoveSelectionForwardByCharacterCommand();
+
+if (window.layoutTestController)
+ layoutTestController.dumpEditingCallbacks();
+
+Markup.dump('test', 'Initial condition');
+for (var i = 0; i < 3; i++) {
+ execMoveSelectionForwardByLineCommand();
+ Markup.dump('test', 'After moving forward by line');
+}
+
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt (117391 => 117392)
--- trunk/LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt 2012-05-17 03:58:23 UTC (rev 117391)
+++ trunk/LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt 2012-05-17 04:09:47 UTC (rev 117392)
@@ -87,8 +87,9 @@
"opq rst uvw xyz"[15, 12, 8, 4, 0], "abc def ghi jkl mn "[16, 12, 8, 4, 0]
Test 18, RTL:
Move left by one word
-" abc def AAA AAA hij AAA AAA uvw xyz "[1, 5, 8, 12, 16, 20, 24, 28, 32, 36], <DIV>[0], "AAA kj AAA mn opq AAA AAA"[3, 6, 10, 13, 17, 21, 25] FAIL expected: [" abc def AAA AAA hij AAA AAA uvw xyz "[ 1, 5, 8, 12, 16, 20, 24, 28, 32, 36, ]"AAA kj AAA mn opq AAA AAA"[ 3, 6, 10, 13, 17, 21, 25]
+" abc def AAA AAA hij AAA AAA uvw xyz "[1, 5, 8, 12, 16, 20, 24, 28, 32, 36], <DIV>[0], <DIV>[0], "AAA kj AAA mn opq AAA AAA"[3, 6, 10, 13, 17, 21, 25] FAIL expected: [" abc def AAA AAA hij AAA AAA uvw xyz "[ 1, 5, 8, 12, 16, 20, 24, 28, 32, 36, ]"AAA kj AAA mn opq AAA AAA"[ 3, 6, 10, 13, 17, 21, 25]
" abc def AAA AAA hij AAA AAA uvw xyz "[36], <DIV>[0] FAIL expected "AAA kj AAA mn opq AAA AAA"[ 3]
+<DIV>[0], <DIV>[0] FAIL expected "AAA kj AAA mn opq AAA AAA"[ 3]
Move right by one word
"AAA kj AAA mn opq AAA AAA"[25, 22, 18, 14, 11, 7, 4, 0], " abc def AAA AAA hij AAA AAA uvw xyz "[33, 29, 25, 21, 17, 13, 9, 4, 1]
Test 19, LTR:
Modified: trunk/Source/WebCore/ChangeLog (117391 => 117392)
--- trunk/Source/WebCore/ChangeLog 2012-05-17 03:58:23 UTC (rev 117391)
+++ trunk/Source/WebCore/ChangeLog 2012-05-17 04:09:47 UTC (rev 117392)
@@ -1,3 +1,26 @@
+2012-05-16 Ryosuke Niwa <rn...@webkit.org>
+
+ Moving caret up or down skips lines when there's a non-editable line
+ https://bugs.webkit.org/show_bug.cgi?id=81490
+
+ Reviewed by Eric Seidel.
+
+ The bug was caused by previousRootInlineBoxCandidatePosition and nextRootInlineBoxCandidatePosition
+ skipping leaf nodes that constitute a new line and belong to the same editable region because block elements
+ that separate lines are not editable so it looked as if all editable lines belong to a single line as far as
+ those two functions are concerned.
+
+ Fixed the bug by using the first leaf node that belongs to the same editable region but does not belong in
+ the same as the start node.
+
+ This patch is based on a patch authored by Yi Shen (Nokia).
+
+ Test: editing/selection/move-between-lines-of-different-editabilities.html
+
+ * editing/visible_units.cpp:
+ (WebCore::previousRootInlineBoxCandidatePosition):
+ (WebCore::nextRootInlineBoxCandidatePosition):
+
2012-05-16 MORITA Hajime <morr...@google.com>
Unreviewed attempt to fix Mac SL build.
Modified: trunk/Source/WebCore/editing/visible_units.cpp (117391 => 117392)
--- trunk/Source/WebCore/editing/visible_units.cpp 2012-05-17 03:58:23 UTC (rev 117391)
+++ trunk/Source/WebCore/editing/visible_units.cpp 2012-05-17 04:09:47 UTC (rev 117392)
@@ -60,15 +60,6 @@
return 0;
}
-static Node* enclosingNodeWithNonInlineRenderer(Node* node)
-{
- for (; node; node = node->parentNode()) {
- if (node->renderer() && !node->renderer()->isInline())
- return node;
- }
- return 0;
-}
-
static Node* nextLeafWithSameEditability(Node* node, EditableType editableType = ContentIsEditable)
{
if (!node)
@@ -88,10 +79,9 @@
static Position previousRootInlineBoxCandidatePosition(Node* node, const VisiblePosition& visiblePosition, EditableType editableType)
{
Node* highestRoot = highestEditableRoot(visiblePosition.deepEquivalent(), editableType);
- Node* enclosingBlockNode = enclosingNodeWithNonInlineRenderer(node);
Node* previousNode = previousLeafWithSameEditability(node, editableType);
- while (previousNode && enclosingBlockNode == enclosingNodeWithNonInlineRenderer(previousNode))
+ while (previousNode && inSameLine(firstPositionInOrBeforeNode(previousNode), visiblePosition))
previousNode = previousLeafWithSameEditability(previousNode, editableType);
while (previousNode && !previousNode->isShadowRoot()) {
@@ -112,9 +102,8 @@
static Position nextRootInlineBoxCandidatePosition(Node* node, const VisiblePosition& visiblePosition, EditableType editableType)
{
Node* highestRoot = highestEditableRoot(visiblePosition.deepEquivalent(), editableType);
- Node* enclosingBlockNode = enclosingNodeWithNonInlineRenderer(node);
Node* nextNode = nextLeafWithSameEditability(node, editableType);
- while (nextNode && enclosingBlockNode == enclosingNodeWithNonInlineRenderer(nextNode))
+ while (nextNode && inSameLine(firstPositionInOrBeforeNode(nextNode), visiblePosition))
nextNode = nextLeafWithSameEditability(nextNode, ContentIsEditable);
while (nextNode && !nextNode->isShadowRoot()) {
_______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
