LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 May 2006 21:43:03 +0000 (21:43 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 May 2006 21:43:03 +0000 (21:43 +0000)
        Reviewed by darin

        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8653>
        Remove a use of hasMoreThanOneBlock

        Deleting didn't merge because of code that stopped the merge whenever
        the end of the selection to delete was in a fully selected line.
        * editing/deleting/merge-endOfParagraph-expected.checksum: Added.
        * editing/deleting/merge-endOfParagraph-expected.png: Added.
        * editing/deleting/merge-endOfParagraph-expected.txt: Added.
        * editing/deleting/merge-endOfParagraph.html: Added.

        Two testcases where paste did not request a merge from deletion, but should have.
        * editing/pasteboard/merge-after-delete-1-expected.checksum: Added.
        * editing/pasteboard/merge-after-delete-1-expected.png: Added.
        * editing/pasteboard/merge-after-delete-1-expected.txt: Added.
        * editing/pasteboard/merge-after-delete-1.html: Added.
        * editing/pasteboard/merge-after-delete-2-expected.checksum: Added.
        * editing/pasteboard/merge-after-delete-2-expected.png: Added.
        * editing/pasteboard/merge-after-delete-2-expected.txt: Added.
        * editing/pasteboard/merge-after-delete-2.html: Added.

        Code that prevents nesting incoming blocks in the block being pasted into
        could reverse the order of pasted paragraphs.
        * editing/pasteboard/prevent-block-nesting-01-expected.checksum: Added.
        * editing/pasteboard/prevent-block-nesting-01-expected.png: Added.
        * editing/pasteboard/prevent-block-nesting-01-expected.txt: Added.
        * editing/pasteboard/prevent-block-nesting-01.html: Added.

WebCore:

        Reviewed by darin

        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8653>
        Remove a use of hasMoreThanOneBlock, which uses info from the test rendering.

        * editing/DeleteSelectionCommand.cpp:
        (WebCore::DeleteSelectionCommand::initializePositionData):
        Removed code that stopped the merge if the end of the selection to delete
        was in a fully selected line, which was nonsense.

        (WebCore::DeleteSelectionCommand::mergeParagraphs):
        Deletion does a bad job of updating the endpoints of the selection as it removes
        content.  If the endpoints have been flip flipped, bail.
        If deletion has removed everything from the block that contained the
        start of the selection to delete, we can't create a visible position inside
        that block to serve as a destination for the merge.  So, we insert a placeholder
        at that position to prop the block open to let content in.

        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::doApply):
        Added an assert and two early returns for cases where we'll crash.
        Removed a use of !fragment.hasMoreThanOneBlock, which uses test rendering info
        and which was wrong.
        If we've already inserted content during the start merge, insertionPos will be
        the position just after that content, so inserting new content before insertionPos
        will reverse its order.

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

20 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/deleting/merge-endOfParagraph-expected.checksum [new file with mode: 0644]
LayoutTests/editing/deleting/merge-endOfParagraph-expected.png [new file with mode: 0644]
LayoutTests/editing/deleting/merge-endOfParagraph-expected.txt [new file with mode: 0644]
LayoutTests/editing/deleting/merge-endOfParagraph.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/merge-after-delete-1-expected.checksum [new file with mode: 0644]
LayoutTests/editing/pasteboard/merge-after-delete-1-expected.png [new file with mode: 0644]
LayoutTests/editing/pasteboard/merge-after-delete-1-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/merge-after-delete-1.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/merge-after-delete-2-expected.checksum [new file with mode: 0644]
LayoutTests/editing/pasteboard/merge-after-delete-2-expected.png [new file with mode: 0644]
LayoutTests/editing/pasteboard/merge-after-delete-2-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/merge-after-delete-2.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.checksum [new file with mode: 0644]
LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.png [new file with mode: 0644]
LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/prevent-block-nesting-01.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/DeleteSelectionCommand.cpp
WebCore/editing/ReplaceSelectionCommand.cpp

