LayoutTests:

        Reviewed by Harrison.

        <rdar://problem/4889598> Problems with moveDown: and moveUp: in Notes with ToDos

        * editing/selection/4889598-expected.checksum: Added.
        * editing/selection/4889598-expected.png: Added.
        * editing/selection/4889598-expected.txt: Added.
        * editing/selection/4889598.html: Added.

WebCore:

        Reviewed by Harrison.

        <rdar://problem/4889598> Problems with moveDown: and moveUp: in Notes with ToDos

        The caret would disappear when moving from content above or below
        a ToDo if that ToDo doesn't have any content in it with the same
        x position as the caret. That's because closestLeafChildForXPos
        would return non-editable leaves, and which turn into non-editable
        VisiblePositions, which are invisible.

        * editing/visible_units.cpp:
        (WebCore::previousLinePosition): Ask closestLeafForXPos to only
        return editable leaves.
        (WebCore::nextLinePosition): Ditto.
        * rendering/RootInlineBox.cpp:
        (WebCore::isEditableLeaf): Added.
        (WebCore::RootInlineBox::closestLeafChildForXPos): If requested,
        return the closest editable leaf. Removed an early return if the
        position is before the first leaf, it's not really much of an
        optimization.
        * rendering/RootInlineBox.h:

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 480cca817dcf495bab10d44bce8c947294cc3cf2..dc58e445d3954b0ceeab211b928c6c3ac08b31a1 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,14 @@
+2007-06-06  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Harrison.
+
+        <rdar://problem/4889598> Problems with moveDown: and moveUp: in Notes with ToDos
+
+        * editing/selection/4889598-expected.checksum: Added.
+        * editing/selection/4889598-expected.png: Added.
+        * editing/selection/4889598-expected.txt: Added.
+        * editing/selection/4889598.html: Added.
+
 2007-06-06  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Hyatt.

diff --git a/LayoutTests/editing/selection/4889598-expected.checksum b/LayoutTests/editing/selection/4889598-expected.checksum
new file mode 100644 (file)
index 0000000..e5a49d8
--- /dev/null
+++ b/LayoutTests/editing/selection/4889598-expected.checksum
@@ -0,0 +1 @@
+327bce76bc950ef78dab1e48513b5b0c
\ No newline at end of file

diff --git a/LayoutTests/editing/selection/4889598-expected.png b/LayoutTests/editing/selection/4889598-expected.png
new file mode 100644 (file)
index 0000000..26c4fb2
Binary files /dev/null and b/LayoutTests/editing/selection/4889598-expected.png differ

