LayoutTests:
authorjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 May 2006 01:26:20 +0000 (01:26 +0000)
committerjusting <justing@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 11 May 2006 01:26:20 +0000 (01:26 +0000)
        Reviewed by darin

        Fixed a select-after-replacement problem:
        * editing/pasteboard/drag-drop-modifies-page-expected.checksum:
        * editing/pasteboard/drag-drop-modifies-page-expected.png:
        * editing/pasteboard/drag-drop-modifies-page-expected.txt:

        The trailing interchange newline used to be lost:
        * editing/pasteboard/paste-text-012-expected.checksum:
        * editing/pasteboard/paste-text-012-expected.png:
        * editing/pasteboard/paste-text-012-expected.txt:
        * editing/pasteboard/paste-text-012.html:
        * editing/pasteboard/paste-text-016-expected.checksum:
        * editing/pasteboard/paste-text-016-expected.png:
        * editing/pasteboard/paste-text-016-expected.txt:
        * editing/pasteboard/paste-text-017-expected.checksum:
        * editing/pasteboard/paste-text-017-expected.png:
        * editing/pasteboard/paste-text-017-expected.txt:

        Illustrates the bug fixed in smart replace whitespace handling:
        * editing/pasteboard/smart-paste-008.html
        * editing/pasteboard/smart-paste-008-expected.txt
        * editing/pasteboard/smart-paste-008-expected.png
        * editing/pasteboard/smart-paste-008-expected.checksum

WebCore:

        Reviewed by darin

        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::doApply):
        Removed the code to find out if we must later add smart replace whitespace.  We can
        wait until we've done the insertion to figure it out, and the position sampled (startPos)
        to make the decision about trailing whitespace was wrong.
        Changed the order that work is done during a paste: 1) Insert everything 2) Do one of
        the following: a) handle a trailing interchange newline, b) uncollapse the last incoming
        br if it has been collapsed because of quirks mode, c) do an end merge 3) Add smart replace
        whitespace (2 and 3 were reversed because the end merge must happen before we can know
        whether or not we need to add a trailing space).
        Don't do an end merge if the last node inserted was a br because the end merge will
        clobber it.

        (WebCore::ReplaceSelectionCommand::removeEndBRIfNeeded):
        brs that are at the end of a block and not at the start of a block are not the one brs
        that are collapsed because of quirks mode.

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

20 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/drag-drop-modifies-page-expected.checksum
LayoutTests/editing/pasteboard/drag-drop-modifies-page-expected.png
LayoutTests/editing/pasteboard/drag-drop-modifies-page-expected.txt
LayoutTests/editing/pasteboard/paste-text-012-expected.checksum
LayoutTests/editing/pasteboard/paste-text-012-expected.png
LayoutTests/editing/pasteboard/paste-text-012-expected.txt
LayoutTests/editing/pasteboard/paste-text-012.html
LayoutTests/editing/pasteboard/paste-text-016-expected.checksum
LayoutTests/editing/pasteboard/paste-text-016-expected.png
LayoutTests/editing/pasteboard/paste-text-016-expected.txt
LayoutTests/editing/pasteboard/paste-text-017-expected.checksum
LayoutTests/editing/pasteboard/paste-text-017-expected.png
LayoutTests/editing/pasteboard/paste-text-017-expected.txt
LayoutTests/editing/pasteboard/smart-paste-008-expected.checksum [new file with mode: 0644]
LayoutTests/editing/pasteboard/smart-paste-008-expected.png [new file with mode: 0644]
LayoutTests/editing/pasteboard/smart-paste-008-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/smart-paste-008.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/ReplaceSelectionCommand.cpp

