REGRESSION: Moving up doesn't work in some cases
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Sep 2011 23:32:07 +0000 (23:32 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 12 Sep 2011 23:32:07 +0000 (23:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=67522

Reviewed by Eric Seidel.

Source/WebCore:

The bug was caused by previousLinePosition's attempting to obtain the last root line box using
a position at minCaretOffset (which is, in practice, located at the beginning of wrapped lines).

Fix the bug by calling maxCaretOffset instead. Because isCandidate returns false at (br, 1),
use the positionBeforeNode for br elements.

Test: editing/selection/move-up-into-wrapped-line.html

* editing/visible_units.cpp:
(WebCore::previousLinePosition):

LayoutTests:

Add a test to move caret upwards from an empty line below wrapped lines.

WebKit used to skip wrapped lines and placed caret at the beginning of the first of those wrapped lines
instead of before the last.

* editing/selection/move-up-into-wrapped-line-expected.txt: Added.
* editing/selection/move-up-into-wrapped-line.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/move-up-into-wrapped-line-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/move-up-into-wrapped-line.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/editing/visible_units.cpp

index 0a4494a..c49a8e6 100644 (file)
@@ -1,3 +1,18 @@
+2011-09-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION: Moving up doesn't work in some cases
+        https://bugs.webkit.org/show_bug.cgi?id=67522
+
+        Reviewed by Eric Seidel.
+
+        Add a test to move caret upwards from an empty line below wrapped lines.
+
+        WebKit used to skip wrapped lines and placed caret at the beginning of the first of those wrapped lines
+        instead of before the last.
+
+        * editing/selection/move-up-into-wrapped-line-expected.txt: Added.
+        * editing/selection/move-up-into-wrapped-line.html: Added.
+
 2011-09-12  Beth Dakin  <bdakin@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=67898
diff --git a/LayoutTests/editing/selection/move-up-into-wrapped-line-expected.txt b/LayoutTests/editing/selection/move-up-into-wrapped-line-expected.txt
new file mode 100644 (file)
index 0000000..3c2a7ec
--- /dev/null
@@ -0,0 +1,10 @@
+This test moves up caret into a wrapped line. 
+i.e. the caret is moved from the empty line below "hello world" to before "world".
+| "
+"
+| <div>
+|   "hello <#selection-caret>world"
+| <div>
+|   <br>
+| "
+"
diff --git a/LayoutTests/editing/selection/move-up-into-wrapped-line.html b/LayoutTests/editing/selection/move-up-into-wrapped-line.html
new file mode 100644 (file)
index 0000000..95d43e8
--- /dev/null
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div style="width: 6ex;" contenteditable=true>
+<div>hello world</div><div><br></div>
+</div>
+<script src="../../resources/dump-as-markup.js"></script>
+<script>
+
+Markup.description('This test moves up caret into a wrapped line. \n'
+ + 'i.e. the caret is moved from the empty line below "hello world" to before "world".');
+
+var div = document.getElementsByTagName('div')[0];
+div.focus();
+getSelection().setPosition(div, div.childNodes.length);
+getSelection().modify('move', 'backward', 'line');
+
+Markup.dump(div);
+
+</script>
+</body>
+</html>
index b999290..693a252 100644 (file)
@@ -1,3 +1,21 @@
+2011-09-12  Ryosuke Niwa  <rniwa@webkit.org>
+
+        REGRESSION: Moving up doesn't work in some cases
+        https://bugs.webkit.org/show_bug.cgi?id=67522
+
+        Reviewed by Eric Seidel.
+
+        The bug was caused by previousLinePosition's attempting to obtain the last root line box using
+        a position at minCaretOffset (which is, in practice, located at the beginning of wrapped lines).
+
+        Fix the bug by calling maxCaretOffset instead. Because isCandidate returns false at (br, 1),
+        use the positionBeforeNode for br elements.
+
+        Test: editing/selection/move-up-into-wrapped-line.html
+
+        * editing/visible_units.cpp:
+        (WebCore::previousLinePosition):
+
 2011-09-12  David Levin  <levin@chromium.org>
 
         Make the ThreadSafeRefCounted support in CrossThreadCopier work for T*.
index 02d3a8d..a6d2dda 100644 (file)
@@ -533,7 +533,7 @@ VisiblePosition previousLinePosition(const VisiblePosition &visiblePosition, int
         while (n) {
             if (highestEditableRoot(firstPositionInOrBeforeNode(n)) != highestRoot)
                 break;
-            Position pos = createLegacyEditingPosition(n, caretMinOffset(n));
+            Position pos = n->hasTagName(brTag) ? positionBeforeNode(n) : createLegacyEditingPosition(n, caretMaxOffset(n));
             if (pos.isCandidate()) {
                 pos.getInlineBoxAndOffset(DOWNSTREAM, box, ignoredCaretOffset);
                 if (box) {