Title: [121997] releases/WebKitGTK/webkit-1.8
Revision
121997
Author
mrobin...@webkit.org
Date
2012-07-06 13:01:44 -0700 (Fri, 06 Jul 2012)

Log Message

Merge 116669 - Crash in ApplyStyleCommand::joinChildTextNodes.
https://bugs.webkit.org/show_bug.cgi?id=85939

Patch by Abhishek Arya <infe...@chromium.org> on 2012-05-10
Reviewed by Ryosuke Niwa.

Source/WebCore:

Test: editing/style/apply-style-join-child-text-nodes-crash.html

* editing/ApplyStyleCommand.cpp:
(WebCore::ApplyStyleCommand::applyRelativeFontStyleChange): add conditions
to bail out if our start and end position nodes are removed due to
mutation events in joinChildTextNodes.
(WebCore::ApplyStyleCommand::applyInlineStyle): this executes after
applyRelativeFontStyleChange in ApplyStyleCommand::doApply. So, need
to bail out if our start and end position nodes are removed due to
mutation events.
(WebCore::ApplyStyleCommand::joinChildTextNodes): hold all the children
in a ref vector to prevent them from getting destroyed due to mutation events.

LayoutTests:

* editing/style/apply-style-join-child-text-nodes-crash-expected.txt: Added.
* editing/style/apply-style-join-child-text-nodes-crash.html: Added.

Modified Paths

Added Paths

Diff

Modified: releases/WebKitGTK/webkit-1.8/LayoutTests/ChangeLog (121996 => 121997)


--- releases/WebKitGTK/webkit-1.8/LayoutTests/ChangeLog	2012-07-06 20:01:36 UTC (rev 121996)
+++ releases/WebKitGTK/webkit-1.8/LayoutTests/ChangeLog	2012-07-06 20:01:44 UTC (rev 121997)
@@ -1,3 +1,13 @@
+2012-05-10  Abhishek Arya  <infe...@chromium.org>
+
+        Crash in ApplyStyleCommand::joinChildTextNodes.
+        https://bugs.webkit.org/show_bug.cgi?id=85939
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/style/apply-style-join-child-text-nodes-crash-expected.txt: Added.
+        * editing/style/apply-style-join-child-text-nodes-crash.html: Added.
+
 2012-05-09  Abhishek Arya  <infe...@chromium.org>
 
         Crash in ReplaceSelectionCommand::performTrivialReplace

Added: releases/WebKitGTK/webkit-1.8/LayoutTests/editing/style/apply-style-join-child-text-nodes-crash-expected.txt (0 => 121997)


--- releases/WebKitGTK/webkit-1.8/LayoutTests/editing/style/apply-style-join-child-text-nodes-crash-expected.txt	                        (rev 0)
+++ releases/WebKitGTK/webkit-1.8/LayoutTests/editing/style/apply-style-join-child-text-nodes-crash-expected.txt	2012-07-06 20:01:44 UTC (rev 121997)
@@ -0,0 +1,4 @@
+PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS. WebKit didn't crash.
Property changes on: releases/WebKitGTK/webkit-1.8/LayoutTests/editing/style/apply-style-join-child-text-nodes-crash-expected.txt
___________________________________________________________________

Added: svn:eol-style

Added: releases/WebKitGTK/webkit-1.8/LayoutTests/editing/style/apply-style-join-child-text-nodes-crash.html (0 => 121997)


--- releases/WebKitGTK/webkit-1.8/LayoutTests/editing/style/apply-style-join-child-text-nodes-crash.html	                        (rev 0)
+++ releases/WebKitGTK/webkit-1.8/LayoutTests/editing/style/apply-style-join-child-text-nodes-crash.html	2012-07-06 20:01:44 UTC (rev 121997)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script id="script1">
+document.addEventListener("DOMCharacterDataModified",function(){
+    document.body.innerHTML = "PASS. WebKit didn't crash."; 
+    gc();
+    finishJSTest();
+},true);
+</script>
+<script>
+window.jsTestIsAsync = true;
+
+if (window.layoutTestController)
+    layoutTestController.waitUntilDone();
+
+var scriptElement = document.getElementById('script1');
+scriptElement.parentNode.appendChild(scriptElement.firstChild);
+scriptElement.parentNode.removeChild(scriptElement);
+document.designMode = "on";
+document.execCommand("SelectAll");
+document.execCommand("FontSizeDelta", false, 3);
+</script>
+<script src=""
+</body>
+</html>
+
Property changes on: releases/WebKitGTK/webkit-1.8/LayoutTests/editing/style/apply-style-join-child-text-nodes-crash.html
___________________________________________________________________

Added: svn:executable

Added: svn:eol-style

Modified: releases/WebKitGTK/webkit-1.8/Source/WebCore/ChangeLog (121996 => 121997)


