LayoutTests:

        Reviewed by ggaren and harrison

        <rdar://problem/4744008>
        9A270: Mail crashes when I try to paste large items from Safari

        * editing/pasteboard/4744008-expected.txt: Added.
        * editing/pasteboard/4744008.html: Added.

WebCore:

        Reviewed by ggaren and harrison

        <rdar://problem/4744008>
        9A270: Mail crashes when I try to paste large items from Safari

        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::removeRedundantStyles):
        Even though we put nodes in the hash map in pre-order, they don't necessary
        come out of the iterator that way.  If a node is a redundant style span
        and one of its pruned ancestors comes off the hash map after it, and that
        ancestor is a redundant style span or font tag, we will try to remove it
        even though it has already been removed and we'll crash.
        The test case added with the fix depends on our hash map implementation
        because it requires that a particular redundant style span come out of
        the iterator before its parent. So, it may eventually not provide
        coverage for the fix.

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

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 6057c389e770ec93054b4267ffac99d4c2bdb8f6..8ab67c01002ec98573db077a5ab02534f12db51d 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2006-10-13  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by ggaren and harrison
+        
+        <rdar://problem/4744008>
+        9A270: Mail crashes when I try to paste large items from Safari
+        
+        * editing/pasteboard/4744008-expected.txt: Added.
+        * editing/pasteboard/4744008.html: Added.
+
 2006-10-13  Geoffrey Garen  <ggaren@apple.com>
 
         Tests for accessing renderer-dependent properties from a javascript: URL.

diff --git a/LayoutTests/editing/pasteboard/4744008-expected.txt b/LayoutTests/editing/pasteboard/4744008-expected.txt
new file mode 100644 (file)
index 0000000..23eca9e
--- /dev/null
+++ b/LayoutTests/editing/pasteboard/4744008-expected.txt
@@ -0,0 +1,10 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:range from 0 of DIV > BODY > HTML > #document to 0 of DIV > BODY > HTML > #document toDOMRange:range from 3 of #text > DIV > BODY > HTML > #document to 3 of #text > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+This tests for a crash on paste inside removeRedundantStyles. You should see 'foo bar' below.
+
+foo bar
+

diff --git a/LayoutTests/editing/pasteboard/4744008.html b/LayoutTests/editing/pasteboard/4744008.html
new file mode 100644 (file)
index 0000000..42d2563
--- /dev/null
+++ b/LayoutTests/editing/pasteboard/4744008.html
@@ -0,0 +1,13 @@
+<p>This tests for a crash on paste inside removeRedundantStyles.  You should see 'foo bar' below.</p>
+<div id="div" contenteditable="true"></div>
+
+<script>
+if (window.layoutTestController)
+    window.layoutTestController.dumpAsText();
+    
+var div = document.getElementById("div");
+var sel = window.getSelection();
+
+sel.setPosition(div, 0);
+document.execCommand("InsertHTML", false, "foo <span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><span></span><font><span class='Apple-style-span'></span></font>bar");
+</script>

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index db8a8730f17b2c0cb15b53e88292b3e418509f7d..c25b2d9f661cad17018d632a77ca0c9613ee357d 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,22 @@
+2006-10-13  Justin Garcia  <justin.garcia@apple.com>
+
+        Reviewed by ggaren and harrison
+        
+        <rdar://problem/4744008>
+        9A270: Mail crashes when I try to paste large items from Safari
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::removeRedundantStyles):
+        Even though we put nodes in the hash map in pre-order, they don't necessary
+        come out of the iterator that way.  If a node is a redundant style span
+        and one of its pruned ancestors comes off the hash map after it, and that
+        ancestor is a redundant style span or font tag, we will try to remove it
+        even though it has already been removed and we'll crash.
+        The test case added with the fix depends on our hash map implementation
+        because it requires that a particular redundant style span come out of 
+        the iterator before its parent. So, it may eventually not provide 
+        coverage for the fix.
+
 2006-10-13  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Darin.

diff --git a/WebCore/editing/ReplaceSelectionCommand.cpp b/WebCore/editing/ReplaceSelectionCommand.cpp
index b1a0901524e4d9a3ffaae6f29e32a2b61ca739de..851ca7fee0e86934f41ee50536ecdb5a050babbb 100644 (file)
--- a/WebCore/editing/ReplaceSelectionCommand.cpp
+++ b/WebCore/editing/ReplaceSelectionCommand.cpp
@@ -382,7 +382,13 @@ void ReplaceSelectionCommand::removeRedundantStyles()
     
     NodeStyleMap::const_iterator e = map.end();
     for (NodeStyleMap::const_iterator it = map.begin(); it != e; ++it) {
-        Node *node = it->first;
+        Node* node = it->first;
+        
+        // This node could have aleady been removed during pruning if one 
+        // of its descendants came off of the hash map iterator before it 
+        // and that descendant was a redundant style span or font tag.
+        if (!node->inDocument())
+            continue;
         
         // Remove empty style spans.
         if (isStyleSpan(node) && !node->hasChildNodes()) {