Reviewed by Ken Kocienda.
authorharrison <harrison@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Nov 2004 18:29:23 +0000 (18:29 +0000)
committerharrison <harrison@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 4 Nov 2004 18:29:23 +0000 (18:29 +0000)
<rdar://problem/3857753> REGRESSION (Mail): Delete incorrectly causes text to take on new style

        * khtml/editing/htmlediting.cpp:
        (khtml::DeleteSelectionCommand::moveNodesAfterNode): Fixed to move entire source subtree (up
        to, but not including, the enclosingBlockFlowElement) rather than just the source element.
        Fixed to insert after the destination subtree, rather than the destination element.  Handles
        edge case of deleting back to the top of the tree, where there is nothing left to insert after.
        * khtml/xml/dom_nodeimpl.cpp:
        (NodeImpl::enclosingNonBlockFlowElement): New method to support moveNodesAfterNode changes.
        * khtml/xml/dom_nodeimpl.h: Declare NodeImpl::enclosingNonBlockFlowElement
        * layout-tests/editing/deleting/delete-3857753-fix-expected.txt: Added.
        * layout-tests/editing/deleting/delete-3857753-fix.html: Added.

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

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

diff --git a/LayoutTests/editing/deleting/delete-3857753-fix-expected.txt b/LayoutTests/editing/deleting/delete-3857753-fix-expected.txt
new file mode 100644 (file)
index 0000000..ddc95cd
--- /dev/null
@@ -0,0 +1,23 @@
+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 [border: (2px solid #FF0000)]
+      RenderBlock {DIV} at (14,14) size 756x28
+        RenderInline {B} at (0,0) size 25x28
+          RenderText {TEXT} at (0,0) size 25x28
+            text run at (0,0) width 25: "on"
+        RenderInline {I} at (0,0) size 12x28
+          RenderText {TEXT} at (25,0) size 12x28
+            text run at (25,0) width 12: "o"
+      RenderBlock {DIV} at (14,42) size 756x28
+        RenderInline {B} at (0,0) size 54x28
+          RenderText {TEXT} at (0,0) size 54x28
+            text run at (0,0) width 54: "three"
+        RenderInline {I} at (0,0) size 40x28
+          RenderText {TEXT} at (54,0) size 40x28
+            text run at (54,0) width 40: "four"
+selection is CARET:
+start:      position 2 of child 1 {TEXT} of child 1 {B} of child 1 {DIV} of root {BODY}
+upstream:   position 2 of child 1 {TEXT} of child 1 {B} of child 1 {DIV} of root {BODY}
+downstream: position 0 of child 1 {TEXT} of child 2 {I} of child 1 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/deleting/delete-3857753-fix.html b/LayoutTests/editing/deleting/delete-3857753-fix.html
new file mode 100644 (file)
index 0000000..946e7f7
--- /dev/null
@@ -0,0 +1,36 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+    font-size: 24px; 
+}
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 2; i++)
+        moveSelectionForwardByCharacterCommand();    
+    for (i = 0; i < 4; i++)
+        extendSelectionForwardByCharacterCommand();    
+    deleteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root" class="editing">
+<div id="test"><b>one</b></div><div><i>two</i></div>
+<div><b>three</b><i>four</i></div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index 21092ba3074df8359cb32c773122cb9982d5c58b..7f846d6458ad51b6d6a55bb5d51a136bc291a1e6 100644 (file)
@@ -1,3 +1,20 @@
+2004-11-04  David Harrison  <harrison@apple.com>
+
+        Reviewed by Ken Kocienda.
+
+               <rdar://problem/3857753> REGRESSION (Mail): Delete incorrectly causes text to take on new style
+               
+        * khtml/editing/htmlediting.cpp:
+        (khtml::DeleteSelectionCommand::moveNodesAfterNode): Fixed to move entire source subtree (up
+        to, but not including, the enclosingBlockFlowElement) rather than just the source element.
+        Fixed to insert after the destination subtree, rather than the destination element.  Handles
+        edge case of deleting back to the top of the tree, where there is nothing left to insert after.
+        * khtml/xml/dom_nodeimpl.cpp:
+        (NodeImpl::enclosingNonBlockFlowElement): New method to support moveNodesAfterNode changes.
+        * khtml/xml/dom_nodeimpl.h: Declare NodeImpl::enclosingNonBlockFlowElement
+        * layout-tests/editing/deleting/delete-3857753-fix-expected.txt: Added.
+        * layout-tests/editing/deleting/delete-3857753-fix.html: Added.
+
 2004-11-03  Ken Kocienda  <kocienda@apple.com>
 
         Reviewed by me
