Web Inspector: REGRESSION(173684): Edit as HTML not working
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Aug 2015 19:36:58 +0000 (19:36 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Aug 2015 19:36:58 +0000 (19:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148268

Patch by Joseph Pecoraro <pecoraro@apple.com> on 2015-08-21
Reviewed by Chris Dumez.

Source/WebCore:

Tests: inspector/dom/getOuterHTML.html
       inspector/dom/setOuterHTML.html

* inspector/DOMPatchSupport.cpp:
(WebCore::DOMPatchSupport::innerPatchChildren):
Revert the optimization change made in r173684. The optimization changes
had a few issues. It changed the logic to potentially drop out of the
loop before all new items were processed and using a node reference
to track an index did not account for the modifications insertBefore
may have made to that node's index in the list.

LayoutTests:

* inspector/dom/getOuterHTML-expected.txt: Added.
* inspector/dom/getOuterHTML.html: Added.
* inspector/dom/setOuterHTML-expected.txt: Added.
* inspector/dom/setOuterHTML.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/inspector/dom/getOuterHTML-expected.txt [new file with mode: 0644]
LayoutTests/inspector/dom/getOuterHTML.html [new file with mode: 0644]
LayoutTests/inspector/dom/setOuterHTML-expected.txt [new file with mode: 0644]
LayoutTests/inspector/dom/setOuterHTML.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/inspector/DOMPatchSupport.cpp

index a056b9b..cc676d2 100644 (file)
@@ -1,3 +1,15 @@
+2015-08-21  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: REGRESSION(173684): Edit as HTML not working
+        https://bugs.webkit.org/show_bug.cgi?id=148268
+
+        Reviewed by Chris Dumez.
+
+        * inspector/dom/getOuterHTML-expected.txt: Added.
+        * inspector/dom/getOuterHTML.html: Added.
+        * inspector/dom/setOuterHTML-expected.txt: Added.
+        * inspector/dom/setOuterHTML.html: Added.
+
 2015-08-21  Yusuke Suzuki  <utatane.tea@gmail.com>
 
         Skip no-llint tests that fail due to running out of executable memory after r188969
diff --git a/LayoutTests/inspector/dom/getOuterHTML-expected.txt b/LayoutTests/inspector/dom/getOuterHTML-expected.txt
new file mode 100644 (file)
index 0000000..3217f17
--- /dev/null
@@ -0,0 +1,11 @@
+Test for DOM.getOuterHTML (Copy HTML).
+
+
+== Running test suite: DOM.getOuterHTML
+-- Running test case: GetOuterHTML
+Dumping outerHTML for element div#x:
+<div id="x" style="display:none">
+    <h1>A Title</h1>
+    <p>A Paragraph</p>
+</div>
+
diff --git a/LayoutTests/inspector/dom/getOuterHTML.html b/LayoutTests/inspector/dom/getOuterHTML.html
new file mode 100644 (file)
index 0000000..d0f930e
--- /dev/null
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function test()
+{
+    let suite = InspectorTest.createAsyncSuite("DOM.getOuterHTML");
+
+    suite.addTestCase({
+        name: "GetOuterHTML",
+        description: "Get the OuterHTML of the target element.",
+        test: (resolve, reject) => {
+            WebInspector.domTreeManager.requestDocument(function(documentNode) {
+                WebInspector.domTreeManager.querySelector(documentNode.id, "#x", function(nodeId) {
+                    let domNode = WebInspector.domTreeManager.nodeForId(nodeId);
+                    InspectorTest.assert(domNode, "DOMNode exists");
+                    domNode.getOuterHTML(function(error, outerHTML) {
+                        InspectorTest.expectNoError(error);
+                        InspectorTest.log("Dumping outerHTML for element div#x:");
+                        InspectorTest.log(outerHTML);
+                        resolve();
+                    });
+                });
+            });
+        }
+    });
+
+    suite.runTestCasesAndFinish();
+}
+</script>
+</head>
+<body onload="runTest()">
+    <p>Test for DOM.getOuterHTML (Copy HTML).</p>
+
+<div id="x" style="display:none">
+    <h1>A Title</h1>
+    <p>A Paragraph</p>
+</div>
+
+</body>
+</html>
diff --git a/LayoutTests/inspector/dom/setOuterHTML-expected.txt b/LayoutTests/inspector/dom/setOuterHTML-expected.txt
new file mode 100644 (file)
index 0000000..21c09d8
--- /dev/null
@@ -0,0 +1,35 @@
+Test for DOM.setOuterHTML (Edit as HTML).
+
+
+== Running test suite: DOM.setOuterHTML
+-- Running test case: OuterHTMLBefore
+<div id="x" style="display:none">
+    <h1>Original Title</h1>
+    <p>Original Paragraph</p>
+</div>
+
+-- Running test case: SetOuterHTMLRemovingElements
+-- Running test case: CheckOuterHTMLAfterRemovingElements
+PASS: The outerHTML should be what was just set.
+<div id="x"></div>
+
+-- Running test case: SetOuterHTMLAddingElements
+-- Running test case: CheckOuterHTMLAfterAddingElements
+PASS: The outerHTML should be what was just set.
+<div id="x" style="display:none; color:red">
+    <div class="container">
+        <h1>A Title</h1>
+        <p>A Paragraph</p>
+    </div>
+</div>
+
+-- Running test case: SetOuterHTMLModifyingElements
+-- Running test case: CheckOuterHTMLAfterModifyingElements
+PASS: The outerHTML should be what was just set.
+<div id="x" style="display:none; color:red">
+    <div class="container">
+        <h1>A Different Title</h1>
+        <p>A Paragraph</p>
+    </div>
+</div>
+
diff --git a/LayoutTests/inspector/dom/setOuterHTML.html b/LayoutTests/inspector/dom/setOuterHTML.html
new file mode 100644 (file)
index 0000000..aec6ffd
--- /dev/null
@@ -0,0 +1,93 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../http/tests/inspector/resources/inspector-test.js"></script>
+<script>
+function test()
+{
+    let domNode;
+    let suite = InspectorTest.createAsyncSuite("DOM.setOuterHTML");
+
+    suite.addTestCase({
+        name: "OuterHTMLBefore",
+        description: "Log the initial outerHTML of the target element.",
+        test: (resolve, reject) => {
+            domNode.getOuterHTML(function(error, outerHTML) {
+                InspectorTest.expectNoError(error);
+                InspectorTest.log(outerHTML);
+                resolve();
+            });
+        }
+    });
+
+    const emptyOuterHTML = `<div id="x"></div>`;
+
+    const newOuterHTML = `
+<div id="x" style="display:none; color:red">
+    <div class="container">
+        <h1>A Title</h1>
+        <p>A Paragraph</p>
+    </div>
+</div>`.trim();
+
+    const modifiedOuterHTML = `
+<div id="x" style="display:none; color:red">
+    <div class="container">
+        <h1>A Different Title</h1>
+        <p>A Paragraph</p>
+    </div>
+</div>`.trim();
+
+    let steps = [
+        ["RemovingElements", emptyOuterHTML],
+        ["AddingElements", newOuterHTML],
+        ["ModifyingElements", modifiedOuterHTML],
+    ];
+
+    steps.forEach(function(tuple) {
+        let [title, replacementOuterHTML] = tuple;
+        suite.addTestCase({
+            name: `SetOuterHTML${title}`,
+            description: "Change the outerHTML.",
+            test: (resolve, reject) => {
+                domNode.setOuterHTML(replacementOuterHTML, function(error, outerHTML) {
+                    InspectorTest.expectNoError(error);
+                    resolve();
+                });
+            }
+        });
+
+        suite.addTestCase({
+            name: `CheckOuterHTMLAfter${title}`,
+            description: "Log the outerHTML of the target element after changes.",
+            test: (resolve, reject) => {
+                domNode.getOuterHTML(function(error, outerHTML) {
+                    InspectorTest.expectNoError(error);
+                    InspectorTest.expectThat(outerHTML === replacementOuterHTML, "The outerHTML should be what was just set.");
+                    InspectorTest.log(outerHTML);
+                    resolve();
+                });
+            }
+        });
+    });
+
+    WebInspector.domTreeManager.requestDocument(function(documentNode) {
+        WebInspector.domTreeManager.querySelector(documentNode.id, "#x", function(nodeId) {
+            domNode = WebInspector.domTreeManager.nodeForId(nodeId);
+            InspectorTest.assert(domNode, "DOMNode exists.");
+            suite.runTestCasesAndFinish();
+        });
+    });
+}
+</script>
+</head>
+<body onload="runTest()">
+    <p>Test for DOM.setOuterHTML (Edit as HTML).</p>
+
+<div id="x" style="display:none">
+    <h1>Original Title</h1>
+    <p>Original Paragraph</p>
+</div>
+
+</body>
+</html>
index 57a7d20..9d21ae7 100644 (file)
@@ -1,3 +1,21 @@
+2015-08-21  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: REGRESSION(173684): Edit as HTML not working
+        https://bugs.webkit.org/show_bug.cgi?id=148268
+
+        Reviewed by Chris Dumez.
+
+        Tests: inspector/dom/getOuterHTML.html
+               inspector/dom/setOuterHTML.html
+
+        * inspector/DOMPatchSupport.cpp:
+        (WebCore::DOMPatchSupport::innerPatchChildren):
+        Revert the optimization change made in r173684. The optimization changes
+        had a few issues. It changed the logic to potentially drop out of the
+        loop before all new items were processed and using a node reference
+        to track an index did not account for the modifications insertBefore
+        may have made to that node's index in the list.
+
 2015-08-21  Beth Dakin  <bdakin@apple.com>
 
         HistoryItems will null CachedPages should never be left in the list of items; 
index a19ee86..c782b7b 100644 (file)
@@ -342,7 +342,7 @@ bool DOMPatchSupport::innerPatchChildren(ContainerNode* parentNode, const Vector
     }
 
     // Mark retained nodes as used, do not reuse node more than once.
-    HashSet<size_t, WTF::IntHash<size_t>, WTF::UnsignedWithZeroKeyHashTraits<size_t>>  usedOldOrdinals;
+    HashSet<size_t, WTF::IntHash<size_t>, WTF::UnsignedWithZeroKeyHashTraits<size_t>> usedOldOrdinals;
     for (size_t i = 0; i < newList.size(); ++i) {
         if (!newMap[i].first)
             continue;
@@ -374,11 +374,10 @@ bool DOMPatchSupport::innerPatchChildren(ContainerNode* parentNode, const Vector
     }
 
     // 3. Insert missing nodes.
-    Node* node = parentNode->firstChild();
-    for (unsigned i = 0; node && i < newMap.size(); ++i, node = node->nextSibling()) {
+    for (size_t i = 0; i < newMap.size(); ++i) {
         if (newMap[i].first || merges.contains(newList[i].get()))
             continue;
-        if (!insertBeforeAndMarkAsUsed(parentNode, newList[i].get(), node, ec))
+        if (!insertBeforeAndMarkAsUsed(parentNode, newList[i].get(), parentNode->traverseToChildAt(i), ec))
             return false;
     }