2005-03-01 Ken Kocienda <kocienda@apple.com>
authorkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Mar 2005 17:04:57 +0000 (17:04 +0000)
committerkocienda <kocienda@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 2 Mar 2005 17:04:57 +0000 (17:04 +0000)
        Reviewed by Maciej

        Improved fix for this bug:

        <rdar://problem/3996605> Insert paragraph command puts new block in wrong place, creating difficult-to-handle HTML

        Maciej and I discussed this situation at length, and we came up with a better fix than I did earlier.

        * khtml/editing/htmlediting.cpp:
        (khtml::InsertParagraphSeparatorCommand::doApply): Simplify one special case so that it only handles the
        "last in block" situation. Remove special case for "downstream node is in different block" and handle
        this case with a little bit of special code in the general insertion case.

        Results studied to make sure there were no problems.

        * layout-tests/editing/deleting/delete-3959464-fix-expected.txt
        * layout-tests/editing/inserting/insert-div-001-expected.txt
        * layout-tests/editing/inserting/insert-div-002-expected.txt
        * layout-tests/editing/inserting/insert-div-004-expected.txt
        * layout-tests/editing/inserting/insert-div-005-expected.txt
        * layout-tests/editing/inserting/insert-div-009-expected.txt
        * layout-tests/editing/inserting/insert-div-014-expected.txt
        * layout-tests/editing/inserting/insert-div-018-expected.txt
        * layout-tests/editing/inserting/insert-div-024-expected.txt
        * layout-tests/editing/pasteboard/paste-text-011-expected.txt
        * layout-tests/editing/pasteboard/paste-text-013-expected.txt
        * layout-tests/editing/pasteboard/paste-text-015-expected.txt
        * layout-tests/editing/style/block-style-004-expected.txt
        * layout-tests/editing/style/block-style-005-expected.txt
        * layout-tests/editing/style/block-style-006-expected.txt

        New test:

        * layout-tests/editing/inserting/insert-div-027.html
        * layout-tests/editing/inserting/insert-div-027-expected.txt

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

LayoutTests/editing/inserting/insert-div-027-expected.txt [new file with mode: 0644]
LayoutTests/editing/inserting/insert-div-027.html [new file with mode: 0644]
WebCore/ChangeLog-2005-08-23
WebCore/khtml/editing/htmlediting.cpp

