Reviewed by John
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Feb 2005 17:54:00 +0000 (17:54 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Feb 2005 17:54:00 +0000 (17:54 +0000)
        Fix for this bug:

        <rdar://problem/3980209> Mail crashed when I pressed Cmd-Shift-[ (nil-deref in ApplyStyleCommand::addBlockStyleIfNeeded)

        * khtml/editing/htmlediting.cpp:
        (khtml::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary): Reordered the new block
        insertion so that it come before the move. The logic stays exactly the same, however, with the old
        ordering, the new block could want to become a child of itself come insertion time. I considered
        making a more complicated code change to fix this problem, but the simple reordering works just
        as well, and seems less risky.

        These all changed in an insignificant way. It seems that with the new code, some empty text nodes
        got reordered in the document. This has no effect on anything visible to the user.

        * layout-tests/editing/style/create-block-for-style-003-expected.txt
        * layout-tests/editing/style/create-block-for-style-004-expected.txt
        * layout-tests/editing/style/create-block-for-style-009-expected.txt
        * layout-tests/editing/style/create-block-for-style-011-expected.txt
        * layout-tests/editing/style/create-block-for-style-013-expected.txt

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

LayoutTests/editing/style/create-block-for-style-003-expected.txt
LayoutTests/editing/style/create-block-for-style-004-expected.txt
LayoutTests/editing/style/create-block-for-style-009-expected.txt
LayoutTests/editing/style/create-block-for-style-011-expected.txt
LayoutTests/editing/style/create-block-for-style-013-expected.txt
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp

index f200a15787d2d392d344ec440b27696b84592be1..d6dffe1860ca61b0d7f8d62bcf59a865cbd111b6 100644 (file)
@@ -19,6 +19,7 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (14,70) size 756x56 [border: (2px solid #FF0000)]
             RenderText {TEXT} at (361,14) size 34x28
               text run at (361,14) width 34: "baz"
+          RenderBlock (anonymous) at (14,126) size 756x0
 selection is CARET:
 start:      position 0 of child 1 {TEXT} of child 5 {DIV} of child 1 {DIV} of root {DIV}
 upstream:   position 0 of child 5 {DIV} of child 1 {DIV} of root {DIV}
index 7db6e3f88bbc24b7c14007f1698278226eb88f61..9cf2cbbe1e4e6702fb5b1581900c1d8a98b635fe 100644 (file)
@@ -20,6 +20,7 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
             RenderText {TEXT} at (361,14) size 34x28
               text run at (361,14) width 34: "baz"
+          RenderBlock (anonymous) at (14,98) size 756x0
 selection is CARET:
 start:      position 3 of child 1 {TEXT} of child 3 {DIV} of child 2 {DIV} of root {DIV}
 upstream:   position 3 of child 1 {TEXT} of child 3 {DIV} of child 2 {DIV} of root {DIV}
index d6d5c5a2c253710e0d86ed1141c18d755b08ef01..1258e16122790861ea5a7320b4dd6992783de7bf 100644 (file)
@@ -15,10 +15,10 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
             RenderText {TEXT} at (14,14) size 31x28
               text run at (14,14) width 31: "bar"
-          RenderBlock (anonymous) at (14,98) size 756x0
           RenderBlock {DIV} at (14,98) size 756x56 [border: (2px solid #FF0000)]
             RenderText {TEXT} at (361,14) size 34x28
               text run at (361,14) width 34: "baz"
+          RenderBlock (anonymous) at (14,154) size 756x0
 selection is CARET:
 start:      position 2 of child 1 {TEXT} of child 3 {DIV} of child 1 {DIV} of root {DIV}
 upstream:   position 2 of child 1 {TEXT} of child 3 {DIV} of child 1 {DIV} of root {DIV}
index 815268521bbeecf3191d78528a9c150471264811..73ed21cea27df40d454b88bd7782cfeed7ab2013 100644 (file)
@@ -15,10 +15,10 @@ layer at (0,0) size 800x600
           RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
             RenderText {TEXT} at (362,14) size 31x28
               text run at (362,14) width 31: "bar"
-          RenderBlock (anonymous) at (14,98) size 756x0
           RenderBlock {DIV} at (14,98) size 756x56 [border: (2px solid #FF0000)]
             RenderText {TEXT} at (361,14) size 34x28
               text run at (361,14) width 34: "baz"
+          RenderBlock (anonymous) at (14,154) size 756x0
 selection is RANGE:
 start:      position 0 of child 1 {TEXT} of child 2 {DIV} of child 1 {DIV} of root {DIV}
 upstream:   position 0 of child 2 {DIV} of child 1 {DIV} of root {DIV}
index 4ac6e6bed9156fac506a0cc6853fb47db69414b5..ef81a1ddc0a3d96cf56bfa49c935de7351ea6df5 100644 (file)
@@ -20,6 +20,7 @@ layer at (0,0) size 800x600
             RenderBlock {DIV} at (14,42) size 728x56 [border: (2px solid #FF0000)]
               RenderText {TEXT} at (347,14) size 34x28
                 text run at (347,14) width 34: "baz"
+            RenderBlock (anonymous) at (14,98) size 728x0
 selection is CARET:
 start:      position 0 of child 1 {TEXT} of child 3 {DIV} of child 2 {DIV} of child 1 {DIV} of root {DIV}
 upstream:   position 0 of child 3 {DIV} of child 2 {DIV} of child 1 {DIV} of root {DIV}
index 97aaffb2c43cbc8104e495c9707e764ec33c5e92..f4f46ed175c175bd7a916fd08744e03305f32004 100644 (file)
@@ -1,3 +1,27 @@
+2005-02-23  Ken Kocienda  <kocienda@apple.com>
+
+        Reviewed by John
+
+        Fix for this bug:
+        
+        <rdar://problem/3980209> Mail crashed when I pressed Cmd-Shift-[ (nil-deref in ApplyStyleCommand::addBlockStyleIfNeeded)
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary): Reordered the new block
+        insertion so that it come before the move. The logic stays exactly the same, however, with the old
+        ordering, the new block could want to become a child of itself come insertion time. I considered
+        making a more complicated code change to fix this problem, but the simple reordering works just
+        as well, and seems less risky.
+        
+        These all changed in an insignificant way. It seems that with the new code, some empty text nodes
+        got reordered in the document. This has no effect on anything visible to the user.
+        
+        * layout-tests/editing/style/create-block-for-style-003-expected.txt
+        * layout-tests/editing/style/create-block-for-style-004-expected.txt
+        * layout-tests/editing/style/create-block-for-style-009-expected.txt
+        * layout-tests/editing/style/create-block-for-style-011-expected.txt
+        * layout-tests/editing/style/create-block-for-style-013-expected.txt
+
 2005-02-23  Darin Adler  <darin@apple.com>
 
         Reviewed by John.
index 9ed36862ec55219da2a7e14341263989a9b35551..c3185b4f054e685e53f09bbee6895c22f860575f 100644 (file)
@@ -1171,14 +1171,6 @@ void CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Posi
     if (paragraphStart.offset() >= paragraphStart.node()->caretMaxOffset())
         moveNode = moveNode->traverseNextNode();
     NodeImpl *endNode = paragraphEnd.node();
-    while (moveNode && !moveNode->isBlockFlow()) {
-        NodeImpl *next = moveNode->traverseNextNode();
-        removeNode(moveNode);
-        appendNode(moveNode, newBlock);
-        if (moveNode == endNode)
-            break;
-        moveNode = next;
-    }
 
     if (paragraphStart.node()->id() == ID_BODY) {
         insertNodeAt(newBlock, paragraphStart.node(), 0);
@@ -1195,6 +1187,15 @@ void CompositeEditCommand::moveParagraphContentsToNewBlockIfNecessary(const Posi
     else {
         insertNodeAfter(newBlock, beforeParagraphStart.node());
     }
+
+    while (moveNode && !moveNode->isBlockFlow()) {
+        NodeImpl *next = moveNode->traverseNextNode();
+        removeNode(moveNode);
+        appendNode(moveNode, newBlock);
+        if (moveNode == endNode)
+            break;
+        moveNode = next;
+    }
 }
 
 //==========================================================================================