Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Jan 2005 00:16:54 +0000 (00:16 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Jan 2005 00:16:54 +0000 (00:16 +0000)
        Fix for this bug:

        <rdar://problem/3959464> REGRESSION (Mail): Insertion point goes back to beginning of document after deleting

        * khtml/editing/htmlediting.cpp:
        (khtml::DeleteSelectionCommand::handleGeneralDelete): Add special case to handle retaining a fully-selected block.
        This fixes the bug.
        * layout-tests/editing/deleting/delete-3959464-fix-expected.txt: Added.
        * layout-tests/editing/deleting/delete-3959464-fix.html: Added.

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

LayoutTests/editing/deleting/delete-3959464-fix-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/delete-3959464-fix.html [new file with mode: 0644]
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp

diff --git a/LayoutTests/editing/deleting/delete-3959464-fix-expected.txt b/LayoutTests/editing/deleting/delete-3959464-fix-expected.txt
new file mode 100644 (file)
index 0000000..dea5fd1
--- /dev/null
@@ -0,0 +1,26 @@
+layer at (0,0) size 800x600
+  RenderCanvas 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 {DIV} at (0,0) size 784x84 [border: (2px solid #0000FF)]
+        RenderText {TEXT} at (14,14) size 715x56
+          text run at (14,14) width 715: "Should see \"foo\" and \"bar\" each in a separate red box. Note that the \"foo\""
+          text run at (14,42) width 197: "text is really \"foo \". "
+          text run at (211,42) width 63: "There "
+        RenderInline {B} at (0,0) size 50x28
+          RenderText {TEXT} at (274,42) size 50x28
+            text run at (274,42) width 50: "must"
+        RenderText {TEXT} at (324,42) size 318x28
+          text run at (324,42) width 318: " be a space at the end of the line."
+      RenderBlock {DIV} at (0,108) size 784x140
+        RenderBlock {DIV} at (0,0) size 784x56 [border: (2px solid #FF0000)]
+          RenderText {TEXT} at (14,14) size 32x28
+            text run at (14,14) width 32: "foo"
+        RenderBlock {DIV} at (0,56) size 784x56 [border: (2px solid #FF0000)]
+          RenderBR {BR} at (14,14) size 0x28
+        RenderBlock {DIV} at (0,112) size 784x28 [border: (2px solid #FF0000)]
+selection is CARET:
+start:      position 0 of child 3 {DIV} of root {DIV}
+upstream:   position 0 of child 3 {DIV} of root {DIV}
+downstream: position 1 of child 3 {DIV} of root {DIV}
diff --git a/LayoutTests/editing/deleting/delete-3959464-fix.html b/LayoutTests/editing/deleting/delete-3959464-fix.html
new file mode 100644 (file)
index 0000000..a5ddd44
--- /dev/null
@@ -0,0 +1,50 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 4; i++)
+        moveSelectionForwardByCharacterCommand();
+    for (i = 0; i < 3; i++)
+        insertParagraphCommand();
+    typeCharacterCommand();
+    extendSelectionBackwardByLineCommand();
+    deleteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body>
+<div class="explanation">
+Should see "foo" and "bar" each in a separate red box. Note that the "foo" text is really "foo ". 
+There <b>must</b> be a space at the end of the line.
+</div>
+
+<div contenteditable="true" id="root" style="word-wrap: break-word; -khtml-nbsp-mode: space; -khtml-line-break: after-white-space;">
+<div id="test" class="editing">foo</div>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index 52e96af..459e6d6 100644 (file)
@@ -1,3 +1,17 @@
+2005-01-21  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+
+        Fix for this bug:
+        
+        <rdar://problem/3959464> REGRESSION (Mail): Insertion point goes back to beginning of document after deleting
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::DeleteSelectionCommand::handleGeneralDelete): Add special case to handle retaining a fully-selected block.
+        This fixes the bug.
+        * layout-tests/editing/deleting/delete-3959464-fix-expected.txt: Added.
+        * layout-tests/editing/deleting/delete-3959464-fix.html: Added.
+
 2005-01-21  Richard Williamson   <rjw@apple.com>
 
        Fixed <rdar://problem/3966998> REGRESSION(179-TOT) clicking on gmail message brings me to blank screen
index b56993c..dccc32b 100644 (file)
@@ -1922,17 +1922,37 @@ void DeleteSelectionCommand::handleGeneralDelete()
 
     if (startOffset == 0 && m_startNode->isBlockFlow() && m_startBlock != m_endBlock && !m_endBlock->isAncestor(m_startBlock)) {
         // The block containing the start of the selection is completely selected. 
-        // Delete it all in one step right here.
+        // See if it can be deleted in one step right here.
         ASSERT(!m_downstreamEnd.node()->isAncestor(m_startNode));
 
-        // shift the start node to the start of the next block.
+        // The next few lines help us deal with a bit of a quirk.
+        //     1. Open a new Blot or Mail document
+        //     2. hit Return ten times or so
+        //     3. Type a letter (do not hit Return after it)
+        //     4. Type shift-up-arrow to select the line containing the letter and the previous blank line
+        //     5. Hit Delete
+        // You expect the insertion point to wind up at the start of the line where your selection began.
+        // Because of the nature of HTML, the editing code needs to perform a special check to get
+        // this behavior. So:
+        // If the entire start block is selected, and
+        //     a) the selection does not extend to the end of the document, then delete the start block, otherwise
+        //     b) the selection extends to the end of the document, then do not delete the start block.
+        //
         NodeImpl *old = m_startNode;
-        m_startNode = m_startBlock->traverseNextSibling();
-        m_startNode->ref();
+        VisiblePosition visibleEnd = VisiblePosition(m_downstreamEnd);
+        if (visibleEnd.next().isNull() && !isFirstVisiblePositionOnLine(visibleEnd)) {
+            m_startNode = m_startBlock->firstChild();
+        }
+        else {
+            // shift the start node to the start of the next block.
+            m_startNode = m_startBlock->traverseNextSibling();
+            removeFullySelectedNode(m_startBlock);
+        }
+
+        if (m_startNode)
+            m_startNode->ref();
         old->deref();
         startOffset = 0;
-
-        removeFullySelectedNode(m_startBlock);
     }
     else if (startOffset >= m_startNode->caretMaxOffset()) {
         // Move the start node to the next node in the tree since the startOffset is equal to