WebCore:
authorharrison <harrison@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Sep 2007 01:03:00 +0000 (01:03 +0000)
committerharrison <harrison@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 11 Sep 2007 01:03:00 +0000 (01:03 +0000)
        Reviewed by Kevin and Tristan.

        Tests added:
        * editing/pasteboard/paste-into-anchor-text.html: Added.
        * editing/pasteboard/paste-table-cells.html: Added.

        Source changes:
        * editing/CompositeEditCommand.cpp:
        (WebCore::CompositeEditCommand::positionAvoidingSpecialElementBoundary):
        Nil check enclosingAnchor.

        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::removeNodeAndPruneAncestors):
        New. Keeps m_firstNodeInserted and m_lastLeafInserted updated.

        (WebCore::ReplaceSelectionCommand::negateStyleRulesThatAffectAppearance):
        Added a comment.

        (WebCore::ReplaceSelectionCommand::removeRedundantStyles):
        Let ReplaceSelectionCommand::removeNodeAndPruneAncestors() update the nodes.

        (WebCore::ReplaceSelectionCommand::doApply):
        Pass originalVisPosBeforeEndBR to shouldRemoveEndBR()

        (WebCore::ReplaceSelectionCommand::shouldRemoveEndBR):
        Don't remove the br if nothing was inserted.

        * editing/ReplaceSelectionCommand.h:
        Add VisiblePosition parameter to shouldRemoveEndBR()

        * editing/markup.cpp:
        (WebCore::createMarkup):
        Wrap orphan tr element with a table element, just like we were doing
        for tobody elements.

LayoutTests:

        Reviewed by Kevin and Tristan.

        <rdar://problem/5456800> Mail crashes at WebCore::nextCandidate() after pasting back into a <table> multiple times

        * editing/pasteboard/paste-into-anchor-text.html: Added.
        * editing/pasteboard/paste-table-cells.html: Added.
        * editing/pasteboard/quirks-mode-br-1-expected.checksum:
        * editing/pasteboard/quirks-mode-br-1-expected.txt:
        * editing/pasteboard/quirks-mode-br-1.html:
        * platform/mac/editing/pasteboard/paste-into-anchor-text-expected.checksum: Added.
        * platform/mac/editing/pasteboard/paste-into-anchor-text-expected.png: Added.
        * platform/mac/editing/pasteboard/paste-into-anchor-text-expected.txt: Added.
        * platform/mac/editing/pasteboard/paste-table-cells-expected.checksum: Added.
        * platform/mac/editing/pasteboard/paste-table-cells-expected.png: Added.
        * platform/mac/editing/pasteboard/paste-table-cells-expected.txt: Added.

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