diff --git a/LayoutTests/editing/selection/4889598-expected.txt b/LayoutTests/editing/selection/4889598-expected.txt
new file mode 100644 (file)
index 0000000..546d704
--- /dev/null
+++ b/LayoutTests/editing/selection/4889598-expected.txt
@@ -0,0 +1,65 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 1 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 1 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 1 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 1 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 1 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 1 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldEndEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidEndEditing:WebViewDidEndEditingNotification
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 1 of SPAN > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+layer at (0,0) size 800x600
+  RenderView at (0,0) size 800x600
+layer at (0,0) size 800x600
+  RenderBlock {HTML} at (0,0) size 800x600
+    RenderBody {BODY} at (8,8) size 784x584
+      RenderBlock {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 784x18
+          text run at (0,0) width 784: "This tests for a bug moving down by a line from a line just above a ToDo, and moving up by a line from just below a ToDo."
+      RenderBlock {DIV} at (0,34) size 784x64
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderText {#text} at (0,0) size 279x18
+            text run at (0,0) width 279: "Click anywhere in this line and move down."
+        RenderTable {TABLE} at (0,18) size 335x28 [border: (1px outset #808080)]
+          RenderTableSection {TBODY} at (1,1) size 333x26
+            RenderTableRow {TR} at (0,2) size 333x22
+              RenderTableCell {TD} at (2,2) size 329x22 [border: (1px inset #808080)] [r=0 c=0 rs=1 cs=1]
+                RenderText {#text} at (2,2) size 63x18
+                  text run at (2,2) width 63: "The caret "
+                RenderInline {SPAN} at (0,0) size 42x18 [color=#0000FF]
+                  RenderText {#text} at (65,2) size 42x18
+                    text run at (65,2) width 42: "should"
+                RenderText {#text} at (107,2) size 220x18
+                  text run at (107,2) width 220: " always go into the editable region."
+        RenderBlock {DIV} at (0,46) size 784x18
+          RenderText {#text} at (0,0) size 259x18
+            text run at (0,0) width 259: "Click anywhere in this line and move up."
+caret: position 6 of child 0 {#text} of child 1 {SPAN} of child 0 {TD} of child 0 {TR} of child 0 {TBODY} of child 3 {TABLE} of child 2 {DIV} of child 0 {BODY} of child 0 {HTML} of document

diff --git a/LayoutTests/editing/selection/4889598.html b/LayoutTests/editing/selection/4889598.html
new file mode 100644 (file)
index 0000000..9d53617
--- /dev/null
+++ b/LayoutTests/editing/selection/4889598.html
@@ -0,0 +1,22 @@
+<p>This tests for a bug moving down by a line from a line just above a ToDo, and moving up by a line from just below a ToDo.</p>
+<div id="div" contenteditable="true">
+<div id="above">Click anywhere in this line and move down.</div>
+<table border="1" contenteditable="false"><tr><td>The caret <span style="color: blue;" contenteditable="true">should</span> always go into the editable region.</td></tr></table>
+<div id="below">Click anywhere in this line and move up.</div>
+</div>
+
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpEditingCallbacks();
+var selection = window.getSelection();
+var above = document.getElementById("above").firstChild;
+selection.setPosition(above, 0);
+selection.modify("move", "forward", "line");
+selection.setPosition(above, above.length);
+selection.modify("move", "forward", "line");
+var below = document.getElementById("below").firstChild;
+selection.setPosition(below, 0);
+selection.modify("move", "backward", "line");
+selection.setPosition(below, below.length);
+selection.modify("move", "backward", "line");
+</script>

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 345242f165ab7df074218b7778a14c1ffc4ce68a..2d4afa7bd7db614ea1145f6d14b1160bc24cb181 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,27 @@
+2007-06-06  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by Harrison.
+
+        <rdar://problem/4889598> Problems with moveDown: and moveUp: in Notes with ToDos
+        
+        The caret would disappear when moving from content above or below
+        a ToDo if that ToDo doesn't have any content in it with the same
+        x position as the caret. That's because closestLeafChildForXPos
+        would return non-editable leaves, and which turn into non-editable
+        VisiblePositions, which are invisible.
+
+        * editing/visible_units.cpp:
+        (WebCore::previousLinePosition): Ask closestLeafForXPos to only
+        return editable leaves.
+        (WebCore::nextLinePosition): Ditto.
+        * rendering/RootInlineBox.cpp:
+        (WebCore::isEditableLeaf): Added.
+        (WebCore::RootInlineBox::closestLeafChildForXPos): If requested,
+        return the closest editable leaf. Removed an early return if the
+        position is before the first leaf, it's not really much of an
+        optimization.
+        * rendering/RootInlineBox.h:
+
 2007-06-06  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Anders.

diff --git a/WebCore/editing/visible_units.cpp b/WebCore/editing/visible_units.cpp
index 2efff8b133da928ae20969fc8fe4d8ae00fab8cf..8a922e00fa4cbb703461d1f6458be770a891e403 100644 (file)
--- a/WebCore/editing/visible_units.cpp
+++ b/WebCore/editing/visible_units.cpp
@@ -432,7 +432,7 @@ VisiblePosition previousLinePosition(const VisiblePosition &visiblePosition, int
         containingBlock->absolutePositionForContent(absx, absy);
         if (containingBlock->hasOverflowClip())
             containingBlock->layer()->subtractScrollOffset(absx, absy);
-        RenderObject *renderer = root->closestLeafChildForXPos(x - absx)->object();
+        RenderObject *renderer = root->closestLeafChildForXPos(x - absx, isEditablePosition(p))->object();
         Node* node = renderer->element();
         if (editingIgnoresContent(node))
             return Position(node->parent(), node->nodeIndex());
@@ -502,7 +502,7 @@ VisiblePosition nextLinePosition(const VisiblePosition &visiblePosition, int x)
         containingBlock->absolutePositionForContent(absx, absy);
         if (containingBlock->hasOverflowClip())
             containingBlock->layer()->subtractScrollOffset(absx, absy);
-        RenderObject *renderer = root->closestLeafChildForXPos(x - absx)->object();
+        RenderObject *renderer = root->closestLeafChildForXPos(x - absx, isEditablePosition(p))->object();
         Node* node = renderer->element();
         if (editingIgnoresContent(node))
             return Position(node->parent(), node->nodeIndex());

diff --git a/WebCore/rendering/RootInlineBox.cpp b/WebCore/rendering/RootInlineBox.cpp
index 72dadb11389af7a72d8983996c4fc6ac7bf2b2f2..bfc88178128b238ef4e7e3462735910cb2bc5f20 100644 (file)
--- a/WebCore/rendering/RootInlineBox.cpp
+++ b/WebCore/rendering/RootInlineBox.cpp
@@ -315,27 +315,34 @@ RenderBlock* RootInlineBox::block() const
     return static_cast<RenderBlock*>(m_object);
 }
 
-InlineBox* RootInlineBox::closestLeafChildForXPos(int x)
+bool isEditableLeaf(InlineBox* leaf)
+{
+    return leaf && leaf->object() && leaf->object()->element() && leaf->object()->element()->isContentEditable();
+}
+
+InlineBox* RootInlineBox::closestLeafChildForXPos(int x, bool onlyEditableLeaves)
 {
     InlineBox* firstLeaf = firstLeafChildAfterBox();
     InlineBox* lastLeaf = lastLeafChildBeforeBox();
-    if (firstLeaf == lastLeaf)
+    if (firstLeaf == lastLeaf && (!onlyEditableLeaves || isEditableLeaf(firstLeaf)))
         return firstLeaf;
 
     // Avoid returning a list marker when possible.
-    if (x <= firstLeaf->m_x && !firstLeaf->object()->isListMarker())
+    if (x <= firstLeaf->m_x && !firstLeaf->object()->isListMarker() && (!onlyEditableLeaves || isEditableLeaf(firstLeaf)))
         // The x coordinate is less or equal to left edge of the firstLeaf.
         // Return it.
         return firstLeaf;
 
-    if (x >= lastLeaf->m_x + lastLeaf->m_width && !lastLeaf->object()->isListMarker())
+    if (x >= lastLeaf->m_x + lastLeaf->m_width && !lastLeaf->object()->isListMarker() && (!onlyEditableLeaves || isEditableLeaf(lastLeaf)))
         // The x coordinate is greater or equal to right edge of the lastLeaf.
         // Return it.
         return lastLeaf;
 
+    InlineBox* closestLeaf = lastLeaf;
     for (InlineBox* leaf = firstLeaf; leaf && leaf != lastLeaf; leaf = leaf->nextLeafChild()) {
-        if (!leaf->object()->isListMarker()) {
+        if (!leaf->object()->isListMarker() && (!onlyEditableLeaves || isEditableLeaf(leaf))) {
             int leafX = leaf->m_x;
+            closestLeaf = leaf;
             if (x < leafX + leaf->m_width)
                 // The x coordinate is less than the right edge of the box.
                 // Return it.
@@ -343,7 +350,7 @@ InlineBox* RootInlineBox::closestLeafChildForXPos(int x)
         }
     }
 
-    return lastLeaf;
+    return closestLeaf;
 }
 
 void RootInlineBox::setLineBreakInfo(RenderObject* obj, unsigned breakPos, BidiStatus* status, BidiContext* context)

diff --git a/WebCore/rendering/RootInlineBox.h b/WebCore/rendering/RootInlineBox.h
index 7bde2a014decf69e7afdee1476b5424aa15f8acb..c6bce405f70a0521a4f38d62b2996b5b2a9a3178 100644 (file)
--- a/WebCore/rendering/RootInlineBox.h
+++ b/WebCore/rendering/RootInlineBox.h
@@ -120,7 +120,7 @@ public:
     int selectionBottom() { return m_overflow ? m_overflow->m_selectionBottom : m_y + m_height; }
     int selectionHeight() { return max(0, selectionBottom() - selectionTop()); }
 
-    InlineBox* closestLeafChildForXPos(int x);
+    InlineBox* closestLeafChildForXPos(int x, bool onlyEditableLeaves = false);
 
 protected:
     // Normally we are only as tall as the style on our block dictates, but we might have content