[webkit-changes] [86852] trunk

Thu, 19 May 2011 10:22:28 -0700

Title: [86852] trunk
Revision
86852
Author
[email protected]
Date
2011-05-19 10:22:24 -0700 (Thu, 19 May 2011)

Log Message

2011-05-19  Ryosuke Niwa  <[email protected]>

        Reviewed by Darin Adler.

        REGRESSION (r83322): Many crashes in Mail.app in WebCore::Node::nodeIndex
        https://bugs.webkit.org/show_bug.cgi?id=61012

        Added a test to ensure WebKit does not crash when inserting a content immediately after
        a styled element inside a Mail blockquote. Regrettably the expected result is incorrect,
        but it matches the behavior of WebKit before r83322.

        * editing/pasteboard/5065605-expected.txt: Reintroduced redundant style spans.
        * editing/pasteboard/paste-text-011-expected.txt: Ditto.
        * platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt: Ditto.
        * editing/pasteboard/paste-after-inline-style-element-expected.txt: Added.
        * editing/pasteboard/paste-after-inline-style-element.html: Added.
2011-05-19  Ryosuke Niwa  <[email protected]>

        Reviewed by Darin Adler.

        REGRESSION (r83322): Many crashes in Mail.app in WebCore::Node::nodeIndex
        https://bugs.webkit.org/show_bug.cgi?id=61012

        The crash was caused by ReplaceSelectionCommand's inserting content into a middle of the paragraph
        being moved when the insertion position's container node is the node to split to. Fixed the crash
        by not changing the insertion position in such a case.

        Unfortunately, this fix caused markup to bloat in some tests but we'll take this regression since
        it's much better than crashing.

        Test: editing/pasteboard/paste-after-inline-style-element.html

        * editing/ReplaceSelectionCommand.cpp:
        (WebCore::ReplaceSelectionCommand::doApply):

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (86851 => 86852)


--- trunk/LayoutTests/ChangeLog	2011-05-19 17:17:18 UTC (rev 86851)
+++ trunk/LayoutTests/ChangeLog	2011-05-19 17:22:24 UTC (rev 86852)
@@ -1,3 +1,20 @@
+2011-05-19  Ryosuke Niwa  <[email protected]>
+
+        Reviewed by Darin Adler.
+
+        REGRESSION (r83322): Many crashes in Mail.app in WebCore::Node::nodeIndex
+        https://bugs.webkit.org/show_bug.cgi?id=61012
+
+        Added a test to ensure WebKit does not crash when inserting a content immediately after
+        a styled element inside a Mail blockquote. Regrettably the expected result is incorrect,
+        but it matches the behavior of WebKit before r83322.
+
+        * editing/pasteboard/5065605-expected.txt: Reintroduced redundant style spans.
+        * editing/pasteboard/paste-text-011-expected.txt: Ditto.
+        * platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt: Ditto.
+        * editing/pasteboard/paste-after-inline-style-element-expected.txt: Added.
+        * editing/pasteboard/paste-after-inline-style-element.html: Added.
+
 2011-05-19  Csaba Osztrogonác  <[email protected]>
 
         [Qt] Skip failing test after r86841.

Modified: trunk/LayoutTests/editing/pasteboard/5065605-expected.txt (86851 => 86852)


--- trunk/LayoutTests/editing/pasteboard/5065605-expected.txt	2011-05-19 17:17:18 UTC (rev 86851)
+++ trunk/LayoutTests/editing/pasteboard/5065605-expected.txt	2011-05-19 17:22:24 UTC (rev 86852)
@@ -21,7 +21,7 @@
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of DIV > DIV > BODY > HTML > #document to 0 of DIV > DIV > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 24 of #text > FONT > DIV > FONT > DIV > DIV > BODY > HTML > #document to 24 of #text > FONT > DIV > FONT > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -40,9 +40,16 @@
 |   <font>
 |     class="Apple-style-span"
 |     color="#ff0000"
-|     "This text should be red."
-|   <div>
-|     <font>
+|     <span>
 |       class="Apple-style-span"
-|       color="#ff0000"
-|       "This text should be red.<#selection-caret>"
+|       style="color: rgb(0, 0, 0); "
+|       <font>
+|         class="Apple-style-span"
+|         color="#ff0000"
+|         "This text should be red."
+|     <div>
+|       style="color: rgb(0, 0, 0); "
+|       <font>
+|         class="Apple-style-span"
+|         color="#ff0000"
+|         "This text should be red.<#selection-caret>"

Added: trunk/LayoutTests/editing/pasteboard/paste-after-inline-style-element-expected.txt (0 => 86852)


--- trunk/LayoutTests/editing/pasteboard/paste-after-inline-style-element-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/paste-after-inline-style-element-expected.txt	2011-05-19 17:22:24 UTC (rev 86852)
@@ -0,0 +1,9 @@
+This test ensures WebKit does not crash when pasting content immediately after an inline style element.
+This test exhibits a bug. The inserted content should be on a separate line but it is not.
+| <blockquote>
+|   type="cite"
+|   <b>
+|     "line 1 "
+|     "line 2<#selection-caret>"
+|     " "
+|     <br>

Added: trunk/LayoutTests/editing/pasteboard/paste-after-inline-style-element.html (0 => 86852)