index 468c650c7aab08f141880d35f65e09e4400ec562..2589846f6554444b029d7ee8f9fbe6067f121565 100644 (file)
@@ -1,3 +1,34 @@
+2006-04-28  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+        
+        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8653>
+        Remove a use of hasMoreThanOneBlock
+
+        Deleting didn't merge because of code that stopped the merge whenever 
+        the end of the selection to delete was in a fully selected line.
+        * editing/deleting/merge-endOfParagraph-expected.checksum: Added.
+        * editing/deleting/merge-endOfParagraph-expected.png: Added.
+        * editing/deleting/merge-endOfParagraph-expected.txt: Added.
+        * editing/deleting/merge-endOfParagraph.html: Added.
+        
+        Two testcases where paste did not request a merge from deletion, but should have.
+        * editing/pasteboard/merge-after-delete-1-expected.checksum: Added.
+        * editing/pasteboard/merge-after-delete-1-expected.png: Added.
+        * editing/pasteboard/merge-after-delete-1-expected.txt: Added.
+        * editing/pasteboard/merge-after-delete-1.html: Added.
+        * editing/pasteboard/merge-after-delete-2-expected.checksum: Added.
+        * editing/pasteboard/merge-after-delete-2-expected.png: Added.
+        * editing/pasteboard/merge-after-delete-2-expected.txt: Added.
+        * editing/pasteboard/merge-after-delete-2.html: Added.
+        
+        Code that prevents nesting incoming blocks in the block being pasted into
+        could reverse the order of pasted paragraphs.
+        * editing/pasteboard/prevent-block-nesting-01-expected.checksum: Added.
+        * editing/pasteboard/prevent-block-nesting-01-expected.png: Added.
+        * editing/pasteboard/prevent-block-nesting-01-expected.txt: Added.
+        * editing/pasteboard/prevent-block-nesting-01.html: Added.
+
 2006-04-28  Alexey Proskuryakov  <ap@nypop.com>
 
         Reviewed by hyatt.