index 6252d102da2a55b0fb6e08eecd2bfababf5a63bb..5d0a045875ab68ae791d6a93e8de2d1c4c5830e3 100644 (file)
@@ -1645,18 +1645,23 @@ void DeleteSelectionCommand::moveNodesAfterNode()
     // can be removed.
     removeBlockPlaceholderIfNeeded(startBlock);
 
-    NodeImpl *node = startNode == startBlock ? startBlock->firstChild() : startNode;
+    // Move the subtree containing node
+    NodeImpl *node = startNode->enclosingNonBlockFlowElement();
+
+    // Insert after the subtree containing destNode
+    NodeImpl *refNode = dstNode->enclosingNonBlockFlowElement();
+
+    // Nothing to do if start is already at the beginning of dstBlock
+    NodeImpl *dstBlock = refNode->enclosingBlockFlowElement();
+    if (startBlock == dstBlock->firstChild())
+        return;
 
     // Do the move.
-    NodeImpl *refNode = dstNode;
     while (node && node->isAncestor(startBlock)) {
         NodeImpl *moveNode = node;
         node = node->nextSibling();
         removeNode(moveNode);
-        if (refNode->isBlockFlow())
-            appendNode(moveNode, refNode);
-        else
-            insertNodeAfter(moveNode, refNode);
+        insertNodeAfter(moveNode, refNode);
         refNode = moveNode;
     }
 
index b40cb3ba737b24e78ddd34ae46c0b2b48afd7c06..b7149a6cdb8f759dcaf8202e6ad59fa62b676ec6 100644 (file)
 #include "xbl/xbl_binding_manager.h"
 #endif
 
+#if APPLE_CHANGES
+#include "KWQAssertions.h"
+#include "KWQLogging.h"
+#else
+#define ASSERT(assertion) assert(assertion)
+#define LOG(channel, formatAndArgs...) ((void)0)
+#endif
+
 using namespace DOM;
 using namespace khtml;
 
@@ -1357,6 +1365,21 @@ ElementImpl *NodeImpl::enclosingBlockFlowElement() const
     return 0;
 }
 
+ElementImpl *NodeImpl::enclosingNonBlockFlowElement() const
+{
+    NodeImpl *n = const_cast<NodeImpl *>(this);
+    NodeImpl *p;
+
+    while (1) {
+        p = n->parentNode();
+        if (!p || p->isBlockFlow() || p->id() == ID_BODY)
+            return static_cast<ElementImpl *>(n);
+        n = p;
+    }
+    ASSERT_NOT_REACHED();
+    return 0;
+}
+
 ElementImpl *NodeImpl::rootEditableElement() const
 {
     if (!isContentEditable())
index 79e7ca44231da0417483d95a774ad2728bad2feb..d8b8b3c940f0a373f12755c63d9fbcb7b2a5e45a 100644 (file)
@@ -169,6 +169,7 @@ public:
 
     bool isEditableBlock() const;
     ElementImpl *enclosingBlockFlowElement() const;
+    ElementImpl *enclosingNonBlockFlowElement() const;
     ElementImpl *rootEditableElement() const;
     
     bool inSameRootEditableElement(NodeImpl *);