Changeset 117392 in webkit

Timestamp:

May 16, 2012 9:09:47 PM (12 years ago)

Author:

rniwa@webkit.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/selection/move-between-lines-of-different-editabilities-expected.txt (added)
• LayoutTests/editing/selection/move-between-lines-of-different-editabilities.html (added)
• LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/editing/visible_units.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2012-05-16  Ryosuke Niwa  <rniwa@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  <macpherson@chromium.org>
 

trunk/LayoutTests/editing/selection/move-by-word-visually-mac-expected.txt

@@ -88 +88 @@
 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]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-16  Ryosuke Niwa  <rniwa@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 <morrita@google.com>
 

trunk/Source/WebCore/editing/visible_units.cpp

@@ -61 +61 @@
 }
 
-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)
 {
@@ -89 +80 @@
 {
     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);
   
@@ -113 +103 @@
 {
     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);