2010-03-05 Tony Chang <tony@chromium.org>

        Reviewed by Eric Seidel.

        https://bugs.webkit.org/show_bug.cgi?id=33247
        Backwards cursor movement incorrect when previous block ends with <br>.

        If the cursor is trying to move into a node that has a height of 0,
        skip over it.

        * editing/execCommand/move-selection-back-line-expected.txt: Added.
        * editing/execCommand/move-selection-back-line.html: Added.
2010-03-05  Tony Chang  <tony@chromium.org>

        Reviewed by Eric Seidel.

        https://bugs.webkit.org/show_bug.cgi?id=33247
        Backwards cursor movement incorrect when previous block ends with <br>.

        If the cursor is trying to move into a node that has a height of 0,
        skip over it.

        Test: editing/execCommand/move-selection-back-line.html

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

Canonical link: https://commits.webkit.org/46910@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@55613 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,16 @@
+2010-03-05  Tony Chang  <tony@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        https://bugs.webkit.org/show_bug.cgi?id=33247
+        Backwards cursor movement incorrect when previous block ends with <br>.
+
+        If the cursor is trying to move into a node that has a height of 0,
+        skip over it.
+
+        * editing/execCommand/move-selection-back-line-expected.txt: Added.
+        * editing/execCommand/move-selection-back-line.html: Added.
+
 2010-03-05  Chris Fleizach  <cfleizach@apple.com>
 
         Reviewed by Eric Seidel.

LayoutTests/editing/execCommand/move-selection-back-line-expected.txt

@@ -0,0 +1,6 @@
+first line.. test test test test -[ ] test test test
+
+second line. Put your cursor here [ ] and press the up arrow. The cursor should appear in the green box on the first line
+
+PASS
+

LayoutTests/editing/execCommand/move-selection-back-line.html

@@ -0,0 +1,17 @@
+<div contentEditable="true" style="font-family: monospace;">
+<p>first line.. test test test test -<span id="target" style="background-color:green">[ ]</span> test test test<br/></p>
+<p>second line. Put your cursor here [<span id="test"> </span>] and press the up arrow. The cursor should appear in the green box on the first line</p>
+</div>
+<div id="results">FAILED</div>
+<script src="../editing.js"></script>
+<script>
+function editingTest()
+{
+    execMoveSelectionBackwardByLineCommand();
+
+    // Verify that we ended up in "target".
+    if (window.getSelection().anchorNode.parentNode == document.getElementById("target"))
+        document.getElementById("results").innerText = "PASS";
+}
+runDumpAsTextEditingTest(false);
+</script>

WebCore/ChangeLog

@@ -1,3 +1,18 @@
+2010-03-05  Tony Chang  <tony@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        https://bugs.webkit.org/show_bug.cgi?id=33247
+        Backwards cursor movement incorrect when previous block ends with <br>.
+
+        If the cursor is trying to move into a node that has a height of 0,
+        skip over it.
+
+        Test: editing/execCommand/move-selection-back-line.html
+
+        * editing/visible_units.cpp:
+        (WebCore::previousLinePosition):
+
 2010-03-05  Andrey Kosyakov  <caseq@chromium.org>
 
         Reviewed by Eric Seidel.

WebCore/editing/visible_units.cpp

@@ -31,6 +31,7 @@
 #include "HTMLNames.h"
 #include "RenderBlock.h"
 #include "RenderLayer.h"
+#include "RenderObject.h"
 #include "TextBoundaries.h"
 #include "TextBreakIterator.h"
 #include "TextIterator.h"
@@ -253,6 +254,12 @@ static VisiblePosition nextBoundary(const VisiblePosition& c, BoundarySearchFunc
     return VisiblePosition(pos, VP_UPSTREAM_IF_POSSIBLE);
 }
 
+static bool canHaveCursor(RenderObject* o)
+{
+    return (o->isText() && toRenderText(o)->linesBoundingBox().height())
+        || (o->isBox() && toRenderBox(o)->borderBoundingBox().height());
+}
+
 // ---------
 
 static unsigned startWordBoundary(const UChar* characters, unsigned length, unsigned offset, BoundarySearchContextAvailability mayHaveMoreContext, bool& needMoreContext)
@@ -590,17 +597,20 @@ VisiblePosition previousLinePosition(const VisiblePosition &visiblePosition, int
                 break;
             Position pos(n, caretMinOffset(n));
             if (pos.isCandidate()) {
-                ASSERT(n->renderer());
-                Position maxPos(n, caretMaxOffset(n));
-                maxPos.getInlineBoxAndOffset(DOWNSTREAM, box, ignoredCaretOffset);
-                if (box) {
-                    // previous root line box found
-                    root = box->root();
-                    containingBlock = n->renderer()->containingBlock();
-                    break;
+                RenderObject* o = n->renderer();
+                ASSERT(o);
+                if (canHaveCursor(o)) {
+                    Position maxPos(n, caretMaxOffset(n));
+                    maxPos.getInlineBoxAndOffset(DOWNSTREAM, box, ignoredCaretOffset);
+                    if (box) {
+                        // previous root line box found
+                        root = box->root();
+                        containingBlock = n->renderer()->containingBlock();
+                        break;
+                    }
+
+                    return VisiblePosition(pos, DOWNSTREAM);
                 }
-
-                return VisiblePosition(pos, DOWNSTREAM);
             }
             n = previousLeafWithSameEditability(n);
         }