--- trunk/LayoutTests/editing/pasteboard/paste-after-inline-style-element.html	                        (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/paste-after-inline-style-element.html	2011-05-19 17:22:24 UTC (rev 86852)
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<body>
+<div id="test" contenteditable><blockquote type="cite"><b>line 1 <br></b>&nbsp;</blockquote></div>
+<script src=""
+<script>
+
+Markup.description('This test ensures WebKit does not crash when pasting content immediately after an inline style element.\n'
++ 'This test exhibits a bug. The inserted content should be on a separate line but it is not.');
+
+window.getSelection().setPosition(document.getElementById('test').firstChild, 1);
+document.execCommand('InsertHTML', false, '<blockquote type="cite">line 2</blockquote>');
+
+Markup.dump('test');
+
+</script>
+</body>
+</html>

Modified: trunk/LayoutTests/editing/pasteboard/paste-text-011-expected.txt (86851 => 86852)


--- trunk/LayoutTests/editing/pasteboard/paste-text-011-expected.txt	2011-05-19 17:17:18 UTC (rev 86851)
+++ trunk/LayoutTests/editing/pasteboard/paste-text-011-expected.txt	2011-05-19 17:22:24 UTC (rev 86852)
@@ -6,7 +6,7 @@
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of P > BODY > HTML > #document to 0 of P > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -29,18 +29,19 @@
 | <p>
 |   <font>
 |     face="Monaco"
-|     <p>
-|       style="font-family: Times; "
-|       <font>
-|         face="Monaco"
-|         <b>
-|           "hello"
-|     <p>
-|       style="font-family: Times; "
-|       <font>
-|         face="Monaco"
-|         <b>
-|           "there<#selection-caret>"
+|     <b>
+|       <p>
+|         style="font-family: Times; font-weight: normal; "
+|         <font>
+|           face="Monaco"
+|           <b>
+|             "hello"
+|       <p>
+|         style="font-family: Times; font-weight: normal; "
+|         <font>
+|           face="Monaco"
+|           <b>
+|             "there<#selection-caret>"
 | "

Modified: trunk/LayoutTests/platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt (86851 => 86852)


--- trunk/LayoutTests/platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt	2011-05-19 17:17:18 UTC (rev 86851)
+++ trunk/LayoutTests/platform/chromium-win/editing/pasteboard/paste-text-011-expected.txt	2011-05-19 17:22:24 UTC (rev 86852)
@@ -6,7 +6,7 @@
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: shouldInsertNode:#document-fragment replacingDOMRange:range from 0 of P > BODY > HTML > #document to 0 of P > BODY > HTML > #document givenAction:WebViewInsertActionPasted
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
-EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document to 5 of #text > B > FONT > P > B > FONT > P > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
 EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
 EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
@@ -29,18 +29,19 @@
 | <p>
 |   <font>
 |     face="Monaco"
-|     <p>
-|       style="font-family: 'times new roman'; "
-|       <font>
-|         face="Monaco"
-|         <b>
-|           "hello"
-|     <p>
-|       style="font-family: 'times new roman'; "
-|       <font>
-|         face="Monaco"
-|         <b>
-|           "there<#selection-caret>"
+|     <b>
+|       <p>
+|         style="font-family: 'times new roman'; font-weight: normal; "
+|         <font>
+|           face="Monaco"
+|           <b>
+|             "hello"
+|       <p>
+|         style="font-family: 'times new roman'; font-weight: normal; "
+|         <font>
+|           face="Monaco"
+|           <b>
+|             "there<#selection-caret>"
 | "

Modified: trunk/Source/WebCore/ChangeLog (86851 => 86852)


--- trunk/Source/WebCore/ChangeLog	2011-05-19 17:17:18 UTC (rev 86851)
+++ trunk/Source/WebCore/ChangeLog	2011-05-19 17:22:24 UTC (rev 86852)
@@ -1,3 +1,22 @@
+2011-05-19  Ryosuke Niwa  <[email protected]>
+
+        Reviewed by Darin Adler.
+
+        REGRESSION (r83322): Many crashes in Mail.app in WebCore::Node::nodeIndex
+        https://bugs.webkit.org/show_bug.cgi?id=61012
+
+        The crash was caused by ReplaceSelectionCommand's inserting content into a middle of the paragraph
+        being moved when the insertion position's container node is the node to split to. Fixed the crash
+        by not changing the insertion position in such a case.
+
+        Unfortunately, this fix caused markup to bloat in some tests but we'll take this regression since
+        it's much better than crashing.
+
+        Test: editing/pasteboard/paste-after-inline-style-element.html
+
+        * editing/ReplaceSelectionCommand.cpp:
+        (WebCore::ReplaceSelectionCommand::doApply):
+
 2011-05-19  Brady Eidson  <[email protected]>
 
         Try to fix SUPPORT_AUTOCORRECTION_PANEL build.

Modified: trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp (86851 => 86852)


--- trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp	2011-05-19 17:17:18 UTC (rev 86851)
+++ trunk/Source/WebCore/editing/ReplaceSelectionCommand.cpp	2011-05-19 17:22:24 UTC (rev 86852)
@@ -980,9 +980,10 @@
 
         // FIXME: isInlineNodeWithStyle does not check editability.
         if (RefPtr<Node> nodeToSplitTo = highestEnclosingNodeOfType(insertionPos, isInlineNodeWithStyle)) {
-            if (insertionPos.containerNode() != nodeToSplitTo)
+            if (insertionPos.containerNode() != nodeToSplitTo) {
                 nodeToSplitTo = splitTreeToNode(insertionPos.anchorNode(), nodeToSplitTo.get(), true).get();
-            insertionPos = positionInParentBeforeNode(nodeToSplitTo.get());
+                insertionPos = positionInParentBeforeNode(nodeToSplitTo.get());
+            }
         }
     }
 

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to