index 0feeb99..4512f5f 100644 (file)
@@ -1,3 +1,30 @@
+2006-05-10  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        Fixed a select-after-replacement problem:
+        * editing/pasteboard/drag-drop-modifies-page-expected.checksum:
+        * editing/pasteboard/drag-drop-modifies-page-expected.png:
+        * editing/pasteboard/drag-drop-modifies-page-expected.txt:
+        
+        The trailing interchange newline used to be lost:
+        * editing/pasteboard/paste-text-012-expected.checksum:
+        * editing/pasteboard/paste-text-012-expected.png:
+        * editing/pasteboard/paste-text-012-expected.txt:
+        * editing/pasteboard/paste-text-012.html:
+        * editing/pasteboard/paste-text-016-expected.checksum:
+        * editing/pasteboard/paste-text-016-expected.png:
+        * editing/pasteboard/paste-text-016-expected.txt:
+        * editing/pasteboard/paste-text-017-expected.checksum:
+        * editing/pasteboard/paste-text-017-expected.png:
+        * editing/pasteboard/paste-text-017-expected.txt:
+        
+        Illustrates the bug fixed in smart replace whitespace handling:
+        * editing/pasteboard/smart-paste-008.html
+        * editing/pasteboard/smart-paste-008-expected.txt
+        * editing/pasteboard/smart-paste-008-expected.png
+        * editing/pasteboard/smart-paste-008-expected.checksum
+
 2006-05-10  Alexey Proskuryakov  <ap@nypop.com>
 
         Reviewed by Anders.