--- releases/WebKitGTK/webkit-1.8/Source/WebCore/ChangeLog	2012-07-06 20:01:36 UTC (rev 121996)
+++ releases/WebKitGTK/webkit-1.8/Source/WebCore/ChangeLog	2012-07-06 20:01:44 UTC (rev 121997)
@@ -1,3 +1,23 @@
+2012-05-10  Abhishek Arya  <infe...@chromium.org>
+
+        Crash in ApplyStyleCommand::joinChildTextNodes.
+        https://bugs.webkit.org/show_bug.cgi?id=85939
+
+        Reviewed by Ryosuke Niwa.
+
+        Test: editing/style/apply-style-join-child-text-nodes-crash.html
+
+        * editing/ApplyStyleCommand.cpp:
+        (WebCore::ApplyStyleCommand::applyRelativeFontStyleChange): add conditions
+        to bail out if our start and end position nodes are removed due to 
+        mutation events in joinChildTextNodes.
+        (WebCore::ApplyStyleCommand::applyInlineStyle): this executes after
+        applyRelativeFontStyleChange in ApplyStyleCommand::doApply. So, need
+        to bail out if our start and end position nodes are removed due to
+        mutation events.
+        (WebCore::ApplyStyleCommand::joinChildTextNodes): hold all the children
+        in a ref vector to prevent them from getting destroyed due to mutation events.
+
 2012-05-09  Abhishek Arya  <infe...@chromium.org>
 
         Crash in ReplaceSelectionCommand::performTrivialReplace

Modified: releases/WebKitGTK/webkit-1.8/Source/WebCore/editing/ApplyStyleCommand.cpp (121996 => 121997)


--- releases/WebKitGTK/webkit-1.8/Source/WebCore/editing/ApplyStyleCommand.cpp	2012-07-06 20:01:36 UTC (rev 121996)
+++ releases/WebKitGTK/webkit-1.8/Source/WebCore/editing/ApplyStyleCommand.cpp	2012-07-06 20:01:44 UTC (rev 121997)
@@ -320,12 +320,19 @@
         start = startPosition();
         end = endPosition();
     }
+    
+    if (start.isNull() || end.isNull())
+        return;
+
     if (end.deprecatedNode()->isTextNode() && start.deprecatedNode()->parentNode() != end.deprecatedNode()->parentNode()) {
         joinChildTextNodes(end.deprecatedNode()->parentNode(), start, end);
         start = startPosition();
         end = endPosition();
     }
 
+    if (start.isNull() || end.isNull())
+        return;
+
     // Split the start text nodes if needed to apply style.
     if (isValidCaretPositionInTextNode(start)) {
         splitTextAtStart(start, end);
@@ -542,6 +549,10 @@
     // adjust to the positions we want to use for applying style
     Position start = startPosition();
     Position end = endPosition();
+
+    if (start.isNull() || end.isNull())
+        return;
+
     if (comparePositions(end, start) < 0) {
         Position swap = start;
         start = end;
@@ -1430,26 +1441,31 @@
     Position newStart = start;
     Position newEnd = end;
 
-    Node* child = node->firstChild();
-    while (child) {
-        Node* next = child->nextSibling();
-        if (child->isTextNode() && next && next->isTextNode()) {
-            Text* childText = toText(child);
-            Text* nextText = toText(next);
-            if (start.anchorType() == Position::PositionIsOffsetInAnchor && next == start.containerNode())
-                newStart = Position(childText, childText->length() + start.offsetInContainerNode());
-            if (end.anchorType() == Position::PositionIsOffsetInAnchor && next == end.containerNode())
-                newEnd = Position(childText, childText->length() + end.offsetInContainerNode());
-            String textToMove = nextText->data();
-            insertTextIntoNode(childText, childText->length(), textToMove);
-            removeNode(next);
-            // don't move child node pointer. it may want to merge with more text nodes.
-        }
-        else {
-            child = child->nextSibling();
-        }
+    Vector<RefPtr<Text> > textNodes;
+    for (Node* curr = node->firstChild(); curr; curr = curr->nextSibling()) {
+        if (!curr->isTextNode())
+            continue;
+        
+        textNodes.append(toText(curr));
     }
 
+    for (size_t i = 0; i < textNodes.size(); ++i) {
+        Text* childText = textNodes[i].get();
+        Node* next = childText->nextSibling();
+        if (!next || !next->isTextNode())
+            continue;
+    
+        Text* nextText = toText(next);
+        if (start.anchorType() == Position::PositionIsOffsetInAnchor && next == start.containerNode())
+            newStart = Position(childText, childText->length() + start.offsetInContainerNode());
+        if (end.anchorType() == Position::PositionIsOffsetInAnchor && next == end.containerNode())
+            newEnd = Position(childText, childText->length() + end.offsetInContainerNode());
+        String textToMove = nextText->data();
+        insertTextIntoNode(childText, childText->length(), textToMove);
+        removeNode(next);
+        // don't move child node pointer. it may want to merge with more text nodes.
+    }
+
     updateStartEnd(newStart, newEnd);
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to