Title: [94988] trunk
Revision
94988
Author
rn...@webkit.org
Date
2011-09-12 16:32:07 -0700 (Mon, 12 Sep 2011)

Log Message

REGRESSION: Moving up doesn't work in some cases
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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (94987 => 94988)


--- trunk/LayoutTests/ChangeLog	2011-09-12 23:14:30 UTC (rev 94987)
+++ trunk/LayoutTests/ChangeLog	2011-09-12 23:32:07 UTC (rev 94988)
@@ -1,3 +1,18 @@
+2011-09-12  Ryosuke Niwa  <rn...@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  <bda...@apple.com>
 
         https://bugs.webkit.org/show_bug.cgi?id=67898

Added: trunk/LayoutTests/editing/selection/move-up-into-wrapped-line-expected.txt (0 => 94988)


--- trunk/LayoutTests/editing/selection/move-up-into-wrapped-line-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/move-up-into-wrapped-line-expected.txt	2011-09-12 23:32:07 UTC (rev 94988)
@@ -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>
+| "
+"

Added: trunk/LayoutTests/editing/selection/move-up-into-wrapped-line.html (0 => 94988)


--- trunk/LayoutTests/editing/selection/move-up-into-wrapped-line.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/move-up-into-wrapped-line.html	2011-09-12 23:32:07 UTC (rev 94988)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div style="width: 6ex;" contenteditable=true>
+<div>hello world</div><div><br></div>
+</div>
+<script src=""
+<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>

Modified: trunk/Source/WebCore/ChangeLog (94987 => 94988)


--- trunk/Source/WebCore/ChangeLog	2011-09-12 23:14:30 UTC (rev 94987)
+++ trunk/Source/WebCore/ChangeLog	2011-09-12 23:32:07 UTC (rev 94988)
@@ -1,3 +1,21 @@
+2011-09-12  Ryosuke Niwa  <rn...@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  <le...@chromium.org>
 
         Make the ThreadSafeRefCounted support in CrossThreadCopier work for T*.

Modified: trunk/Source/WebCore/editing/visible_units.cpp (94987 => 94988)


--- trunk/Source/WebCore/editing/visible_units.cpp	2011-09-12 23:14:30 UTC (rev 94987)
+++ trunk/Source/WebCore/editing/visible_units.cpp	2011-09-12 23:32:07 UTC (rev 94988)
@@ -533,7 +533,7 @@
         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) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to