index 2d1e3ce..c056991 100644 (file)
Binary files a/LayoutTests/editing/pasteboard/drag-drop-modifies-page-expected.png and b/LayoutTests/editing/pasteboard/drag-drop-modifies-page-expected.png differ
index 99f2102..3d33130 100644 (file)
@@ -3,7 +3,7 @@ EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 6 of #text > DIV > BODY > HTML > #document to 6 of #text > DIV > BODY > HTML > #document givenAction:WebViewInsertActionDropped
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 1 of #text > DIV > BODY > HTML > #document to 1 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 5 of #text > SPAN > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -22,4 +22,5 @@ layer at (0,0) size 800x600
         RenderInline {SPAN} at (0,0) size 31x18
           RenderText {#text} at (41,0) size 31x18
             text run at (41,0) width 31: "hello"
-caret: position 1 of child 1 {#text} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+selection start: position 0 of child 1 {#text} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+selection end:   position 5 of child 0 {#text} of child 2 {SPAN} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 39e5b4d..69c2387 100644 (file)
@@ -1 +1 @@
-a2145c022462d0e0994a63e92c98db9d
\ No newline at end of file
+dcf4b064c0e03e43588e9638c3a557b0
\ No newline at end of file
index 04f39ce..c49151a 100644 (file)
Binary files a/LayoutTests/editing/pasteboard/paste-text-012-expected.png and b/LayoutTests/editing/pasteboard/paste-text-012-expected.png differ
index 5933748..abe6221 100644 (file)
@@ -1,4 +1,4 @@
-EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of BODY > HTML > #document to 10 of BODY > HTML > #document
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of BODY > HTML > #document to 12 of BODY > HTML > #document
 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -7,7 +7,8 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 0 of BLOCKQUOTE > DIV > DIV > BODY > HTML > #document to 0 of BLOCKQUOTE > 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
@@ -23,16 +24,28 @@ layer at (0,0) size 800x600
         RenderText {#text} at (252,0) size 740x36
           text run at (252,0) width 488: " \"Paste as Quotation\" in Mail just pastes (<blockquote> tag seems to be lost). "
           text run at (0,18) width 496: "Should see two boxes with blockquoted \"foo\" text, followed by an empty box."
-      RenderBlock {DIV} at (0,36) size 784x24
-      RenderBlock {DIV} at (0,60) size 784x104 [border: (2px solid #FF0000)]
+        RenderBR {BR} at (496,32) size 0x0
+      RenderBlock {P} at (0,52) size 784x54
+        RenderInline {B} at (0,0) size 749x36
+          RenderText {#text} at (0,0) size 749x36
+            text run at (0,0) width 416: "This demonstrates a bug: an extra line is added during paste. "
+            text run at (416,0) width 333: "Pasting a line of text plus an interchange newline "
+            text run at (0,18) width 210: "should result in one empty line."
+        RenderText {#text} at (210,18) size 779x36
+          text run at (210,18) width 4: " "
+          text run at (214,18) width 565: "We need to prune the empty end div in the same way that we would prune it if it had a br "
+          text run at (0,36) width 108: "propping it open."
+      RenderBlock {DIV} at (0,122) size 784x104 [border: (2px solid #FF0000)]
         RenderBlock {DIV} at (14,38) size 756x28
           RenderBlock {BLOCKQUOTE} at (40,0) size 676x28
             RenderText {#text} at (0,0) size 32x28
               text run at (0,0) width 32: "foo"
-      RenderBlock {DIV} at (0,164) size 784x104 [border: (2px solid #FF0000)]
-        RenderBlock {DIV} at (14,38) size 756x28
+      RenderBlock {DIV} at (0,226) size 784x156 [border: (2px solid #FF0000)]
+        RenderBlock {DIV} at (14,38) size 756x80
           RenderBlock {BLOCKQUOTE} at (40,0) size 676x28
             RenderText {#text} at (0,0) size 32x28
               text run at (0,0) width 32: "foo"
-      RenderBlock {DIV} at (0,268) size 784x28 [border: (2px solid #FF0000)]
-caret: position 0 of child 8 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+          RenderBlock {BLOCKQUOTE} at (40,52) size 676x28
+            RenderBR {BR} at (0,0) size 0x28
+      RenderBlock {DIV} at (0,382) size 784x28 [border: (2px solid #FF0000)]
+caret: position 0 of child 0 {BR} of child 1 {BLOCKQUOTE} of child 0 {DIV} of child 9 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 59326f0..d78abfb 100644 (file)
@@ -28,8 +28,8 @@ function editingTest() {
 <body contenteditable id="root">
 
 See this bug: <a href="rdar://problem/3918712">&lt;rdar://problem/3918712&gt;</a> "Paste as Quotation" in Mail just pastes (&lt;blockquote&gt; tag seems to be lost).
-Should see two boxes with blockquoted "foo" text, followed by an empty box.
-<div style="height: 24px"></div>
+Should see two boxes with blockquoted "foo" text, followed by an empty box.<br>
+<p><b>This demonstrates a bug: an extra line is added during paste.  Pasting a line of text plus an interchange newline should result in one empty line.</b>  We need to prune the empty end div in the same way that we would prune it if it had a br propping it open.</p>
 
 <div id="test" class="editing"><div><blockquote>foo</blockquote></div></div>
 <div class="editing"></div>
index f067bae..4fb8efd 100644 (file)
@@ -1 +1 @@
-b0627a2c344122f441226913df740d4a
\ No newline at end of file
+7d233a099920f1881928deadf93f1504
\ No newline at end of file
index 4011b29..14c5840 100644 (file)
Binary files a/LayoutTests/editing/pasteboard/paste-text-016-expected.png and b/LayoutTests/editing/pasteboard/paste-text-016-expected.png differ
index 539afb8..5602aa9 100644 (file)
@@ -18,7 +18,7 @@ EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotificatio
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 2 of P > DIV > DIV > BODY > HTML > #document to 2 of P > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > P > DIV > DIV > BODY > HTML > #document to 0 of #text > P > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of P > P > DIV > DIV > BODY > HTML > #document to 0 of P > P > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
@@ -38,9 +38,9 @@ layer at (0,0) size 800x600
         RenderText {#text} at (0,18) size 378x18
           text run at (0,18) width 378: "***TEST*** line should be second, following the first line."
       RenderBlock {DIV} at (0,36) size 784x12
-      RenderBlock {DIV} at (0,48) size 784x196
-        RenderBlock {DIV} at (0,0) size 784x196 [border: (2px solid #FF0000)]
-          RenderBlock {P} at (14,14) size 756x84
+      RenderBlock {DIV} at (0,48) size 784x224
+        RenderBlock {DIV} at (0,0) size 784x224 [border: (2px solid #FF0000)]
+          RenderBlock {P} at (14,14) size 756x112
             RenderBlock (anonymous) at (0,0) size 756x28
               RenderText {#text} at (0,0) size 319x28
                 text run at (0,0) width 319: "Should be first line of document."
@@ -48,17 +48,19 @@ layer at (0,0) size 800x600
             RenderBlock {P} at (0,28) size 756x28
               RenderText {#text} at (0,0) size 130x28
                 text run at (0,0) width 130: "***TEST***"
-            RenderBlock (anonymous) at (0,56) size 756x28
+            RenderBlock {P} at (0,56) size 756x28
+              RenderBR {BR} at (0,0) size 0x28
+            RenderBlock (anonymous) at (0,84) size 756x28
               RenderText {#text} at (0,0) size 128x28
                 text run at (0,0) width 128: "Another line."
-          RenderBlock {P} at (14,98) size 756x0
-          RenderBlock (anonymous) at (14,98) size 756x28
-            RenderText {#text} at (0,0) size 6x28
-              text run at (0,0) width 6: " "
           RenderBlock {P} at (14,126) size 756x0
           RenderBlock (anonymous) at (14,126) size 756x28
             RenderText {#text} at (0,0) size 6x28
               text run at (0,0) width 6: " "
-          RenderBlock {P} at (14,154) size 756x28
+          RenderBlock {P} at (14,154) size 756x0
+          RenderBlock (anonymous) at (14,154) size 756x28
+            RenderText {#text} at (0,0) size 6x28
+              text run at (0,0) width 6: " "
+          RenderBlock {P} at (14,182) size 756x28
             RenderBR {BR} at (0,0) size 0x28
-caret: position 0 of child 3 {#text} of child 0 {P} of child 1 {DIV} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 0 of child 0 {BR} of child 3 {P} of child 0 {P} of child 1 {DIV} of child 7 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index 975a166..5ab1392 100644 (file)
@@ -1 +1 @@
-1e05ffac22396c85428d0466c50757de
\ No newline at end of file
+2136d853988645543964c905ec7d2b08
\ No newline at end of file
index 1523d3e..53ed188 100644 (file)
Binary files a/LayoutTests/editing/pasteboard/paste-text-017-expected.png and b/LayoutTests/editing/pasteboard/paste-text-017-expected.png differ
index f2c75d3..a0f8434 100644 (file)
@@ -32,8 +32,8 @@ layer at (0,0) size 800x600
           RenderBR {BR} at (189,22) size 0x0
           RenderText {#text} at (0,28) size 490x28
             text run at (0,28) width 490: "Should see a blank line between \"two\" and \"three\""
-      RenderBlock {DIV} at (0,236) size 784x116
-        RenderBlock {DIV} at (0,0) size 784x116 [border: (2px solid #FF0000)]
+      RenderBlock {DIV} at (0,236) size 784x144
+        RenderBlock {DIV} at (0,0) size 784x144 [border: (2px solid #FF0000)]
           RenderBlock {DIV} at (2,2) size 780x28
             RenderText {#text} at (0,0) size 35x28
               text run at (0,0) width 35: "one"
@@ -43,6 +43,8 @@ layer at (0,0) size 800x600
             RenderText {#text} at (0,0) size 36x28
               text run at (0,0) width 36: "two"
           RenderBlock {DIV} at (2,86) size 780x28
+            RenderBR {BR} at (0,0) size 0x28
+          RenderBlock {DIV} at (2,114) size 780x28
             RenderText {#text} at (0,0) size 49x28
               text run at (0,0) width 49: "three"
-caret: position 5 of child 0 {#text} of child 7 {DIV} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+caret: position 0 of child 0 {BR} of child 6 {DIV} of child 1 {DIV} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/pasteboard/smart-paste-008-expected.checksum b/LayoutTests/editing/pasteboard/smart-paste-008-expected.checksum
new file mode 100644 (file)
index 0000000..ecce325
--- /dev/null
@@ -0,0 +1 @@
+77c65db21e6622657d86135c62c6b67d
\ No newline at end of file
diff --git a/LayoutTests/editing/pasteboard/smart-paste-008-expected.png b/LayoutTests/editing/pasteboard/smart-paste-008-expected.png
new file mode 100644 (file)
index 0000000..60920a6
Binary files /dev/null and b/LayoutTests/editing/pasteboard/smart-paste-008-expected.png differ
diff --git a/LayoutTests/editing/pasteboard/smart-paste-008-expected.txt b/LayoutTests/editing/pasteboard/smart-paste-008-expected.txt
new file mode 100644 (file)
index 0000000..c411976
--- /dev/null
@@ -0,0 +1,35 @@
+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: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 1 of #text > DIV > DIV > BODY > HTML > #document to 1 of #text > DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 4 of #text > DIV > DIV > BODY > HTML > #document to 4 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 784x36
+        RenderText {#text} at (0,0) size 773x36
+          text run at (0,0) width 394: "There was a bug in paste's smart replace whitespace handling. "
+          text run at (394,0) width 379: "In some cases, it used information gathered at the start of the"
+          text run at (0,18) width 722: "selection being pasted into to decide whether or not a space needed to be added to the end of the incoming content."
+      RenderBlock {P} at (0,52) size 784x36
+        RenderText {#text} at (0,0) size 765x36
+          text run at (0,0) width 548: "A smart paste is performed into a selection starting in one block and ending in another. "
+          text run at (548,0) width 217: "Spaces should surround the pasted"
+          text run at (0,18) width 37: "word."
+      RenderBlock {DIV} at (0,104) size 784x18
+        RenderBlock {DIV} at (0,0) size 784x18
+          RenderText {#text} at (0,0) size 5x18
+            text run at (0,0) width 5: "f"
+          RenderText {#text} at (5,0) size 25x18
+            text run at (5,0) width 25: " foo"
+          RenderText {#text} at (30,0) size 24x18
+            text run at (30,0) width 24: " bar"
+caret: position 4 of child 1 {#text} of child 0 {DIV} of child 4 {DIV} of child 0 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/editing/pasteboard/smart-paste-008.html b/LayoutTests/editing/pasteboard/smart-paste-008.html
new file mode 100644 (file)
index 0000000..a448bb4
--- /dev/null
@@ -0,0 +1,17 @@
+<p>There was a bug in paste's smart replace whitespace handling.  In some cases, it used information gathered at the start of the selection being pasted into to decide whether or not a space needed to be added to the end of the incoming content.</p>
+<p>A smart paste is performed into a selection starting in one block and ending in another.  Spaces should surround the pasted word.</p>
+<div id="test" contenteditable="true"><div>foo</div><div>x bar</div></div>
+
+<script type="text/javascript" src="../editing.js"></script>
+<script>
+var s = window.getSelection();
+var e = document.getElementById("test");
+
+setSelectionCommand(e, 0, e, 0);
+extendSelectionForwardByWordCommand();
+copyCommand();
+moveSelectionBackwardByCharacterCommand();
+moveSelectionForwardByCharacterCommand();
+extendSelectionForwardByLineCommand();
+pasteCommand();
+</script>
\ No newline at end of file
index 98c0f7c..cfbc4e0 100644 (file)
@@ -1,3 +1,24 @@
+2006-05-10  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by darin
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::doApply):
+        Removed the code to find out if we must later add smart replace whitespace.  We can 
+        wait until we've done the insertion to figure it out, and the position sampled (startPos)
+        to make the decision about trailing whitespace was wrong.
+        Changed the order that work is done during a paste: 1) Insert everything 2) Do one of
+        the following: a) handle a trailing interchange newline, b) uncollapse the last incoming
+        br if it has been collapsed because of quirks mode, c) do an end merge 3) Add smart replace
+        whitespace (2 and 3 were reversed because the end merge must happen before we can know
+        whether or not we need to add a trailing space).
+        Don't do an end merge if the last node inserted was a br because the end merge will 
+        clobber it.
+        
+        (WebCore::ReplaceSelectionCommand::removeEndBRIfNeeded):
+        brs that are at the end of a block and not at the start of a block are not the one brs
+        that are collapsed because of quirks mode.
+
 2006-05-10  David Hyatt  <hyatt@apple.com>
 
         Rename isSpace to treatAsSpace.  Move it and the rounding hack function into
index 69fb4d1..3910d1c 100644 (file)
@@ -609,36 +609,22 @@ void ReplaceSelectionCommand::doApply()
     if (!fragment.firstChild())
         return;
     
-    // check whether to "smart replace" needs to add leading and/or trailing space
-    bool addLeadingSpace = false;
-    bool addTrailingSpace = false;
-    // FIXME: We need the affinity for startPos and endPos, but Position::downstream
-    // and Position::upstream do not give it
-    if (m_smartReplace) {
-        VisiblePosition visiblePos = VisiblePosition(startPos, VP_DEFAULT_AFFINITY);
-        assert(visiblePos.isNotNull());
-        addLeadingSpace = startPos.leadingWhitespacePosition(VP_DEFAULT_AFFINITY, true).isNull() && !isStartOfParagraph(visiblePos);
-        if (addLeadingSpace) {
-            UChar previousChar = visiblePos.previous().characterAfter();
-            if (previousChar)
-                addLeadingSpace = !frame->isCharacterSmartReplaceExempt(previousChar, true);
-        }
-        addTrailingSpace = startPos.trailingWhitespacePosition(VP_DEFAULT_AFFINITY, true).isNull() && !isEndOfParagraph(visiblePos);
-        if (addTrailingSpace) {
-            UChar thisChar = visiblePos.characterAfter();
-            if (thisChar)
-                addTrailingSpace = !frame->isCharacterSmartReplaceExempt(thisChar, false);
-        }
-    }
-    
-    // There are five steps to adding the content: merge blocks at start, add remaining blocks,
-    // add "smart replace" space, handle trailing newline, clean up.
-    
+    // 1) Add the first paragraph of the incoming fragment, merging it with the paragraph that contained the 
+    // start of the selection that was pasted into.
+    // 2) Add everything else.
+    // 3) Restore the styles of inserted nodes (since styles were removed during the test insertion).
+    // 4) Do one of the following: a) if there was a br at the end of the fragment, expand it if it collapsed 
+    // because of quirks mode, b) merge the last paragraph of the incoming fragment with the paragraph that contained the 
+    // end of the selection that was pasted into, or c) handle an interchange newline at the end of the 
+    // incoming fragment.
+    // 5) Add spaces for smart replace.
+    // 6) Select the replacement if requested, and match style if requested.
+        
     // initially, we say the insertion point is the start of selection
     updateLayout();
     Position insertionPos = startPos;
 
-    // step 1: merge content into the start block
+    // Add the first paragraph.
     if (mergeStart) {
         RefPtr<Node> refNode = fragment.mergeStartNode();
         if (refNode) {
@@ -669,7 +655,7 @@ void ReplaceSelectionCommand::doApply()
         }
     }
     
-    // step 2 : merge everything remaining in the fragment
+    // Add everything else.
     if (fragment.firstChild()) {
         RefPtr<Node> refNode = fragment.firstChild();
         RefPtr<Node> node = refNode ? refNode->nextSibling() : 0;
@@ -718,89 +704,48 @@ void ReplaceSelectionCommand::doApply()
         updateLayout();
         insertionPos = Position(m_lastNodeInserted.get(), m_lastNodeInserted->caretMaxOffset());
     }
-
-    // step 3 : handle "smart replace" whitespace
-    if (addTrailingSpace && m_lastNodeInserted) {
-        updateLayout();
-        Position pos(m_lastNodeInserted.get(), m_lastNodeInserted->caretMaxOffset());
-        bool needsTrailingSpace = pos.trailingWhitespacePosition(VP_DEFAULT_AFFINITY, true).isNull();
-        if (needsTrailingSpace) {
-            if (m_lastNodeInserted->isTextNode()) {
-                Text *text = static_cast<Text *>(m_lastNodeInserted.get());
-                insertTextIntoNode(text, text->length(), nonBreakingSpaceString());
-                insertionPos = Position(text, text->length());
-            }
-            else {
-                RefPtr<Node> node = document()->createEditingTextNode(nonBreakingSpaceString());
-                insertNodeAfterAndUpdateNodesInserted(node.get(), m_lastNodeInserted.get());
-                insertionPos = Position(node.get(), 1);
-            }
-        }
-    }
-
-    if (addLeadingSpace && m_firstNodeInserted) {
-        updateLayout();
-        Position pos(m_firstNodeInserted.get(), 0);
-        bool needsLeadingSpace = pos.leadingWhitespacePosition(VP_DEFAULT_AFFINITY, true).isNull();
-        if (needsLeadingSpace) {
-            if (m_firstNodeInserted->isTextNode()) {
-                Text *text = static_cast<Text *>(m_firstNodeInserted.get());
-                insertTextIntoNode(text, 0, nonBreakingSpaceString());
-            } else {
-                RefPtr<Node> node = document()->createEditingTextNode(nonBreakingSpaceString());
-                insertNodeBeforeAndUpdateNodesInserted(node.get(), m_firstNodeInserted.get());
-            }
-        }
-    }
+    
+    removeEndBRIfNeeded(endBR);
+    
+    // Styles were removed during the test insertion.  Restore them.
+    if (!m_matchStyle)
+        fixupNodeStyles(fragment.nodes(), fragment.renderingInfo());
+        
+    VisiblePosition endOfInsertedContent(Position(m_lastNodeInserted.get(), maxDeepOffset(m_lastNodeInserted.get())));
+    VisiblePosition startOfInsertedContent(Position(m_firstNodeInserted.get(), 0));    
     
     Position lastPositionToSelect;
     
-    removeEndBRIfNeeded(endBR);
-
-    // step 4 : handle trailing newline
     if (fragment.hasInterchangeNewlineAtEnd()) {
-
-        if (!m_lastNodeInserted) {
-            lastPositionToSelect = endingSelection().end().downstream();
-        }
-        else {
-            VisiblePosition pos(insertionPos);
-            VisiblePosition next = pos.next();
+        VisiblePosition pos(insertionPos);
+        VisiblePosition next = pos.next();
             
-            if (!isEndOfParagraph(pos) || next.isNull() || next.rootEditableElement() != currentRoot) {
-                setEndingSelection(insertionPos, DOWNSTREAM);
-                insertParagraphSeparator();
-                next = pos.next();
+        if (endWasEndOfParagraph || !isEndOfParagraph(pos) || next.isNull() || next.rootEditableElement() != currentRoot) {
+            setEndingSelection(insertionPos, DOWNSTREAM);
+            insertParagraphSeparator();
+            next = pos.next();
 
-                // Select up to the paragraph separator that was added.
-                lastPositionToSelect = next.deepEquivalent().downstream();
-                updateNodesInserted(lastPositionToSelect.node());
-            } else {
-                // Select up to the preexising paragraph separator.
-                VisiblePosition next = pos.next();
-                lastPositionToSelect = next.deepEquivalent().downstream();
-            }
+            // Select up to the paragraph separator that was added.
+            lastPositionToSelect = next.deepEquivalent().downstream();
+            updateNodesInserted(lastPositionToSelect.node());
+        } else {
+            // Select up to the beginning of the next paragraph.
+            VisiblePosition next = pos.next();
+            lastPositionToSelect = next.deepEquivalent().downstream();
         }
-    } else {
+        
+    } else if (m_lastNodeInserted->hasTagName(brTag)) {
         // We want to honor the last incoming line break, so, if it will collapse away because of quirks mode, 
         // add an extra one.
-        // FIXME: If <div><br></div> is pasted, the br will be expanded.  That's fine, if this code is about 
-        // interpreting incoming brs strictly, but if that's true then we should expand all incoming brs, not 
-        // just the last one. 
-        if (m_lastNodeInserted && m_lastNodeInserted->hasTagName(brTag) && 
-            !document()->inStrictMode() && isEndOfBlock(VisiblePosition(Position(m_lastNodeInserted.get(), 0)))) { 
+        // FIXME: This will expand a br inside a block: <div><br></div>
+        // FIXME: Should we expand all incoming brs that collapse because of quirks mode?
+        if (!document()->inStrictMode() && isEndOfBlock(VisiblePosition(Position(m_lastNodeInserted.get(), 0))))
             insertNodeBeforeAndUpdateNodesInserted(createBreakElement(document()).get(), m_lastNodeInserted.get());
-        }
-    }
-    
-    if (!m_matchStyle)
-        fixupNodeStyles(fragment.nodes(), fragment.renderingInfo());
-    
-    // Make sure that content after the end of the selection being pasted into is in the same paragraph as the 
-    // last bit of content that was inserted.
-    VisiblePosition endOfInsertedContent(Position(m_lastNodeInserted.get(), maxDeepOffset(m_lastNodeInserted.get())));
-    VisiblePosition startOfInsertedContent(Position(m_firstNodeInserted.get(), 0));
-    if (shouldMergeEnd(endOfInsertedContent, fragment.hasInterchangeNewlineAtEnd(), endWasEndOfParagraph)) {
+            
+    } else if (shouldMergeEnd(endOfInsertedContent, fragment.hasInterchangeNewlineAtEnd(), endWasEndOfParagraph)) {
+        // Make sure that content after the end of the selection being pasted into is in the same paragraph as the 
+        // last bit of content that was inserted.
+        
         // Merging two paragraphs will destroy the moved one's block styles.  Always move forward to preserve
         // the block style of the paragraph already in the document, unless the paragraph to move would include the
         // what was the start of the selection that was pasted into.
@@ -810,13 +755,49 @@ void ReplaceSelectionCommand::doApply()
         VisiblePosition startOfParagraphToMove = mergeForward ? startOfParagraph(endOfInsertedContent) : endOfInsertedContent.next();
 
         moveParagraph(startOfParagraphToMove, endOfParagraph(startOfParagraphToMove), destination);
-        // Merging forward will remove the last node inserted from the document.
+        // Merging forward will remove m_lastNodeInserted from the document.
         // FIXME: Maintain positions for the start and end of inserted content instead of keeping nodes.  The nodes are
         // only ever used to create positions where inserted content starts/ends.
         if (mergeForward)
             m_lastNodeInserted = destination.previous().deepEquivalent().node();
     }
     
+    endOfInsertedContent = VisiblePosition(Position(m_lastNodeInserted.get(), maxDeepOffset(m_lastNodeInserted.get())));
+    startOfInsertedContent = VisiblePosition(Position(m_firstNodeInserted.get(), 0));    
+    
+    // Add spaces for smart replace.
+    if (m_smartReplace) {
+        bool needsTrailingSpace = !isEndOfParagraph(endOfInsertedContent) &&
+                                  !frame->isCharacterSmartReplaceExempt(endOfInsertedContent.characterAfter(), false);
+        if (needsTrailingSpace) {
+            if (m_lastNodeInserted->isTextNode()) {
+                Text *text = static_cast<Text *>(m_lastNodeInserted.get());
+                insertTextIntoNode(text, text->length(), nonBreakingSpaceString());
+                insertionPos = Position(text, text->length());
+            } else {
+                RefPtr<Node> node = document()->createEditingTextNode(nonBreakingSpaceString());
+                insertNodeAfterAndUpdateNodesInserted(node.get(), m_lastNodeInserted.get());
+                insertionPos = Position(node.get(), 1);
+            }
+        }
+    
+        bool needsLeadingSpace = !isStartOfParagraph(startOfInsertedContent) &&
+                                 !frame->isCharacterSmartReplaceExempt(startOfInsertedContent.previous().characterAfter(), true);
+        if (needsLeadingSpace) {
+            if (m_firstNodeInserted->isTextNode()) {
+                Text *text = static_cast<Text *>(m_firstNodeInserted.get());
+                insertTextIntoNode(text, 0, nonBreakingSpaceString());
+            } else {
+                RefPtr<Node> node = document()->createEditingTextNode(nonBreakingSpaceString());
+                // Don't updateNodesInserted.  Doing so would set m_lastNodeInserted to be the node containing the 
+                // leading space, but m_lastNodeInserted is supposed to mark the end of pasted content.
+                insertNodeBefore(node.get(), m_firstNodeInserted.get());
+                // FIXME: Use positions to track the start/end of inserted content.
+                m_firstNodeInserted = node;
+            }
+        }
+    }
+    
     completeHTMLReplacement(lastPositionToSelect);
 }
 
@@ -828,7 +809,7 @@ void ReplaceSelectionCommand::removeEndBRIfNeeded(Node* endBR)
     VisiblePosition visiblePos(Position(endBR, 0));
     
     if (// The br is collapsed away and so is unnecessary.
-        !document()->inStrictMode() && isEndOfBlock(visiblePos) && !isStartOfBlock(visiblePos) ||
+        !document()->inStrictMode() && isEndOfBlock(visiblePos) && !isStartOfParagraph(visiblePos) ||
         // A br that was originally holding a line open should be displaced by inserted content.
         // A br that was originally acting as a line break should still be acting as a line break, not as a placeholder.
         isStartOfParagraph(visiblePos) && isEndOfParagraph(visiblePos))