diff --git a/LayoutTests/editing/inserting/insert-div-027-expected.txt b/LayoutTests/editing/inserting/insert-div-027-expected.txt
new file mode 100644 (file)
index 0000000..c37b6ac
--- /dev/null
@@ -0,0 +1,29 @@
+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 (anonymous) at (0,0) size 784x56
+        RenderText {TEXT} at (0,0) size 781x56
+          text run at (0,0) width 781: "Test inserting paragraphs: should see two blue boxes, where the second blue box"
+          text run at (0,28) width 245: "starts with an empty line."
+      RenderBlock {DIV} at (0,56) size 784x36
+      RenderBlock {DIV} at (0,92) size 784x112 [border: (2px solid #0000FF)]
+        RenderBlock {DIV} at (14,14) size 756x56 [border: (2px solid #FF0000)]
+          RenderText {TEXT} at (14,14) size 34x28
+            text run at (14,14) width 34: "baz"
+        RenderBlock (anonymous) at (14,70) size 756x28
+          RenderText {TEXT} at (0,0) size 31x28
+            text run at (0,0) width 31: "bar"
+      RenderBlock {DIV} at (0,204) size 784x168 [border: (2px solid #0000FF)]
+        RenderBlock (anonymous) at (14,14) size 756x28
+          RenderBR {BR} at (0,0) size 0x28
+        RenderBlock {DIV} at (14,42) size 756x56 [border: (2px solid #FF0000)]
+          RenderText {TEXT} at (14,14) size 32x28
+            text run at (14,14) width 32: "foo"
+        RenderBlock {DIV} at (14,98) size 756x56 [border: (2px solid #FF0000)]
+          RenderBR {BR} at (14,14) size 0x28
+selection is CARET:
+start:      position 0 of child 1 {BR} of child 5 {DIV} of root {BODY}
+upstream:   position 0 of child 5 {DIV} of root {BODY}
+downstream: position 0 of child 1 {BR} of child 5 {DIV} of root {BODY}
diff --git a/LayoutTests/editing/inserting/insert-div-027.html b/LayoutTests/editing/inserting/insert-div-027.html
new file mode 100644 (file)
index 0000000..3862edd
--- /dev/null
@@ -0,0 +1,48 @@
+<html> 
+<head>
+
+<style>
+body {
+    font-size: 24px; 
+}
+.editing { 
+    border: 2px solid red; 
+    padding: 12px; 
+}
+div {
+    border: 2px solid blue; 
+    padding: 12px; 
+}
+
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+
+function editingTest() {
+    for (i = 0; i < 7; i++)
+        moveSelectionForwardByCharacterCommand();
+    insertParagraphCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body contenteditable id="root">
+
+Test inserting paragraphs: should see two blue boxes, where the second blue box starts with an empty line.
+
+<div style="border: none; height: 12px"></div>
+
+<div>
+<div class="editing" id="test">baz</div>bar<div class="editing">foo</div>
+<div class="editing"><br class="khtml-block-placeholder"></div>
+</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index f62f1d182af5884d0e1280b01e7ebb22b5471b56..38c2798cc145bb2129ac59475177df624c8adba4 100644 (file)
@@ -1,3 +1,41 @@
+ 2005-03-01  Ken Kocienda  <kocienda@apple.com>
+        Reviewed by Maciej
+
+        Improved fix for this bug:
+        
+        <rdar://problem/3996605> Insert paragraph command puts new block in wrong place, creating difficult-to-handle HTML
+
+        Maciej and I discussed this situation at length, and we came up with a better fix than I did earlier.
+
+        * khtml/editing/htmlediting.cpp:
+        (khtml::InsertParagraphSeparatorCommand::doApply): Simplify one special case so that it only handles the 
+        "last in block" situation. Remove special case for "downstream node is in different block" and handle
+        this case with a little bit of special code in the general insertion case.
+
+        Results studied to make sure there were no problems.
+
+        * layout-tests/editing/deleting/delete-3959464-fix-expected.txt
+        * layout-tests/editing/inserting/insert-div-001-expected.txt
+        * layout-tests/editing/inserting/insert-div-002-expected.txt
+        * layout-tests/editing/inserting/insert-div-004-expected.txt
+        * layout-tests/editing/inserting/insert-div-005-expected.txt
+        * layout-tests/editing/inserting/insert-div-009-expected.txt
+        * layout-tests/editing/inserting/insert-div-014-expected.txt
+        * layout-tests/editing/inserting/insert-div-018-expected.txt
+        * layout-tests/editing/inserting/insert-div-024-expected.txt
+        * layout-tests/editing/pasteboard/paste-text-011-expected.txt
+        * layout-tests/editing/pasteboard/paste-text-013-expected.txt
+        * layout-tests/editing/pasteboard/paste-text-015-expected.txt
+        * layout-tests/editing/style/block-style-004-expected.txt
+        * layout-tests/editing/style/block-style-005-expected.txt
+        * layout-tests/editing/style/block-style-006-expected.txt
+
+        New test:
+        
+        * layout-tests/editing/inserting/insert-div-027.html
+        * layout-tests/editing/inserting/insert-div-027-expected.txt
+
 2005-03-01  David Hyatt  <hyatt@apple.com>
 
        Fix for 4030890, regression with <sup> on Google.  Fix some bogus != comparison checks in verticalPositionHint.
index e3f2a19dc3fff65286597c35bced949bd4d17589..b0158863a0408727aac1ffbd13b55782cb9b4047 100644 (file)
@@ -3245,24 +3245,13 @@ void InsertParagraphSeparatorCommand::doApply()
     }
 
     //---------------------------------------------------------------------
-    // Handle case when position is in the last visible position in its block, 
-    // and similar case where downstream position is in another block.
-    bool downstreamInDifferentBlock = startBlock != pos.downstream(DoNotStayInBlock).node()->enclosingBlockFlowElement();
-    if (downstreamInDifferentBlock || isLastInBlock) {
+    // Handle case when position is in the last visible position in its block. 
+    if (isLastInBlock) {
         LOG(Editing, "insert paragraph separator: last in block case");
-        NodeImpl *refNode = isLastInBlock && !startBlockIsRoot ? startBlock : pos.node();
-        NodeImpl *parent = refNode->parentNode();
-        NodeImpl *rootEditable = refNode->rootEditableElement();
-        ASSERT(parent);
-        ASSERT(rootEditable);
-        while (parent && rootEditable && refNode != rootEditable && isLastVisiblePositionInNode(visiblePos, parent)) {
-            refNode = parent;
-            parent = parent->parentNode();
-        }
-        if (refNode == rootEditable)
-            appendNode(blockToInsert, refNode);
+        if (startBlockIsRoot)
+            appendNode(blockToInsert, startBlock);
         else
-            insertNodeAfter(blockToInsert, refNode);
+            insertNodeAfter(blockToInsert, startBlock);
         insertBlockPlaceholder(blockToInsert);
         setEndingSelection(Position(blockToInsert, 0), DOWNSTREAM);
         applyStyleAfterInsertion();
@@ -3329,9 +3318,18 @@ void InsertParagraphSeparatorCommand::doApply()
         parent = child;
     }
 
+    // Insert a block placeholder in this case because we know that ther will be no content
+    // on the first line of the new block before the first block child of the new block.
+    // So, we need the placeholder to "hold the first line open".
+    if (startBlock != pos.downstream(DoNotStayInBlock).node()->enclosingBlockFlowElement())
+        insertBlockPlaceholder(blockToInsert);
+
     // Move the start node and the siblings of the start node.
     if (startNode != startBlock) {
         NodeImpl *n = startNode;
+        if (pos.offset() >= startNode->caretMaxOffset()) {
+            n = startNode->nextSibling();
+        }
         while (n && n != blockToInsert) {
             NodeImpl *next = n->nextSibling();
             removeNode(n);