Reviewed by John

        Some delete and insert cleanups.

        * khtml/editing/htmlediting_impl.cpp:
        (DeleteSelectionCommandImpl::doApply): For ending position case 1,
        the caret should be placed before the first child of the containing block,
        not before the containing block itself. Also, add some code to handle
        converting nbsp's back to regular spaces. This will need to be improved
        some day to convert only nbsp's added by the editor to make rendering come out right.
        (InputTextCommandImpl::execute):
        (TypingCommandImpl::issueCommandForDeleteKey): Make deleting collapsible whitespace part
        of the work of deleting a selection, rather than something that needs to be done by a
        user of DeleteSelectionCommandImpl. This makes it impossible to leave out
        this essential step.
        (TypingCommandImpl::deleteKeyPressed): We can't use a possible optimization here until
        the code to do deletions properly has been factored better. Big FIXME added.
        * layout-tests/editing/deleting/delete-block-contents-001-expected.txt: Updated for
        ending position case 1 behavior change.
        * layout-tests/editing/deleting/delete-block-contents-002-expected.txt: Ditto.
        * layout-tests/editing/deleting/delete-block-contents-003-expected.txt: Ditto.

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

diff --git a/LayoutTests/editing/deleting/delete-block-contents-001-expected.txt b/LayoutTests/editing/deleting/delete-block-contents-001-expected.txt
index 9849805115a5dff8a8dcf90d0af93305de2b11ef..c45f8f78eca3537dcb5f8653bb2c7cd3e2c27e57 100644 (file)
--- a/LayoutTests/editing/deleting/delete-block-contents-001-expected.txt
+++ b/LayoutTests/editing/deleting/delete-block-contents-001-expected.txt
@@ -5,6 +5,6 @@ layer at (0,0) size 800x72
     RenderBody {BODY} at (8,8) size 784x56
       RenderBlock {DIV} at (0,0) size 784x56 [border: (2px solid #FF0000)]
 selection is CARET:
-start:      position 0 of  of root {DIV}
+start:      position 1 of  of root {DIV}
 upstream:   position 0 of  of root {DIV}
-downstream: position 0 of  of root {DIV}
+downstream: position 1 of  of root {DIV}

diff --git a/LayoutTests/editing/deleting/delete-block-contents-002-expected.txt b/LayoutTests/editing/deleting/delete-block-contents-002-expected.txt
index 9849805115a5dff8a8dcf90d0af93305de2b11ef..c45f8f78eca3537dcb5f8653bb2c7cd3e2c27e57 100644 (file)
--- a/LayoutTests/editing/deleting/delete-block-contents-002-expected.txt
+++ b/LayoutTests/editing/deleting/delete-block-contents-002-expected.txt
@@ -5,6 +5,6 @@ layer at (0,0) size 800x72
     RenderBody {BODY} at (8,8) size 784x56
       RenderBlock {DIV} at (0,0) size 784x56 [border: (2px solid #FF0000)]
 selection is CARET:
-start:      position 0 of  of root {DIV}
+start:      position 1 of  of root {DIV}
 upstream:   position 0 of  of root {DIV}
-downstream: position 0 of  of root {DIV}
+downstream: position 1 of  of root {DIV}

diff --git a/LayoutTests/editing/deleting/delete-block-contents-003-expected.txt b/LayoutTests/editing/deleting/delete-block-contents-003-expected.txt
index 6f2063714bbd9ce74f1972290b00dece6a29fb4d..cb786bb6234bb6d91762ea211fa73b6a208074ff 100644 (file)
--- a/LayoutTests/editing/deleting/delete-block-contents-003-expected.txt
+++ b/LayoutTests/editing/deleting/delete-block-contents-003-expected.txt
@@ -12,6 +12,6 @@ layer at (0,0) size 800x128
         RenderText {TEXT} at (0,0) size 0x0
       RenderBlock {DIV} at (0,56) size 784x56 [border: (2px solid #FF0000)]
 selection is CARET:
-start:      position 0 of child 3 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+start:      position 1 of child 3 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
 upstream:   position 0 of child 3 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
-downstream: position 0 of child 3 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}
+downstream: position 1 of child 3 {DIV} of child 2 {BODY} of child 1 {HTML} of root {}

diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23
index e37fcdc072e8140370a875a689bbbe8bc537927e..8d492a665c496e9df02ad70e13892389b45f1a34 100644 (file)
--- a/WebCore/ChangeLog-2005-08-23
+++ b/WebCore/ChangeLog-2005-08-23
@@ -1,3 +1,27 @@
+2004-04-12  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+        
+        Some delete and insert cleanups.
+
+        * khtml/editing/htmlediting_impl.cpp:
+        (DeleteSelectionCommandImpl::doApply): For ending position case 1,
+        the caret should be placed before the first child of the containing block, 
+        not before the containing block itself. Also, add some code to handle
+        converting nbsp's back to regular spaces. This will need to be improved
+        some day to convert only nbsp's added by the editor to make rendering come out right.
+        (InputTextCommandImpl::execute): 
+        (TypingCommandImpl::issueCommandForDeleteKey): Make deleting collapsible whitespace part 
+        of the work of deleting a selection, rather than something that needs to be done by a 
+        user of DeleteSelectionCommandImpl. This makes it impossible to leave out
+        this essential step.
+        (TypingCommandImpl::deleteKeyPressed): We can't use a possible optimization here until 
+        the code to do deletions properly has been factored better. Big FIXME added.
+        * layout-tests/editing/deleting/delete-block-contents-001-expected.txt: Updated for
+        ending position case 1 behavior change.
+        * layout-tests/editing/deleting/delete-block-contents-002-expected.txt: Ditto. 
+        * layout-tests/editing/deleting/delete-block-contents-003-expected.txt: Ditto.
+
 2004-04-09  Ken Kocienda  <kocienda@apple.com>
 
         Reviewed by Darin

diff --git a/WebCore/khtml/editing/htmlediting_impl.cpp b/WebCore/khtml/editing/htmlediting_impl.cpp
index 5c45aa21af170ac448016019f2795ae66f061517..3d2be27249d5e3976d9fa67e0852a195197f3310 100644 (file)
--- a/WebCore/khtml/editing/htmlediting_impl.cpp
+++ b/WebCore/khtml/editing/htmlediting_impl.cpp
@@ -816,7 +816,8 @@ void DeleteSelectionCommandImpl::doApply()
     if (m_selectionToDelete.state() != KHTMLSelection::RANGE)
         return;
 
-    KHTMLSelection selection = m_selectionToDelete;
+    deleteCollapsibleWhitespace(m_selectionToDelete);
+    KHTMLSelection selection = endingSelection();
 
     DOMPosition endingPosition;
     bool adjustEndingPositionDownstream = false;
@@ -855,7 +856,7 @@ void DeleteSelectionCommandImpl::doApply()
     // Start is not completely selected
     if (startAtStartOfBlock) {
         LOG(Editing,  "ending position case 1");
-        endingPosition = DOMPosition(downstreamStart.node()->containingEditableBlock(), 0);
+        endingPosition = DOMPosition(downstreamStart.node()->containingEditableBlock(), 1);
         adjustEndingPositionDownstream = true;
     }
     else if (!startCompletelySelected) {
@@ -1197,8 +1198,19 @@ void InputTextCommandImpl::execute(const DOMString &text)
     // that will replace this some day.
     if (isWS(text))
         insertSpace(textNode, offset);
-    else
+    else {
+        const DOMString &existingText = textNode->data();
+        if (textNode->length() >= 2 && offset >= 2 && isNBSP(existingText[offset - 1]) && !isWS(existingText[offset - 2])) {
+            // DOM looks like this:
+            // character nbsp caret
+            // As we are about to insert a non-whitespace character at the caret
+            // convert the nbsp to a regular space.
+            // EDIT FIXME: This needs to be improved some day to convert back only
+            // those nbsp's added by the editor to make rendering come out right.
+            replaceText(textNode, offset - 1, 1, " ");
+        }
         insertText(textNode, offset, text);
+    }
     setEndingSelection(DOMPosition(textNode, offset + text.length()));
     m_charactersAdded += text.length();
 }
@@ -1695,21 +1707,26 @@ void TypingCommandImpl::insertNewline()
 
 void TypingCommandImpl::issueCommandForDeleteKey()
 {
-    KHTMLSelection selection = endingSelection();
-    ASSERT(selection.state() != KHTMLSelection::NONE);
+    KHTMLSelection selectionToDelete = endingSelection();
+    ASSERT(selectionToDelete.state() != KHTMLSelection::NONE);
     
-    if (selection.state() == KHTMLSelection::CARET) {
-        KHTMLSelection selectionToDelete(selection.startPosition().previousCharacterPosition(), selection.startPosition());
-        deleteCollapsibleWhitespace(selectionToDelete);
-    }
-    else { // selection.state() == KHTMLSelection::RANGE
-        deleteCollapsibleWhitespace();
-    }
-    deleteSelection(endingSelection());
+    if (selectionToDelete.state() == KHTMLSelection::CARET)
+        selectionToDelete = KHTMLSelection(selectionToDelete.startPosition().previousCharacterPosition(), selectionToDelete.startPosition());
+    deleteSelection(selectionToDelete);
 }
 
 void TypingCommandImpl::deleteKeyPressed()
 {
+// EDIT FIXME: The ifdef'ed out code below should be re-enabled.
+// In order for this to happen, the deleteCharacter case
+// needs work. Specifically, the caret-positioning code
+// and whitespace-handling code in DeleteSelectionCommandImpl::doApply()
+// needs to be factored out so it can be used again here.
+// Until that work is done, issueCommandForDeleteKey() does the
+// right thing, but less efficiently and with the cost of more
+// objects.
+    issueCommandForDeleteKey();
+#if 0    
     if (m_cmds.count() == 0) {
         issueCommandForDeleteKey();
     }
@@ -1730,6 +1747,7 @@ void TypingCommandImpl::deleteKeyPressed()
             issueCommandForDeleteKey();
         }
     }
+#endif
 }
 
 void TypingCommandImpl::removeCommand(const EditCommand &cmd)