diff --git a/LayoutTests/editing/deleting/merge-endOfParagraph-expected.checksum b/LayoutTests/editing/deleting/merge-endOfParagraph-expected.checksum
new file mode 100644 (file)
index 0000000..c3d374e
--- /dev/null
@@ -0,0 +1 @@
+3409f57bae89d8a3598082b33db9eb19
\ No newline at end of file
diff --git a/LayoutTests/editing/deleting/merge-endOfParagraph-expected.png b/LayoutTests/editing/deleting/merge-endOfParagraph-expected.png
new file mode 100644 (file)
index 0000000..c817b17
Binary files /dev/null and b/LayoutTests/editing/deleting/merge-endOfParagraph-expected.png differ
diff --git a/LayoutTests/editing/deleting/merge-endOfParagraph-expected.txt b/LayoutTests/editing/deleting/merge-endOfParagraph-expected.txt
new file mode 100644 (file)
index 0000000..53db406
--- /dev/null
@@ -0,0 +1,32 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 2 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+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 {P} at (0,0) size 784x36
+        RenderText {#text} at (0,0) size 747x36
+          text run at (0,0) width 747: "When the selection to delete ends at the end of a paragraph, that paragraph will be completely deleted, but a <br> or an"
+          text run at (0,18) width 555: "empty block will remain. Merging must happen to remove that <br> or prune that block."
+      RenderBlock {P} at (0,52) size 784x36
+        RenderText {#text} at (0,0) size 763x36
+          text run at (0,0) width 763: "This test illustrates a case where merging wasn't allowed to happen just because the end of the selection to delete was in a"
+          text run at (0,18) width 237: "fully selected line, which is nonsense."
+      RenderBlock {P} at (0,104) size 784x54
+        RenderText {#text} at (0,0) size 779x54
+          text run at (0,0) width 315: "Fixing that bug exposed a problem with merging. "
+          text run at (315,0) width 462: "If deletion empties out the block that contained the start of the selection to"
+          text run at (0,18) width 472: "delete, that block can collapse away and become impossible to merge into. "
+          text run at (472,18) width 307: "So we insert a placeholder to prop it open so that"
+          text run at (0,36) width 142: "the merge can happen."
+      RenderBlock {DIV} at (5,174) size 774x32 [border: (1px solid #000000)]
+        RenderBlock {DIV} at (6,6) size 762x20 [border: (1px solid #FF0000)]
+          RenderBR {BR} at (1,1) size 0x18
+caret: position 0 of child 0 {BR} of child 0 {DIV} of child 6 {DIV} of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/deleting/merge-endOfParagraph.html b/LayoutTests/editing/deleting/merge-endOfParagraph.html
new file mode 100644 (file)
index 0000000..4fd5f79
--- /dev/null
@@ -0,0 +1,21 @@
+<style>
+div {
+    border: 1px solid black;
+    margin: 5px;
+}
+</style>
+<p>When the selection to delete ends at the end of a paragraph, that paragraph will be completely deleted, but a &lt;br&gt; or an empty block will remain. Merging must happen to remove that &lt;br&gt; or prune that block.</p>
+<p>This test illustrates a case where merging wasn't allowed to happen just because the end of the selection to delete was in a fully selected line, which is nonsense.</p>
+<p>Fixing that bug exposed a problem with merging.  If deletion empties out the block that contained the start of the selection to delete, that block can collapse away and become impossible to merge into.  So we insert a placeholder to prop it open so that the merge can happen.</p>
+<div id="test" contenteditable="true"><div style="border: 1px solid red;">bar</div><div style="border: 1px solid blue;">bar<br></div></div>
+
+<script>
+var s = window.getSelection();
+var e = document.getElementById("test");
+
+s.setPosition(e, 0);
+s.modify("extend", "forward", "line");
+s.modify("extend", "forward", "line");
+s.modify("extend", "forward", "line");
+document.execCommand("Delete");
+</script>
\ No newline at end of file
diff --git a/LayoutTests/editing/pasteboard/merge-after-delete-1-expected.checksum b/LayoutTests/editing/pasteboard/merge-after-delete-1-expected.checksum
new file mode 100644 (file)
index 0000000..db3333e
--- /dev/null
@@ -0,0 +1 @@
+e7f6cb4e3b5f26b22d74e174fde6ed2d
\ No newline at end of file
diff --git a/LayoutTests/editing/pasteboard/merge-after-delete-1-expected.png b/LayoutTests/editing/pasteboard/merge-after-delete-1-expected.png
new file mode 100644 (file)
index 0000000..9896df4
Binary files /dev/null and b/LayoutTests/editing/pasteboard/merge-after-delete-1-expected.png differ
diff --git a/LayoutTests/editing/pasteboard/merge-after-delete-1-expected.txt b/LayoutTests/editing/pasteboard/merge-after-delete-1-expected.txt
new file mode 100644 (file)
index 0000000..bd5544f
--- /dev/null
@@ -0,0 +1,38 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 2 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 3 of #text > DIV > DIV > BODY > HTML > #document to 3 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+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 {P} at (0,0) size 784x54
+        RenderText {#text} at (0,0) size 777x54
+          text run at (0,0) width 773: "When ReplaceSelectionCommand deletes the current selection, it should request that the deletion do a merge in some cases"
+          text run at (0,18) width 777: "(normally, though, it should ask that a merge not be done, because a merge will cause information about block nesting to be"
+          text run at (0,36) width 35: "lost). "
+          text run at (35,36) width 368: "It wasn't requesting a merge in cases where it should have."
+      RenderBlock {P} at (0,70) size 784x72
+        RenderText {#text} at (0,0) size 783x72
+          text run at (0,0) width 783: "The failing case is where the incoming fragment has more than one block, and the selection being pasted into ends at the end"
+          text run at (0,18) width 99: "of a paragraph. "
+          text run at (99,18) width 683: "Any time the selection being pasted into ends at the end of a paragraph, deleting will leave leave a) a br or b)"
+          text run at (0,36) width 318: "an empty block at the end of the deleted selection. "
+          text run at (318,36) width 457: "So, not merging will leave an extraneous empty line or a collapsed block"
+          text run at (0,54) width 154: "after the paste operation."
+      RenderBlock {DIV} at (2,158) size 780x48 [border: (1px solid #000000)]
+        RenderBlock {DIV} at (3,3) size 774x20 [border: (1px solid #FF0000)]
+          RenderText {#text} at (1,1) size 8x18
+            text run at (1,1) width 8: "b"
+          RenderText {#text} at (9,1) size 21x18
+            text run at (9,1) width 21: "foo"
+        RenderBlock {DIV} at (3,25) size 774x20 [border: (1px solid #000000)]
+          RenderText {#text} at (1,1) size 20x18
+            text run at (1,1) width 20: "bar"
+caret: position 3 of child 0 {#text} of child 1 {DIV} of child 4 {DIV} of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/pasteboard/merge-after-delete-1.html b/LayoutTests/editing/pasteboard/merge-after-delete-1.html
new file mode 100644 (file)
index 0000000..eed832b
--- /dev/null
@@ -0,0 +1,20 @@
+<style>
+div {
+    border: 1px solid black;
+    margin: 2px;
+}
+</style>
+<p>When ReplaceSelectionCommand deletes the current selection, it should request that the deletion do a merge in some cases (normally, though, it should ask that a merge not be done, because a merge will cause information about block nesting to be lost).  It wasn't requesting a merge in cases where it should have.</p>
+<p>The failing case is where the incoming fragment has more than one block, and the selection being pasted into ends at the end of a paragraph.  Any time the selection being pasted into ends at the end of a paragraph, deleting will leave leave a) a br or b) an empty block at the end of the deleted selection.  So, not merging will leave an extraneous empty line or a collapsed block after the paste operation.</p>
+<div id="test" contenteditable="true"><div style="border: 1px solid red;">bar</div><div style="border: 1px solid blue;">bar<br></div></div>
+
+<script>
+var s = window.getSelection();
+var e = document.getElementById("test");
+
+s.setPosition(e, 0);
+s.modify("move", "forward", "character");
+s.modify("extend", "forward", "line");
+s.modify("extend", "forward", "line");
+document.execCommand("InsertHTML", false, "<div>foo</div><div>bar</div>");
+</script>
\ No newline at end of file
diff --git a/LayoutTests/editing/pasteboard/merge-after-delete-2-expected.checksum b/LayoutTests/editing/pasteboard/merge-after-delete-2-expected.checksum
new file mode 100644 (file)
index 0000000..db3333e
--- /dev/null
@@ -0,0 +1 @@
+e7f6cb4e3b5f26b22d74e174fde6ed2d
\ No newline at end of file
diff --git a/LayoutTests/editing/pasteboard/merge-after-delete-2-expected.png b/LayoutTests/editing/pasteboard/merge-after-delete-2-expected.png
new file mode 100644 (file)
index 0000000..9896df4
Binary files /dev/null and b/LayoutTests/editing/pasteboard/merge-after-delete-2-expected.png differ
diff --git a/LayoutTests/editing/pasteboard/merge-after-delete-2-expected.txt b/LayoutTests/editing/pasteboard/merge-after-delete-2-expected.txt
new file mode 100644 (file)
index 0000000..db82f75
--- /dev/null
@@ -0,0 +1,39 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 3 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 3 of #text > DIV > DIV > BODY > HTML > #document to 3 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+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 {P} at (0,0) size 784x54
+        RenderText {#text} at (0,0) size 777x54
+          text run at (0,0) width 773: "When ReplaceSelectionCommand deletes the current selection, it should request that the deletion do a merge in some cases"
+          text run at (0,18) width 777: "(normally, though, it should ask that a merge not be done, because a merge will cause information about block nesting to be"
+          text run at (0,36) width 35: "lost). "
+          text run at (35,36) width 368: "It wasn't requesting a merge in cases where it should have."
+      RenderBlock {P} at (0,70) size 784x72
+        RenderText {#text} at (0,0) size 783x72
+          text run at (0,0) width 783: "The failing case is where the incoming fragment has more than one block, and the selection being pasted into ends at the end"
+          text run at (0,18) width 99: "of a paragraph. "
+          text run at (99,18) width 683: "Any time the selection being pasted into ends at the end of a paragraph, deleting will leave leave a) a br or b)"
+          text run at (0,36) width 318: "an empty block at the end of the deleted selection. "
+          text run at (318,36) width 457: "So, not merging will leave an extraneous empty line or a collapsed block"
+          text run at (0,54) width 154: "after the paste operation."
+      RenderBlock {DIV} at (2,158) size 780x48 [border: (1px solid #000000)]
+        RenderBlock {DIV} at (3,3) size 774x20 [border: (1px solid #FF0000)]
+          RenderText {#text} at (1,1) size 8x18
+            text run at (1,1) width 8: "b"
+          RenderText {#text} at (9,1) size 21x18
+            text run at (9,1) width 21: "foo"
+        RenderBlock (anonymous) at (1,25) size 778x0
+        RenderBlock {DIV} at (3,25) size 774x20 [border: (1px solid #000000)]
+          RenderText {#text} at (1,1) size 20x18
+            text run at (1,1) width 20: "bar"
+caret: position 3 of child 0 {#text} of child 1 {DIV} of child 4 {DIV} of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/pasteboard/merge-after-delete-2.html b/LayoutTests/editing/pasteboard/merge-after-delete-2.html
new file mode 100644 (file)
index 0000000..84eb825
--- /dev/null
@@ -0,0 +1,20 @@
+<style>
+div {
+    border: 1px solid black;
+    margin: 2px;
+}
+</style>
+<p>When ReplaceSelectionCommand deletes the current selection, it should request that the deletion do a merge in some cases (normally, though, it should ask that a merge not be done, because a merge will cause information about block nesting to be lost).  It wasn't requesting a merge in cases where it should have.</p>
+<p>The failing case is where the incoming fragment has more than one block, and the selection being pasted into ends at the end of a paragraph.  Any time the selection being pasted into ends at the end of a paragraph, deleting will leave leave a) a br or b) an empty block at the end of the deleted selection.  So, not merging will leave an extraneous empty line or a collapsed block after the paste operation.</p>
+<div id="test" contenteditable="true"><div style="border: 1px solid red;">bar</div>bar<br></div>
+
+<script>
+var s = window.getSelection();
+var e = document.getElementById("test");
+
+s.setPosition(e, 0);
+s.modify("move", "forward", "character");
+s.modify("extend", "forward", "line");
+s.modify("extend", "forward", "line");
+document.execCommand("InsertHTML", false, "<div>foo</div><div>bar</div>");
+</script>
\ No newline at end of file
diff --git a/LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.checksum b/LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.checksum
new file mode 100644 (file)
index 0000000..23d4649
--- /dev/null
@@ -0,0 +1 @@
+e26abbffa2e9e5b16e26c0ae45590271
\ No newline at end of file
diff --git a/LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.png b/LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.png
new file mode 100644 (file)
index 0000000..f93e13f
Binary files /dev/null and b/LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.png differ
diff --git a/LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.txt b/LayoutTests/editing/pasteboard/prevent-block-nesting-01-expected.txt
new file mode 100644 (file)
index 0000000..1db84af
--- /dev/null
@@ -0,0 +1,28 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 85 of #text > DIV > SPAN > DIV > BODY > HTML > #document to 85 of #text > DIV > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+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 {P} at (0,0) size 784x18
+        RenderText {#text} at (0,0) size 705x18
+          text run at (0,0) width 705: "The code in paste that prevents block nesting had a bug where the order of pasted paragraphs could be reversed."
+      RenderBlock {DIV} at (2,34) size 780x66 [border: (1px solid #FF0000)]
+        RenderBlock (anonymous) at (1,1) size 778x18
+          RenderText {#text} at (0,0) size 389x18
+            text run at (0,0) width 389: "There should be an empty line between these two paragraphs."
+          RenderInline {SPAN} at (0,0) size 0x18
+        RenderBlock (anonymous) at (1,21) size 778x42
+          RenderBlock {DIV} at (2,0) size 774x20 [border: (1px solid #FF0000)]
+            RenderBR {BR} at (1,1) size 0x18
+          RenderBlock {DIV} at (2,22) size 774x20 [border: (1px solid #FF0000)]
+            RenderText {#text} at (1,1) size 540x18
+              text run at (1,1) width 540: "This paragraph and the empty line should have be in their own divs with a red border."
+        RenderBlock (anonymous) at (1,65) size 778x0
+          RenderInline {SPAN} at (0,0) size 0x0
+caret: position 85 of child 0 {#text} of child 1 {DIV} of child 1 {SPAN} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/pasteboard/prevent-block-nesting-01.html b/LayoutTests/editing/pasteboard/prevent-block-nesting-01.html
new file mode 100644 (file)
index 0000000..41a4c95
--- /dev/null
@@ -0,0 +1,16 @@
+<style>
+div {
+    border: 1px solid red;
+    margin: 2px;
+}
+</style>
+<p>The code in paste that prevents block nesting had a bug where the order of pasted paragraphs could be reversed.</p>
+<div id="test" contenteditable="true"><br></div>
+
+<script>
+var s = window.getSelection();
+var e = document.getElementById("test");
+
+s.setPosition(e, 0);
+document.execCommand("InsertHTML", false, "There should be an empty line between these two paragraphs.<span><div><br></div></span><div>This paragraph and the empty line should have be in their own divs with a red border.</div>");
+</script>
\ No newline at end of file
index a22a7c35eee9717492c0576de3d1c4f77a8eda87..6019f80bb41a7b21d4bce04b2246287ec2839e0c 100644 (file)
@@ -1,3 +1,32 @@
+2006-05-01  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+        
+        <http://bugzilla.opendarwin.org/show_bug.cgi?id=8653>
+        Remove a use of hasMoreThanOneBlock, which uses info from the test rendering.
+
+        * editing/DeleteSelectionCommand.cpp:
+        (WebCore::DeleteSelectionCommand::initializePositionData):
+        Removed code that stopped the merge if the end of the selection to delete
+        was in a fully selected line, which was nonsense.
+        
+        (WebCore::DeleteSelectionCommand::mergeParagraphs):
+        Deletion does a bad job of updating the endpoints of the selection as it removes 
+        content.  If the endpoints have been flip flipped, bail.
+        If deletion has removed everything from the block that contained the
+        start of the selection to delete, we can't create a visible position inside 
+        that block to serve as a destination for the merge.  So, we insert a placeholder 
+        at that position to prop the block open to let content in.
+        
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::doApply):
+        Added an assert and two early returns for cases where we'll crash.
+        Removed a use of !fragment.hasMoreThanOneBlock, which uses test rendering info 
+        and which was wrong.
+        If we've already inserted content during the start merge, insertionPos will be 
+        the position just after that content, so inserting new content before insertionPos 
+        will reverse its order.
+
 2006-05-01  Mitz Pettel  <opendarwin.org@mitzpettel.com>
 
         Reviewed by Darin.
index bee2480b1f7afd788e718606d655543790e149aa..ea7688b54a72eb8d109d28fdc1139a25604d2c17 100644 (file)
@@ -198,17 +198,6 @@ void DeleteSelectionCommand::initializePositionData()
     m_endBlock = m_upstreamEnd.node()->enclosingBlockFlowElement();
     m_startNode = m_upstreamStart.node();
 
-    //
-    // Handle detecting if the line containing the selection end is itself fully selected.
-    // This is one of the tests that determines if block merging of content needs to be done.
-    //
-    VisiblePosition visibleEnd(m_downstreamEnd, VP_DEFAULT_AFFINITY);
-    if (isEndOfParagraph(visibleEnd)) {
-        Position previousLineStart = previousLinePosition(visibleEnd, 0).deepEquivalent();
-        if (previousLineStart.isNull() || Range::compareBoundaryPoints(previousLineStart, m_downstreamStart) >= 0)
-            m_mergeBlocksAfterDelete = false;
-    }
-
     debugPosition("m_upstreamStart      ", m_upstreamStart);
     debugPosition("m_downstreamStart    ", m_downstreamStart);
     debugPosition("m_upstreamEnd        ", m_upstreamEnd);
@@ -499,18 +488,29 @@ void DeleteSelectionCommand::mergeParagraphs()
     if (!m_downstreamEnd.node()->inDocument() || !m_upstreamStart.node()->inDocument())
          return;
          
-    // Do not move content between parts of a table.
+    // FIXME: The deletion algorithm shouldn't let this happen.
+    if (Range::compareBoundaryPoints(m_upstreamStart, m_downstreamEnd) > 0)
+        return;
+        
+    // FIXME: Merging will always be unnecessary in this case, but we really bail here because this is a case where
+    // deletion commonly fails to adjust its endpoints, which would cause the visible position comparison below to false negative.
+    if (m_endBlock == m_startBlock)
+        return;
+        
+    // Don't move content between parts of a table.
     if (isTableStructureNode(m_downstreamEnd.node()->enclosingBlockFlowElement()) || isTableStructureNode(m_upstreamStart.node()->enclosingBlockFlowElement()))
         return;
         
     VisiblePosition startOfParagraphToMove(m_downstreamEnd);
     VisiblePosition mergeDestination(m_upstreamStart);
     
-    if (mergeDestination == startOfParagraphToMove)
-        return;
+    // We need to merge into m_upstreamStart's block, but it's been emptied out and collapsed by deletion.
+    if (!mergeDestination.deepEquivalent().node()->isAncestor(m_upstreamStart.node()->enclosingBlockFlowElement())) {
+        insertNodeAt(createBreakElement(document()).get(), m_upstreamStart.node(), m_upstreamStart.offset());
+        mergeDestination = VisiblePosition(m_upstreamStart);
+    }
     
-    // FIXME: The above early return should be all we need. 
-    if (m_endBlock == m_startBlock)
+    if (mergeDestination == startOfParagraphToMove)
         return;
         
     VisiblePosition endOfParagraphToMove = endOfParagraph(startOfParagraphToMove);
index efc71792fc98377754d06e2256d8842b9b31bc19..bc678bcf795751f72b67fa6f017ec5dc6b79335a 100644 (file)
@@ -506,6 +506,9 @@ void ReplaceSelectionCommand::doApply()
     // collect information about the current selection, prior to deleting the selection
     Selection selection = endingSelection();
     ASSERT(selection.isCaretOrRange());
+    ASSERT(selection.start().node());
+    if (selection.isNone() || !selection.start().node())
+        return;
     
     if (!selection.isContentRichlyEditable())
         m_matchStyle = true;
@@ -534,7 +537,10 @@ void ReplaceSelectionCommand::doApply()
     
     // delete the current range selection, or insert paragraph for caret selection, as needed
     if (selection.isRange()) {
-        bool mergeBlocksAfterDelete = !(fragment.hasInterchangeNewlineAtStart() || fragment.hasInterchangeNewlineAtEnd() || fragment.hasMoreThanOneBlock());
+        // When the end of the selection to delete is at the end of a paragraph, and the selection
+        // to delete spans multiple blocks, not merging will leave an empty line containing the
+        // end of the selection to delete.
+        bool mergeBlocksAfterDelete = !fragment.hasInterchangeNewlineAtEnd() && !fragment.hasInterchangeNewlineAtStart() && isEndOfParagraph(visibleEnd);
         deleteSelection(false, mergeBlocksAfterDelete);
         updateLayout();
         visibleStart = endingSelection().visibleStart();
@@ -666,11 +672,15 @@ void ReplaceSelectionCommand::doApply()
         bool insertionBlockIsRoot = insertionBlock == insertionRoot;
         VisiblePosition visibleInsertionPos(insertionPos);
         fragment.removeNode(refNode);
-        if (!insertionBlockIsRoot && fragment.isBlockFlow(refNode.get()) && isStartOfBlock(visibleInsertionPos))
+        // FIXME: The first two cases need to be rethought.  They're about preventing the nesting of 
+        // incoming blocks in the block where the paste is being performed.  But, avoiding nesting doesn't 
+        // always produce the desired visual result, and the decisions are based on isBlockFlow, which 
+        // we're getting rid of.
+        if (!insertionBlockIsRoot && fragment.isBlockFlow(refNode.get()) && isStartOfBlock(visibleInsertionPos) && !m_lastNodeInserted)
             insertNodeBeforeAndUpdateNodesInserted(refNode.get(), insertionBlock);
-        else if (!insertionBlockIsRoot && fragment.isBlockFlow(refNode.get()) && isEndOfBlock(visibleInsertionPos)) {
+        else if (!insertionBlockIsRoot && fragment.isBlockFlow(refNode.get()) && isEndOfBlock(visibleInsertionPos))
             insertNodeAfterAndUpdateNodesInserted(refNode.get(), insertionBlock);
-        else if (m_lastNodeInserted && !fragment.isBlockFlow(refNode.get())) {
+        else if (m_lastNodeInserted && !fragment.isBlockFlow(refNode.get())) {
             // A non-null m_lastNodeInserted means we've done merging above.  That means everything in the first paragraph 
             // of the fragment has been merged with everything up to the start of the paragraph where the paste was performed.  
             // refNode is the first node in the second paragraph of the fragment to paste.  Since it's inline, we can't