[Shadow] Executing Italic and InsertUnorderedList in Shadow DOM causes a crash
authorshinyak@chromium.org <shinyak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Jun 2012 16:59:25 +0000 (16:59 +0000)
committershinyak@chromium.org <shinyak@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Jun 2012 16:59:25 +0000 (16:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=88495

Reviewed by Ryosuke Niwa.

Source/WebCore:

InsertionPoint::removedFrom(insertionPoint) tries to find its owner ElementShadow from
parentNode or insertionPoint. If the parent node exsits but we cannot reach ElementShadow from
the parent node, InsertionPoint::removedFrom does not try to find ElementShadow anymore.

It's OK if the ElementShadow is being destructed, but there is a case ElementShadow is not being
destructed in editing. In this case, we should try to find ElementShadow from insertionPoint.
Otherwise it will bring inconsistency to Shadow DOM, and causes a crash.

Actually checking the existence of parentNode() does not make any sense. We should get
shadowRoot() directly.

Test: editing/shadow/insertorderedlist-crash.html

* html/shadow/InsertionPoint.cpp:
(WebCore::InsertionPoint::removedFrom):

LayoutTests:

* editing/shadow/insertorderedlist-crash-expected.txt: Added.
* editing/shadow/insertorderedlist-crash.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/shadow/insertorderedlist-crash-expected.txt [new file with mode: 0644]
LayoutTests/editing/shadow/insertorderedlist-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/shadow/InsertionPoint.cpp

index f114df2..72184b3 100644 (file)
@@ -1,3 +1,13 @@
+2012-06-25  Shinya Kawanaka  <shinyak@chromium.org>
+
+        [Shadow] Executing Italic and InsertUnorderedList in Shadow DOM causes a crash
+        https://bugs.webkit.org/show_bug.cgi?id=88495
+
+        Reviewed by Ryosuke Niwa.
+
+        * editing/shadow/insertorderedlist-crash-expected.txt: Added.
+        * editing/shadow/insertorderedlist-crash.html: Added.
+
 2012-06-25  Allan Sandfeld Jensen  <allan.jensen@nokia.com>
 
         [Qt] Unskip 3D transform tests.
diff --git a/LayoutTests/editing/shadow/insertorderedlist-crash-expected.txt b/LayoutTests/editing/shadow/insertorderedlist-crash-expected.txt
new file mode 100644 (file)
index 0000000..d713963
--- /dev/null
@@ -0,0 +1,3 @@
+This test confirms some combination of editing command with Shadow DOM does not cause a crash. To test manually, select from (before nested) to (after nested), then press Italic, and InsertUnorderedList.
+
+PASS
diff --git a/LayoutTests/editing/shadow/insertorderedlist-crash.html b/LayoutTests/editing/shadow/insertorderedlist-crash.html
new file mode 100644 (file)
index 0000000..31afc57
--- /dev/null
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../fast/dom/shadow/resources/polyfill.js"></script>
+
+<p>This test confirms some combination of editing command with Shadow DOM does not cause a crash.
+To test manually, select from (before nested) to (after nested), then press Italic, and InsertUnorderedList.</p>
+
+<div id="container" contenteditable>
+    <div><p>(before host)</p></div>
+    <div id="host">    <span contenteditable="false">not editable</span></div>
+    <div>(after host)</div>
+
+    <input type="button" value="Italic" onclick="document.execCommand('Italic')" />
+    <input type="button" value="InsertUnorderedList" onclick="document.execCommand('InsertUnorderedList')" />
+</div>
+
+<script>
+if (window.layoutTestController)
+    layoutTestController.dumpAsText();
+
+var shadowRoot = new WebKitShadowRoot(host);
+var div = document.createElement('div');
+shadowRoot.appendChild(div);
+div.innerHTML = "<span contenteditable>(before shadow)</span><shadow></shadow>(after shadow)";
+
+var nestedShadowRoot = new WebKitShadowRoot(div);
+nestedShadowRoot.innerHTML = "<div contenteditable>(before <span id='src'></span>nested)<shadow></shadow>(nested <span id='dst'></span>after)</div>";
+
+var src = nestedShadowRoot.getElementById('src');
+var dst = nestedShadowRoot.getElementById('dst');
+
+if (window.eventSender) {
+    eventSender.mouseMoveTo(src.offsetLeft + src.offsetWidth / 2, src.offsetTop + src.offsetHeight / 2);
+    eventSender.mouseDown();
+    eventSender.mouseMoveTo(dst.offsetLeft + dst.offsetWidth / 2, dst.offsetTop + dst.offsetHeight / 2);
+    eventSender.mouseUp();
+
+    document.execCommand('Italic');
+    document.execCommand('InsertUnorderedList');
+
+    container.innerHTML = "PASS";
+}
+
+var successfullyParsed = true;
+</script>
+
+<script src="../../js/resources/js-test-post.js"></script>
+</body>
+</html>
index e7bd03f..af7c740 100644 (file)
@@ -1,3 +1,26 @@
+2012-06-25  Shinya Kawanaka  <shinyak@chromium.org>
+
+        [Shadow] Executing Italic and InsertUnorderedList in Shadow DOM causes a crash
+        https://bugs.webkit.org/show_bug.cgi?id=88495
+
+        Reviewed by Ryosuke Niwa.
+
+        InsertionPoint::removedFrom(insertionPoint) tries to find its owner ElementShadow from
+        parentNode or insertionPoint. If the parent node exsits but we cannot reach ElementShadow from
+        the parent node, InsertionPoint::removedFrom does not try to find ElementShadow anymore.
+
+        It's OK if the ElementShadow is being destructed, but there is a case ElementShadow is not being
+        destructed in editing. In this case, we should try to find ElementShadow from insertionPoint.
+        Otherwise it will bring inconsistency to Shadow DOM, and causes a crash.
+
+        Actually checking the existence of parentNode() does not make any sense. We should get
+        shadowRoot() directly.
+
+        Test: editing/shadow/insertorderedlist-crash.html
+
+        * html/shadow/InsertionPoint.cpp:
+        (WebCore::InsertionPoint::removedFrom):
+
 2012-06-25  Kinuko Yasuda  <kinuko@chromium.org>
 
         Remove responseBlob field from XMLHttpResponse.idl
index 5805128..0794c34 100644 (file)
@@ -128,14 +128,13 @@ Node::InsertionNotificationRequest InsertionPoint::insertedInto(ContainerNode* i
 void InsertionPoint::removedFrom(ContainerNode* insertionPoint)
 {
     if (insertionPoint->inDocument()) {
-        Node* parent = parentNode();
-        if (!parent)
-            parent = insertionPoint;
-        if (ShadowRoot* root = parent->shadowRoot()) {
-            // host can be null when removedFrom() is called from ElementShadow destructor.
-            if (root->host())
-                root->owner()->invalidateDistribution();
-        }
+        ShadowRoot* root = shadowRoot();
+        if (!root)
+            root = insertionPoint->shadowRoot();
+
+        // host can be null when removedFrom() is called from ElementShadow destructor.
+        if (root && root->host())
+            root->owner()->invalidateDistribution();
 
         // Since this insertion point is no longer visible from the shadow subtree, it need to clean itself up.
         clearDistribution();