17 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/paste-into-anchor-text.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/paste-table-cells.html [new file with mode: 0644]
LayoutTests/editing/pasteboard/quirks-mode-br-1-expected.checksum
LayoutTests/editing/pasteboard/quirks-mode-br-1-expected.txt
LayoutTests/editing/pasteboard/quirks-mode-br-1.html
LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.checksum [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.png [new file with mode: 0644]
LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.txt [new file with mode: 0644]
WebCore/ChangeLog
WebCore/editing/CompositeEditCommand.cpp
WebCore/editing/ReplaceSelectionCommand.cpp
WebCore/editing/ReplaceSelectionCommand.h
WebCore/editing/markup.cpp

index 28a281d..2e8dbb0 100644 (file)
@@ -1,3 +1,21 @@
+2007-09-10  David Harrison  <harrison@apple.com>
+
+        Reviewed by Kevin and Tristan.
+
+        <rdar://problem/5456800> Mail crashes at WebCore::nextCandidate() after pasting back into a <table> multiple times
+
+        * editing/pasteboard/paste-into-anchor-text.html: Added.
+        * editing/pasteboard/paste-table-cells.html: Added.
+        * editing/pasteboard/quirks-mode-br-1-expected.checksum:
+        * editing/pasteboard/quirks-mode-br-1-expected.txt:
+        * editing/pasteboard/quirks-mode-br-1.html:
+        * platform/mac/editing/pasteboard/paste-into-anchor-text-expected.checksum: Added.
+        * platform/mac/editing/pasteboard/paste-into-anchor-text-expected.png: Added.
+        * platform/mac/editing/pasteboard/paste-into-anchor-text-expected.txt: Added.
+        * platform/mac/editing/pasteboard/paste-table-cells-expected.checksum: Added.
+        * platform/mac/editing/pasteboard/paste-table-cells-expected.png: Added.
+        * platform/mac/editing/pasteboard/paste-table-cells-expected.txt: Added.
+
 2007-09-10  Kevin McCullough  <kmccullough@apple.com>
 
         Reviewed by Oliver.
diff --git a/LayoutTests/editing/pasteboard/paste-into-anchor-text.html b/LayoutTests/editing/pasteboard/paste-into-anchor-text.html
new file mode 100644 (file)
index 0000000..60caead
--- /dev/null
@@ -0,0 +1,41 @@
+<script>
+if (window.layoutTestController)
+     layoutTestController.dumpEditingCallbacks();
+</script>
+<script type="text/javascript" src="../editing.js"></script>
+<div contenteditable>
+<table><tbody><tr><td>
+    <table style="position: relative; z-index: 0;">
+        <tbody><tr>
+            <td>
+                <font class="Apple-style-span" color="red">
+                    <span class="Apple-style-span" style="text-decoration:underline;">
+                        <td>
+                            <a href="http://www.cantonrep.com/index.php?ID=373537&Category=17&subCategoryID=30" id=r-11i_1119206349>
+                                <img src=http://news.google.com/news?imgefp=Se6UJuKIqiUJ&imgurl=www.cantonrep.com/photos/2007/08/m_31mOhio_State.jpg width=80 height=80 alt="" border=1>
+                                <br>
+                                <font size=-2>Canton Repository (subscription)</font>
+                            </a>
+                        </td>
+                        <td valign=top>
+                            <a href="http://news.google.com/?ned=us&ncl=1119206349&hl=en&topic=s">
+                                <b id="test">all 79 news articles</b>
+                            </a>
+                        </td>
+                    </span>
+                </font>
+            </td>
+            <td valign=top>
+                <br>
+            <td>
+        </tr></tbody>
+    </table>
+</td></tr></tbody></table>
+</div>
+<script>
+var e = document.getElementById("test").firstChild;
+var s = window.getSelection();
+
+setSelectionCommand(e, 5, e, 5);
+insertHTMLCommand("hello world")
+</script>
diff --git a/LayoutTests/editing/pasteboard/paste-table-cells.html b/LayoutTests/editing/pasteboard/paste-table-cells.html
new file mode 100644 (file)
index 0000000..de35597
--- /dev/null
@@ -0,0 +1,46 @@
+<html> 
+<head>
+
+<style>
+.editing { 
+    border: 2px solid red; 
+    font-size: 24px; 
+}
+.explanation { 
+    border: 2px solid blue; 
+    padding: 12px; 
+    font-size: 24px; 
+    margin-bottom: 24px;
+}
+.scenario { margin-bottom: 16px;}
+.scenario:first-line { font-weight: bold; margin-bottom: 16px;}
+.expected-results:first-line { font-weight: bold }
+</style>
+<script src=../editing.js language="JavaScript" type="text/JavaScript" ></script>
+
+<script>
+function editingTest() {
+    var t1 = document.getElementById("test1");
+    var t2 = document.getElementById("test2");
+    setSelectionCommand(t1, 0, t2, 3);
+    copyCommand();
+    
+    var e = document.getElementById("target");
+    setSelectionCommand(e, 0, e, 1);
+    pasteCommand();
+}
+
+</script>
+
+<title>Editing Test</title> 
+</head> 
+<body id="root" contenteditable>
+<table><tr><td id="test1">hello</td><td id="test2">world</td></tr></table>
+<div id="target">replaceme</div>
+
+<script>
+runEditingTest();
+</script>
+
+</body>
+</html>
index 3bb43d2..546966f 100644 (file)
@@ -1 +1 @@
-25c02b740ed1e07d4119a21c57f8d992
\ No newline at end of file
+dba7fc10054f5af9c815ddfc425bb74a
\ No newline at end of file
index 729aeec..8ed8f1b 100644 (file)
@@ -2,9 +2,6 @@ EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML
 EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 61 of #text > DIV > DIV > BODY > HTML > #document to 61 of #text > DIV > DIV > BODY > HTML > #document toDOMRange:range from 1 of DIV > BODY > HTML > #document to 1 of DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
-EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 layer at (0,0) size 800x600
   RenderView at (0,0) size 800x600
 layer at (0,0) size 800x600
@@ -15,11 +12,10 @@ layer at (0,0) size 800x600
           text run at (0,0) width 779: "This is a test to make sure that the final <br> in an incoming fragment is interpretted strictly in quirks mode (where brs at the"
           text run at (0,18) width 184: "end of blocks are collapsed). "
           text run at (184,18) width 509: "This test used to fail because of unrendered content to the left of the collapsed br."
-      RenderBlock {DIV} at (0,52) size 784x36
+      RenderBlock {DIV} at (0,52) size 784x18
         RenderBlock {DIV} at (0,0) size 784x18
           RenderText {#text} at (0,0) size 370x18
             text run at (0,0) width 370: "The test should add a single blank line after this paragraph."
           RenderInline {SPAN} at (0,0) size 0x18
-        RenderBlock (anonymous) at (0,18) size 784x18
-          RenderBR {BR} at (0,0) size 0x18
-caret: position 0 of child 1 {BR} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
+          RenderBR {BR} at (370,14) size 0x0
+caret: position 61 of child 0 {#text} of child 0 {DIV} of child 2 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index b11a7bd..96dc582 100644 (file)
@@ -11,5 +11,4 @@ var e = document.getElementById("test");
 
 s.setPosition(e, 0);
 s.modify("move", "forward", "line");
-document.execCommand("InsertHTML", false, "<br>");
-</script>
\ No newline at end of file
+</script>
diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.checksum b/LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.checksum
new file mode 100644 (file)
index 0000000..4fe22a5
--- /dev/null
@@ -0,0 +1 @@
+3e980fa43ba47867ac50d6e310e1f98f
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.png b/LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.png
new file mode 100644 (file)
index 0000000..98917d7
Binary files /dev/null and b/LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.txt b/LayoutTests/platform/mac/editing/pasteboard/paste-into-anchor-text-expected.txt
new file mode 100644 (file)
index 0000000..d7f8dca
--- /dev/null
@@ -0,0 +1,57 @@
+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: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 11 of #text > A > B > TD > TR > TBODY > TABLE > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 11 of #text > A > B > TD > TR > TBODY > TABLE > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+layer at (0,0) size 800x600
+  RenderView 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 {DIV} at (0,0) size 784x116
+        RenderTable {TABLE} at (0,0) size 476x116
+          RenderTableSection {TBODY} at (0,0) size 476x116
+            RenderTableRow {TR} at (0,2) size 476x112
+              RenderTableCell {TD} at (2,2) size 472x112 [r=0 c=0 rs=1 cs=1]
+layer at (11,11) size 470x110
+  RenderTable {TABLE} at (1,1) size 470x110
+    RenderTableSection {TBODY} at (0,0) size 470x110
+      RenderTableRow {TR} at (0,2) size 470x106
+        RenderTableCell {TD} at (2,54) size 2x2 [r=0 c=0 rs=1 cs=1]
+          RenderInline {FONT} at (0,0) size 0x0 [color=#FF0000]
+            RenderText {#text} at (0,0) size 0x0
+            RenderInline {SPAN} at (0,0) size 0x0
+              RenderText {#text} at (0,0) size 0x0
+        RenderTableCell {TD} at (6,2) size 136x106 [r=0 c=1 rs=1 cs=1]
+          RenderInline {A} at (0,0) size 138x36 [color=#0000EE]
+            RenderText {#text} at (0,0) size 0x0
+            RenderImage {IMG} at (1,1) size 82x82 [border: (1px solid #0000EE)]
+            RenderText {#text} at (83,69) size 4x18
+              text run at (83,69) width 4: " "
+            RenderBR {BR} at (0,0) size 0x0
+            RenderInline {FONT} at (0,0) size 134x13
+              RenderText {#text} at (1,91) size 134x13
+                text run at (1,91) width 134: "Canton Repository (subscription)"
+            RenderText {#text} at (135,87) size 4x18
+              text run at (135,87) width 4: "                             "
+          RenderText {#text} at (139,87) size 0x18
+            text run at (139,87) width -4: "                         "
+        RenderTableCell {TD} at (144,2) size 316x20 [r=0 c=2 rs=1 cs=1]
+          RenderInline {B} at (0,0) size 314x18
+            RenderInline {A} at (0,0) size 104x18 [color=#0000EE]
+              RenderText {#text} at (1,1) size 28x18
+                text run at (1,1) width 28: "all 7"
+              RenderText {#text} at (29,1) size 76x18
+                text run at (29,1) width 76: "hello world"
+            RenderInline {SPAN} at (0,0) size 210x18 [color=#0000EE]
+              RenderInline {B} at (0,0) size 98x18
+                RenderText {#text} at (105,1) size 98x18
+                  text run at (105,1) width 98: "9 news articles"
+              RenderText {#text} at (203,1) size 112x18
+                text run at (203,1) width 112: "                            "
+        RenderTableCell {TD} at (462,2) size 2x20 [r=0 c=3 rs=1 cs=1]
+          RenderBR {BR} at (1,1) size 0x18
+        RenderTableCell {TD} at (466,54) size 2x2 [r=0 c=4 rs=1 cs=1]
+caret: position 11 of child 1 {#text} of child 0 {A} of child 2 {B} of child 4 {TD} of child 0 {TR} of child 1 {TBODY} of child 1 {TABLE} of child 0 {TD} of child 0 {TR} of child 0 {TBODY} of child 1 {TABLE} of child 0 {DIV} of child 2 {BODY} of child 0 {HTML} of document
diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.checksum b/LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.checksum
new file mode 100644 (file)
index 0000000..67f9f34
--- /dev/null
@@ -0,0 +1 @@
+d4bb3ff5536ff9fa16cacdc3d98dff54
\ No newline at end of file
diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.png b/LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.png
new file mode 100644 (file)
index 0000000..df33901
Binary files /dev/null and b/LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.png differ
diff --git a/LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.txt b/LayoutTests/platform/mac/editing/pasteboard/paste-table-cells-expected.txt
new file mode 100644 (file)
index 0000000..a236f1a
--- /dev/null
@@ -0,0 +1,35 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of BODY > HTML > #document to 6 of BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of #text > DIV > BODY > HTML > #document to 9 of #text > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document to 5 of #text > TD > TR > TBODY > TABLE > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+layer at (0,0) size 800x600
+  RenderView 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
+      RenderTable {TABLE} at (0,0) size 78x24
+        RenderTableSection {TBODY} at (0,0) size 78x24
+          RenderTableRow {TR} at (0,2) size 78x20
+            RenderTableCell {TD} at (2,2) size 33x20 [r=0 c=0 rs=1 cs=1]
+              RenderText {#text} at (1,1) size 31x18
+                text run at (1,1) width 31: "hello"
+            RenderTableCell {TD} at (37,2) size 39x20 [r=0 c=1 rs=1 cs=1]
+              RenderText {#text} at (1,1) size 37x18
+                text run at (1,1) width 37: "world"
+      RenderBlock {DIV} at (0,24) size 784x24
+        RenderTable {TABLE} at (0,0) size 78x24
+          RenderTableSection {TBODY} at (0,0) size 78x24
+            RenderTableRow {TR} at (0,2) size 78x20
+              RenderTableCell {TD} at (2,2) size 33x20 [r=0 c=0 rs=1 cs=1]
+                RenderText {#text} at (1,1) size 31x18
+                  text run at (1,1) width 31: "hello"
+              RenderTableCell {TD} at (37,2) size 39x20 [r=0 c=1 rs=1 cs=1]
+                RenderText {#text} at (1,1) size 37x18
+                  text run at (1,1) width 37: "world"
+        RenderBlock (anonymous) at (0,24) size 784x0
+caret: position 5 of child 0 {#text} of child 1 {TD} of child 0 {TR} of child 0 {TBODY} of child 0 {TABLE} of child 3 {DIV} of child 1 {BODY} of child 0 {HTML} of document
index ca310f0..7e01cbc 100644 (file)
@@ -1,3 +1,40 @@
+2007-09-10  David Harrison  <harrison@apple.com>
+
+        Reviewed by Kevin and Tristan.
+
+        Tests added:
+        * editing/pasteboard/paste-into-anchor-text.html: Added.
+        * editing/pasteboard/paste-table-cells.html: Added.
+
+        Source changes:
+        * editing/CompositeEditCommand.cpp:
+        (WebCore::CompositeEditCommand::positionAvoidingSpecialElementBoundary):
+        Nil check enclosingAnchor.
+        
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::removeNodeAndPruneAncestors):
+        New. Keeps m_firstNodeInserted and m_lastLeafInserted updated.
+        
+        (WebCore::ReplaceSelectionCommand::negateStyleRulesThatAffectAppearance):
+        Added a comment.
+        
+        (WebCore::ReplaceSelectionCommand::removeRedundantStyles):
+        Let ReplaceSelectionCommand::removeNodeAndPruneAncestors() update the nodes.
+         
+        (WebCore::ReplaceSelectionCommand::doApply):
+        Pass originalVisPosBeforeEndBR to shouldRemoveEndBR()
+        
+        (WebCore::ReplaceSelectionCommand::shouldRemoveEndBR):
+        Don't remove the br if nothing was inserted.
+        
+        * editing/ReplaceSelectionCommand.h:
+        Add VisiblePosition parameter to shouldRemoveEndBR()
+        
+        * editing/markup.cpp:
+        (WebCore::createMarkup):
+        Wrap orphan tr element with a table element, just like we were doing
+        for tobody elements.
+
 2007-09-10  David Kilzer  <ddkilzer@apple.com>
 
         Rubberstamped by Kevin Decker.
index b2c67c0..976d0f8 100644 (file)
@@ -855,6 +855,8 @@ Position CompositeEditCommand::positionAvoidingSpecialElementBoundary(const Posi
             if (original.node() != enclosingAnchor && original.node()->parentNode() != enclosingAnchor) {
                 pushAnchorElementDown(enclosingAnchor);
                 enclosingAnchor = enclosingAnchorElement(original);
+                if (!enclosingAnchor)
+                    return original;
             }
             // Don't insert outside an anchor if doing so would skip over a line break.  It would
             // probably be safe to move the line break so that we could still avoid the anchor here.
index 671a91b..c37ddf2 100644 (file)
@@ -323,7 +323,7 @@ static bool isMailPasteAsQuotationNode(Node* node)
     return node && node->hasTagName(blockquoteTag) && node->isElementNode() && static_cast<Element*>(node)->getAttribute(classAttr) == ApplePasteAsQuotation;
 }
 
-// Virtual method used so that ReplaceSelectionCommand can update the node's it tracks.
+// Wrap CompositeEditCommand::removeNodePreservingChildren() so we can update the nodes we track
 void ReplaceSelectionCommand::removeNodePreservingChildren(Node* node)
 {
     if (m_firstNodeInserted == node)
@@ -333,6 +333,23 @@ void ReplaceSelectionCommand::removeNodePreservingChildren(Node* node)
     CompositeEditCommand::removeNodePreservingChildren(node);
 }
 
+// Wrap CompositeEditCommand::removeNodeAndPruneAncestors() so we can update the nodes we track
+void ReplaceSelectionCommand::removeNodeAndPruneAncestors(Node* node)
+{
+    // prepare in case m_firstNodeInserted and/or m_lastLeafInserted get removed
+    // FIXME: shouldn't m_lastLeafInserted be adjusted using traversePreviousNode()?
+    Node* afterFirst = m_firstNodeInserted ? m_firstNodeInserted->traverseNextSibling() : 0;
+    Node* afterLast = m_lastLeafInserted ? m_lastLeafInserted->traverseNextSibling() : 0;
+    
+    CompositeEditCommand::removeNodeAndPruneAncestors(node);
+    
+    // adjust m_firstNodeInserted and m_lastLeafInserted since either or both may have been removed
+    if (m_lastLeafInserted && !m_lastLeafInserted->inDocument())
+        m_lastLeafInserted = afterLast;
+    if (m_firstNodeInserted && !m_firstNodeInserted->inDocument())
+        m_firstNodeInserted = m_lastLeafInserted && m_lastLeafInserted->inDocument() ? afterFirst : 0;
+}
+
 bool ReplaceSelectionCommand::shouldMerge(const VisiblePosition& from, const VisiblePosition& to)
 {
     if (from.isNull() || to.isNull())
@@ -361,6 +378,9 @@ void ReplaceSelectionCommand::negateStyleRulesThatAffectAppearance()
             // There are other styles that style rules can give to style spans,
             // but these are the two important ones because they'll prevent
             // inserted content from appearing in the right paragraph.
+            // FIXME: Hyatt is concerned that selectively using display:inline will give inconsistent
+            // results. We already know one issue because td elements ignore their display property
+            // in quirks mode (which Mail.app is always in). We should look for an alternative.
             if (isBlock(e))
                 e->getInlineStyleDecl()->setProperty(CSS_PROP_DISPLAY, CSS_VAL_INLINE);
             if (e->renderer() && e->renderer()->style()->floating() != FNONE)
@@ -413,12 +433,6 @@ void ReplaceSelectionCommand::removeRedundantStyles(Node* mailBlockquoteEnclosin
 
         // Remove empty style spans.
         if (isStyleSpan(element) && !element->hasChildNodes()) {
-            if (m_firstNodeInserted == m_lastLeafInserted && m_firstNodeInserted == element)
-                m_firstNodeInserted = 0;
-            if (m_firstNodeInserted == element)
-                m_firstNodeInserted = element->traverseNextSibling();
-            if (m_lastLeafInserted == element)
-                m_lastLeafInserted = element->traverseNextSibling();
             removeNodeAndPruneAncestors(element);
             continue;
         }
@@ -547,6 +561,9 @@ void ReplaceSelectionCommand::doApply()
     // p that maps to the same visible position as p (since in the case where a br is at the end of a block and collapsed 
     // away, there are positions after the br which map to the same visible position as [br, 0]).  
     Node* endBR = insertionPos.downstream().node()->hasTagName(brTag) ? insertionPos.downstream().node() : 0;
+    VisiblePosition originalVisPosBeforeEndBR;
+    if (endBR)
+        originalVisPosBeforeEndBR = VisiblePosition(endBR, 0, DOWNSTREAM).previous();
     
     startBlock = enclosingBlock(insertionPos.node());
     
@@ -631,13 +648,14 @@ void ReplaceSelectionCommand::doApply()
     
     bool interchangeNewlineAtEnd = fragment.hasInterchangeNewlineAtEnd();
 
-    if (shouldRemoveEndBR(endBR))
+    if (shouldRemoveEndBR(endBR, originalVisPosBeforeEndBR))
         removeNodeAndPruneAncestors(endBR);
-        
+    
     if (shouldMergeStart(selectionStartWasStartOfParagraph, fragment.hasInterchangeNewlineAtStart())) {
         // Bail to avoid infinite recursion.
         if (m_movingParagraph) {
-            ASSERT_NOT_REACHED();
+            // setting display:inline does not work for td elements in quirks mode
+            ASSERT(m_firstNodeInserted->hasTagName(tdTag));
             return;
         }
         VisiblePosition destination = startOfInsertedContent.previous();
@@ -749,19 +767,24 @@ void ReplaceSelectionCommand::doApply()
     completeHTMLReplacement(lastPositionToSelect);
 }
 
-bool ReplaceSelectionCommand::shouldRemoveEndBR(Node* endBR)
+bool ReplaceSelectionCommand::shouldRemoveEndBR(Node* endBR, const VisiblePosition& originalVisPosBeforeEndBR)
 {
     if (!endBR || !endBR->inDocument())
         return false;
         
     VisiblePosition visiblePos(Position(endBR, 0));
     
-    return
-        // The br is collapsed away and so is unnecessary.
-        !document()->inStrictMode() && isEndOfBlock(visiblePos) && !isStartOfParagraph(visiblePos) ||
-        // A br that was originally holding a line open should be displaced by inserted content or turned into a line break.
-        // 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);
+    // Don't remove the br if nothing was inserted.
+    if (visiblePos.previous() == originalVisPosBeforeEndBR)
+        return false;
+    
+    // Remove the br if it is collapsed away and so is unnecessary.
+    if (!document()->inStrictMode() && isEndOfBlock(visiblePos) && !isStartOfParagraph(visiblePos))
+        return true;
+        
+    // A br that was originally holding a line open should be displaced by inserted content or turned into a line break.
+    // A br that was originally acting as a line break should still be acting as a line break, not as a placeholder.
+    return isStartOfParagraph(visiblePos) && isEndOfParagraph(visiblePos);
 }
 
 void ReplaceSelectionCommand::completeHTMLReplacement(const Position &lastPositionToSelect)
index f0875ce..71bda9d 100644 (file)
@@ -83,7 +83,7 @@ private:
     void insertNodeBeforeAndUpdateNodesInserted(Node* insertChild, Node* refChild);
 
     void updateNodesInserted(Node*);
-    bool shouldRemoveEndBR(Node*);
+    bool shouldRemoveEndBR(Node*, const VisiblePosition&);
     
     bool shouldMergeStart(bool, bool);
     bool shouldMergeEnd(bool);
@@ -95,6 +95,7 @@ private:
     void handlePasteAsQuotationNode();
     
     virtual void removeNodePreservingChildren(Node*);
+    virtual void removeNodeAndPruneAncestors(Node*);
     
     VisiblePosition positionAtStartOfInsertedContent();
     VisiblePosition positionAtEndOfInsertedContent();
index 60e3b77..02f261d 100644 (file)
@@ -588,7 +588,7 @@ DeprecatedString createMarkup(const Range* range, Vector<Node*>* nodes, EAnnotat
     Node* specialCommonAncestor = 0;
     Node* commonAncestorBlock = commonAncestor ? enclosingBlock(commonAncestor) : 0;
     if (annotate && commonAncestorBlock) {
-        if (commonAncestorBlock->hasTagName(tbodyTag)) {
+        if (commonAncestorBlock->hasTagName(tbodyTag) || commonAncestorBlock->hasTagName(trTag)) {
             Node* table = commonAncestorBlock->parentNode();
             while (table && !table->hasTagName(tableTag))
                 table = table->parentNode();