<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[210833] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/210833">210833</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2017-01-17 16:14:37 -0800 (Tue, 17 Jan 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Document title changed twice when setting document.title
https://bugs.webkit.org/show_bug.cgi?id=167065
Reviewed by Darin Adler.
Source/WebCore:
Setting document.title would call the document title to be set twice
first to the empty string and then to the new title. This is because
setting document.title is equivalent to setting title.textContent [1],
which first removes all children and then inserts the new one [2], and
we call updateTitle() for each step. This is because
HTMLTitleElement::childrenChanged() is called twice (once for the
removal of the existing children, and a second time when the new child
is inserted), and childrenChanged() calls document::titleElementTextChanged().
Since no JS event is fired between those 2 mutations, it is safe (i.e. non
observable from JS) to update the title only once after both mutations have
taken place. To achieve this, add a new replaceAllChildren() function
which implements [3]. This replaceAllChildren() has the benefit of
calling ContainerNode::childrenChanged() only once, after both mutations
have taken place, thus avoiding unnecessary work. This fixes the issue
when setting the title and should be performance-positive in general.
[1] https://html.spec.whatwg.org/#document.title
[2] https://dom.spec.whatwg.org/#dom-node-textcontent
[3] https://dom.spec.whatwg.org/#concept-node-replace-all
Test: fast/dom/Node/textContent-mutationEvents.html
* dom/ContainerNode.cpp:
(WebCore::ContainerNode::notifyChildInserted):
(WebCore::ContainerNode::removeChild):
(WebCore::ContainerNode::replaceAllChildren):
(WebCore::ContainerNode::rebuildSVGExtensionsElementsIfNecessary):
(WebCore::ContainerNode::removeChildren):
(WebCore::ContainerNode::updateTreeAfterInsertion):
* dom/ContainerNode.h:
Add new replaceAllChildrenWith() function which implements [3]
in a way that calls ContainerNode::childrenChanged() only once.
* dom/Element.cpp:
(WebCore::Element::childrenChanged):
Deal with new AllChildrenReplaced ChildChange type.
* dom/Node.cpp:
(WebCore::Node::setTextContent):
Call replaceAllChildrenWith() as per the specification:
- https://dom.spec.whatwg.org/#dom-node-textcontent
* dom/Range.cpp:
(WebCore::Range::surroundContents):
Call replaceAllChildrenWith(nullptr) as per the specification:
- https://dom.spec.whatwg.org/#dom-range-surroundcontents
Tools:
Add WebKit2GTK API test that was written by Michael Catanzaro.
* TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebView.cpp:
(testWebViewTitleChange):
(beforeAll):
LayoutTests:
* fast/dom/Node/textContent-mutationEvents-expected.txt: Added.
* fast/dom/Node/textContent-mutationEvents.html: Added.
Add layout test to make sure that the mutation events are properly
fired when setting Node.textContent.
* fast/dom/title-text-property-2-expected.txt:
* fast/dom/title-text-property-2.html:
* fast/dom/title-text-property-expected.txt:
* http/tests/globalhistory/history-delegate-basic-title-expected.txt:
Update / rebaseline existing tests now that we no longer temporarily
reset document.title to the empty string when overriding the title.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsfastdomtitletextproperty2expectedtxt">trunk/LayoutTests/fast/dom/title-text-property-2-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastdomtitletextproperty2html">trunk/LayoutTests/fast/dom/title-text-property-2.html</a></li>
<li><a href="#trunkLayoutTestsfastdomtitletextpropertyexpectedtxt">trunk/LayoutTests/fast/dom/title-text-property-expected.txt</a></li>
<li><a href="#trunkLayoutTestshttptestsglobalhistoryhistorydelegatebasictitleexpectedtxt">trunk/LayoutTests/http/tests/globalhistory/history-delegate-basic-title-expected.txt</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomContainerNodecpp">trunk/Source/WebCore/dom/ContainerNode.cpp</a></li>
<li><a href="#trunkSourceWebCoredomContainerNodeh">trunk/Source/WebCore/dom/ContainerNode.h</a></li>
<li><a href="#trunkSourceWebCoredomElementcpp">trunk/Source/WebCore/dom/Element.cpp</a></li>
<li><a href="#trunkSourceWebCoredomNodecpp">trunk/Source/WebCore/dom/Node.cpp</a></li>
<li><a href="#trunkSourceWebCoredomRangecpp">trunk/Source/WebCore/dom/Range.cpp</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsWebKit2GtkTestWebKitWebViewcpp">trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebView.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastdomNodetextContentmutationEventsexpectedtxt">trunk/LayoutTests/fast/dom/Node/textContent-mutationEvents-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastdomNodetextContentmutationEventshtml">trunk/LayoutTests/fast/dom/Node/textContent-mutationEvents.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/LayoutTests/ChangeLog 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -1,3 +1,22 @@
</span><ins>+2017-01-17 Chris Dumez <cdumez@apple.com>
+
+ Document title changed twice when setting document.title
+ https://bugs.webkit.org/show_bug.cgi?id=167065
+
+ Reviewed by Darin Adler.
+
+ * fast/dom/Node/textContent-mutationEvents-expected.txt: Added.
+ * fast/dom/Node/textContent-mutationEvents.html: Added.
+ Add layout test to make sure that the mutation events are properly
+ fired when setting Node.textContent.
+
+ * fast/dom/title-text-property-2-expected.txt:
+ * fast/dom/title-text-property-2.html:
+ * fast/dom/title-text-property-expected.txt:
+ * http/tests/globalhistory/history-delegate-basic-title-expected.txt:
+ Update / rebaseline existing tests now that we no longer temporarily
+ reset document.title to the empty string when overriding the title.
+
</ins><span class="cx"> 2017-01-17 Zalan Bujtas <zalan@apple.com>
</span><span class="cx">
</span><span class="cx"> Editing nested RTL-LTR content makes the process unresponsive.
</span></span></pre></div>
<a id="trunkLayoutTestsfastdomNodetextContentmutationEventsexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/Node/textContent-mutationEvents-expected.txt (0 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/Node/textContent-mutationEvents-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/Node/textContent-mutationEvents-expected.txt 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -0,0 +1,44 @@
</span><ins>+Tests that the DOM mutation events are fired correctly when Node.textContent is set.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+title.textContent = 'New title';
+
+* DOMNodeRemoved fired
+PASS firedDOMNodeRemoved is false
+PASS firedDOMNodeInserted is false
+PASS firedDOMSubtreeModified is false
+PASS document.title is "Original title"
+
+* DOMNodeInserted fired
+PASS firedDOMNodeRemoved is true
+PASS firedDOMNodeInserted is false
+PASS firedDOMSubtreeModified is false
+PASS document.title is "New title"
+
+* DOMSubtreeModified fired
+PASS firedDOMNodeRemoved is true
+PASS firedDOMNodeInserted is true
+PASS firedDOMSubtreeModified is false
+PASS document.title is "New title"
+
+PASS firedDOMNodeRemoved is true
+PASS firedDOMNodeInserted is true
+PASS firedDOMSubtreeModified is true
+PASS document.title is "New title"
+PASS firedMutationObserver is false
+
+* Mutation observer invoked
+PASS firedMutationObserver is false
+PASS testMutations.length is 1
+PASS mutation.type is "childList"
+PASS mutation.target is title
+PASS mutation.removedNodes.length is 1
+PASS mutation.removedNodes[0].data is "Original title"
+PASS mutation.addedNodes.length is 1
+PASS mutation.addedNodes[0].data is "New title"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastdomNodetextContentmutationEventshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/Node/textContent-mutationEvents.html (0 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/Node/textContent-mutationEvents.html (rev 0)
+++ trunk/LayoutTests/fast/dom/Node/textContent-mutationEvents.html 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -0,0 +1,79 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<head>
+<title>Original title</title>
+<script src="../../../resources/js-test-pre.js"></script>
+</head>
+<body>
+<script>
+description("Tests that the DOM mutation events are fired correctly when Node.textContent is set.");
+jsTestIsAsync = true;
+
+var firedDOMNodeRemoved = false;
+var firedDOMNodeInserted = false;
+var firedDOMSubtreeModified = false;
+var firedMutationObserver = false;
+
+var title = document.getElementsByTagName("title")[0];
+var observer = new MutationObserver(function(mutations) {
+ debug("");
+ debug("* Mutation observer invoked");
+
+ shouldBeFalse("firedMutationObserver");
+ firedMutationObserver = true;
+
+ testMutations = mutations;
+ shouldBe("testMutations.length", "1");
+ mutation = mutations[0];
+ shouldBeEqualToString("mutation.type", "childList");
+ shouldBe("mutation.target", "title");
+ shouldBe("mutation.removedNodes.length", "1");
+ shouldBeEqualToString("mutation.removedNodes[0].data", "Original title");
+ shouldBe("mutation.addedNodes.length", "1");
+ shouldBeEqualToString("mutation.addedNodes[0].data", "New title");
+
+ finishJSTest();
+});
+observer.observe(title, { childList: true });
+
+title.addEventListener("DOMNodeRemoved", function() {
+ debug("");
+ debug("* DOMNodeRemoved fired");
+ shouldBeFalse("firedDOMNodeRemoved");
+ shouldBeFalse("firedDOMNodeInserted");
+ shouldBeFalse("firedDOMSubtreeModified");
+ firedDOMNodeRemoved = true;
+ shouldBeEqualToString("document.title", "Original title");
+});
+title.addEventListener("DOMNodeInserted", function() {
+ debug("");
+ debug("* DOMNodeInserted fired");
+ shouldBeTrue("firedDOMNodeRemoved");
+ shouldBeFalse("firedDOMNodeInserted");
+ shouldBeFalse("firedDOMSubtreeModified");
+ firedDOMNodeInserted = true;
+ shouldBeEqualToString("document.title", "New title");
+});
+title.addEventListener("DOMSubtreeModified", function() {
+ debug("");
+ debug("* DOMSubtreeModified fired");
+ shouldBeTrue("firedDOMNodeRemoved");
+ shouldBeTrue("firedDOMNodeInserted");
+ shouldBeFalse("firedDOMSubtreeModified");
+ firedDOMSubtreeModified = true;
+ shouldBeEqualToString("document.title", "New title");
+});
+
+evalAndLog("title.textContent = 'New title';");
+
+debug("");
+shouldBeTrue("firedDOMNodeRemoved");
+shouldBeTrue("firedDOMNodeInserted");
+shouldBeTrue("firedDOMSubtreeModified");
+shouldBeEqualToString("document.title", "New title");
+shouldBeFalse("firedMutationObserver");
+
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastdomtitletextproperty2expectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/dom/title-text-property-2-expected.txt (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/title-text-property-2-expected.txt 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/LayoutTests/fast/dom/title-text-property-2-expected.txt 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -1,6 +1,8 @@
</span><ins>+CONSOLE MESSAGE: line 10: Setting document.title to TITLE1
+TITLE CHANGED: 'TITLE1'
+CONSOLE MESSAGE: line 15: Setting title element's text to TITLE2
+TITLE CHANGED: 'TITLE2'
+CONSOLE MESSAGE: line 19: Should not set title
+CONSOLE MESSAGE: line 25: Should reset title to the empty string
</ins><span class="cx"> TITLE CHANGED: ''
</span><del>-TITLE CHANGED: '1. setting document.title'
-TITLE CHANGED: ''
-TITLE CHANGED: '2. setting title.text'
-TITLE CHANGED: ''
</del><span class="cx">
</span></span></pre></div>
<a id="trunkLayoutTestsfastdomtitletextproperty2html"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/dom/title-text-property-2.html (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/title-text-property-2.html 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/LayoutTests/fast/dom/title-text-property-2.html 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -6,19 +6,23 @@
</span><span class="cx"> testRunner.dumpAsText();
</span><span class="cx"> testRunner.dumpTitleChanges();
</span><span class="cx"> }
</span><del>-
- document.title = '1. setting document.title';
</del><span class="cx">
</span><ins>+ console.log("Setting document.title to TITLE1");
+ document.title = 'TITLE1';
+
</ins><span class="cx"> title = document.getElementsByTagName('title').item(0);
</span><span class="cx">
</span><del>- title.text = '2. setting title.text';
</del><ins>+ console.log("Setting title element's text to TITLE2");
+ title.text = 'TITLE2';
</ins><span class="cx">
</span><span class="cx"> newTitle = document.createElement('title');
</span><del>- newTitle.appendChild(document.createTextNode('3. appending a new title node (should not trigger a title change)'));
</del><ins>+ console.log("Should not set title");
+ newTitle.appendChild(document.createTextNode('FAIL'));
</ins><span class="cx">
</span><span class="cx"> document.getElementsByTagName('head').item(0).appendChild(newTitle);
</span><del>-
</del><ins>+
</ins><span class="cx"> // Remove both title elements
</span><ins>+ console.log("Should reset title to the empty string");
</ins><span class="cx"> titleElements = document.getElementsByTagName('title');
</span><span class="cx"> while (titleElements.length) {
</span><span class="cx"> titleElement = titleElements[titleElements.length - 1];
</span></span></pre></div>
<a id="trunkLayoutTestsfastdomtitletextpropertyexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/dom/title-text-property-expected.txt (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/title-text-property-expected.txt 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/LayoutTests/fast/dom/title-text-property-expected.txt 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -1,4 +1,3 @@
</span><del>-TITLE CHANGED: ''
</del><span class="cx"> TITLE CHANGED: 'This is the new title'
</span><span class="cx"> Original title is: 'Original Title'
</span><span class="cx"> Setting new title to: 'This is the new title'
</span></span></pre></div>
<a id="trunkLayoutTestshttptestsglobalhistoryhistorydelegatebasictitleexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/http/tests/globalhistory/history-delegate-basic-title-expected.txt (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/http/tests/globalhistory/history-delegate-basic-title-expected.txt 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/LayoutTests/http/tests/globalhistory/history-delegate-basic-title-expected.txt 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -1,5 +1,4 @@
</span><span class="cx"> WebView navigated to url "http://127.0.0.1:8000/globalhistory/history-delegate-basic-title.html" with title "" with HTTP equivalent method "GET". The navigation was successful and was not a client redirect.
</span><span class="cx"> WebView updated the title for history URL "http://127.0.0.1:8000/globalhistory/history-delegate-basic-title.html" to "Test Title 1".
</span><del>-WebView updated the title for history URL "http://127.0.0.1:8000/globalhistory/history-delegate-basic-title.html" to "".
</del><span class="cx"> WebView updated the title for history URL "http://127.0.0.1:8000/globalhistory/history-delegate-basic-title.html" to "Test Title 2".
</span><span class="cx"> This test sees if the history delegate is notified of title changes.
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/Source/WebCore/ChangeLog 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -1,3 +1,58 @@
</span><ins>+2017-01-17 Chris Dumez <cdumez@apple.com>
+
+ Document title changed twice when setting document.title
+ https://bugs.webkit.org/show_bug.cgi?id=167065
+
+ Reviewed by Darin Adler.
+
+ Setting document.title would call the document title to be set twice
+ first to the empty string and then to the new title. This is because
+ setting document.title is equivalent to setting title.textContent [1],
+ which first removes all children and then inserts the new one [2], and
+ we call updateTitle() for each step. This is because
+ HTMLTitleElement::childrenChanged() is called twice (once for the
+ removal of the existing children, and a second time when the new child
+ is inserted), and childrenChanged() calls document::titleElementTextChanged().
+
+ Since no JS event is fired between those 2 mutations, it is safe (i.e. non
+ observable from JS) to update the title only once after both mutations have
+ taken place. To achieve this, add a new replaceAllChildren() function
+ which implements [3]. This replaceAllChildren() has the benefit of
+ calling ContainerNode::childrenChanged() only once, after both mutations
+ have taken place, thus avoiding unnecessary work. This fixes the issue
+ when setting the title and should be performance-positive in general.
+
+ [1] https://html.spec.whatwg.org/#document.title
+ [2] https://dom.spec.whatwg.org/#dom-node-textcontent
+ [3] https://dom.spec.whatwg.org/#concept-node-replace-all
+
+ Test: fast/dom/Node/textContent-mutationEvents.html
+
+ * dom/ContainerNode.cpp:
+ (WebCore::ContainerNode::notifyChildInserted):
+ (WebCore::ContainerNode::removeChild):
+ (WebCore::ContainerNode::replaceAllChildren):
+ (WebCore::ContainerNode::rebuildSVGExtensionsElementsIfNecessary):
+ (WebCore::ContainerNode::removeChildren):
+ (WebCore::ContainerNode::updateTreeAfterInsertion):
+ * dom/ContainerNode.h:
+ Add new replaceAllChildrenWith() function which implements [3]
+ in a way that calls ContainerNode::childrenChanged() only once.
+
+ * dom/Element.cpp:
+ (WebCore::Element::childrenChanged):
+ Deal with new AllChildrenReplaced ChildChange type.
+
+ * dom/Node.cpp:
+ (WebCore::Node::setTextContent):
+ Call replaceAllChildrenWith() as per the specification:
+ - https://dom.spec.whatwg.org/#dom-node-textcontent
+
+ * dom/Range.cpp:
+ (WebCore::Range::surroundContents):
+ Call replaceAllChildrenWith(nullptr) as per the specification:
+ - https://dom.spec.whatwg.org/#dom-range-surroundcontents
+
</ins><span class="cx"> 2017-01-17 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> ENABLE(USER_TIMING) Not Defined for Apple Windows or OS X Ports
</span></span></pre></div>
<a id="trunkSourceWebCoredomContainerNodecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ContainerNode.cpp (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ContainerNode.cpp 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/Source/WebCore/dom/ContainerNode.cpp 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -56,6 +56,7 @@
</span><span class="cx"> #include "SVGDocumentExtensions.h"
</span><span class="cx"> #include "SVGElement.h"
</span><span class="cx"> #include "SVGNames.h"
</span><ins>+#include "SVGUseElement.h"
</ins><span class="cx"> #include "SelectorQuery.h"
</span><span class="cx"> #include "TemplateContentDocumentFragment.h"
</span><span class="cx"> #include <algorithm>
</span><span class="lines">@@ -327,19 +328,26 @@
</span><span class="cx"> m_lastChild = &child;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void ContainerNode::notifyChildInserted(Node& child, ChildChangeSource source)
</del><ins>+inline auto ContainerNode::changeForChildInsertion(Node& child, ChildChangeSource source, ReplacedAllChildren replacedAllChildren) -> ChildChange
</ins><span class="cx"> {
</span><ins>+ if (replacedAllChildren == ReplacedAllChildren::Yes)
+ return { AllChildrenReplaced, nullptr, nullptr, source };
+
+ return {
+ child.isElementNode() ? ElementInserted : child.isTextNode() ? TextInserted : NonContentsChildChanged,
+ ElementTraversal::previousSibling(child),
+ ElementTraversal::nextSibling(child),
+ source
+ };
+}
+
+void ContainerNode::notifyChildInserted(Node& child, const ChildChange& change)
+{
</ins><span class="cx"> ChildListMutationScope(*this).childAdded(child);
</span><span class="cx">
</span><span class="cx"> NodeVector postInsertionNotificationTargets;
</span><span class="cx"> notifyChildNodeInserted(*this, child, postInsertionNotificationTargets);
</span><span class="cx">
</span><del>- ChildChange change;
- change.type = child.isElementNode() ? ElementInserted : child.isTextNode() ? TextInserted : NonContentsChildChanged;
- change.previousSiblingElement = ElementTraversal::previousSibling(child);
- change.nextSiblingElement = ElementTraversal::nextSibling(child);
- change.source = source;
-
</del><span class="cx"> childrenChanged(change);
</span><span class="cx">
</span><span class="cx"> for (auto& target : postInsertionNotificationTargets)
</span><span class="lines">@@ -375,7 +383,7 @@
</span><span class="cx">
</span><span class="cx"> newChild.updateAncestorConnectedSubframeCountForInsertion();
</span><span class="cx">
</span><del>- notifyChildInserted(newChild, ChildChangeSourceParser);
</del><ins>+ notifyChildInserted(newChild, changeForChildInsertion(newChild, ChildChangeSourceParser));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> ExceptionOr<void> ContainerNode::replaceChild(Node& newChild, Node& oldChild)
</span><span class="lines">@@ -534,13 +542,7 @@
</span><span class="cx"> notifyChildRemoved(child, prev, next, ChildChangeSourceAPI);
</span><span class="cx"> }
</span><span class="cx">
</span><del>-
- if (document().svgExtensions()) {
- Element* shadowHost = this->shadowHost();
- if (!shadowHost || !shadowHost->hasTagName(SVGNames::useTag))
- document().accessSVGExtensions().rebuildElements();
- }
-
</del><ins>+ rebuildSVGExtensionsElementsIfNecessary();
</ins><span class="cx"> dispatchSubtreeModifiedEvent();
</span><span class="cx">
</span><span class="cx"> return { };
</span><span class="lines">@@ -598,6 +600,65 @@
</span><span class="cx"> notifyChildRemoved(oldChild, prev, next, ChildChangeSourceParser);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+// https://dom.spec.whatwg.org/#concept-node-replace-all
+void ContainerNode::replaceAllChildren(std::nullptr_t)
+{
+ ChildListMutationScope mutation(*this);
+ removeChildren();
+}
+
+// https://dom.spec.whatwg.org/#concept-node-replace-all
+void ContainerNode::replaceAllChildren(Ref<Node>&& node)
+{
+ // This function assumes the input node is not a DocumentFragment and is parentless to decrease complexity.
+ ASSERT(!is<DocumentFragment>(node));
+ ASSERT(!node->parentNode());
+
+ if (!hasChildNodes()) {
+ // appendChildWithoutPreInsertionValidityCheck() can only throw when node has a parent and we already asserted it doesn't.
+ auto result = appendChildWithoutPreInsertionValidityCheck(node);
+ ASSERT_UNUSED(result, !result.hasException());
+ return;
+ }
+
+ Ref<ContainerNode> protectedThis(*this);
+ ChildListMutationScope mutation(*this);
+
+ // If node is not null, adopt node into parent’s node document.
+ treeScope().adoptIfNeeded(node);
+
+ // Remove all parent's children, in tree order.
+ willRemoveChildren(*this);
+
+ {
+ WidgetHierarchyUpdatesSuspensionScope suspendWidgetHierarchyUpdates;
+ {
+ NoEventDispatchAssertion assertNoEventDispatch;
+ while (RefPtr<Node> child = m_firstChild) {
+ removeBetween(nullptr, child->nextSibling(), *child);
+ notifyChildNodeRemoved(*this, *child);
+ }
+
+ // If node is not null, insert node into parent before null.
+ ASSERT(!ensurePreInsertionValidity(node, nullptr).hasException());
+ InspectorInstrumentation::willInsertDOMNode(document(), *this);
+
+ appendChildCommon(node);
+ }
+
+ updateTreeAfterInsertion(node, ReplacedAllChildren::Yes);
+ }
+
+ rebuildSVGExtensionsElementsIfNecessary();
+ dispatchSubtreeModifiedEvent();
+}
+
+inline void ContainerNode::rebuildSVGExtensionsElementsIfNecessary()
+{
+ if (document().svgExtensions() && !is<SVGUseElement>(shadowHost()))
+ document().accessSVGExtensions().rebuildElements();
+}
+
</ins><span class="cx"> // this differs from other remove functions because it forcibly removes all the children,
</span><span class="cx"> // regardless of read-only status or event exceptions, e.g.
</span><span class="cx"> void ContainerNode::removeChildren()
</span><span class="lines">@@ -626,12 +687,7 @@
</span><span class="cx"> childrenChanged(change);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- if (document().svgExtensions()) {
- Element* shadowHost = this->shadowHost();
- if (!shadowHost || !shadowHost->hasTagName(SVGNames::useTag))
- document().accessSVGExtensions().rebuildElements();
- }
-
</del><ins>+ rebuildSVGExtensionsElementsIfNecessary();
</ins><span class="cx"> dispatchSubtreeModifiedEvent();
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -709,7 +765,7 @@
</span><span class="cx">
</span><span class="cx"> newChild.updateAncestorConnectedSubframeCountForInsertion();
</span><span class="cx">
</span><del>- notifyChildInserted(newChild, ChildChangeSourceParser);
</del><ins>+ notifyChildInserted(newChild, changeForChildInsertion(newChild, ChildChangeSourceParser));
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ContainerNode::childrenChanged(const ChildChange& change)
</span><span class="lines">@@ -792,11 +848,11 @@
</span><span class="cx"> }
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void ContainerNode::updateTreeAfterInsertion(Node& child)
</del><ins>+void ContainerNode::updateTreeAfterInsertion(Node& child, ReplacedAllChildren replacedAllChildren)
</ins><span class="cx"> {
</span><span class="cx"> ASSERT(child.refCount());
</span><span class="cx">
</span><del>- notifyChildInserted(child, ChildChangeSourceAPI);
</del><ins>+ notifyChildInserted(child, changeForChildInsertion(child, ChildChangeSourceAPI, replacedAllChildren));
</ins><span class="cx">
</span><span class="cx"> dispatchChildInsertionEvents(child);
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebCoredomContainerNodeh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/ContainerNode.h (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/ContainerNode.h 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/Source/WebCore/dom/ContainerNode.h 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -53,6 +53,8 @@
</span><span class="cx"> ExceptionOr<void> replaceChild(Node& newChild, Node& oldChild);
</span><span class="cx"> WEBCORE_EXPORT ExceptionOr<void> removeChild(Node& child);
</span><span class="cx"> WEBCORE_EXPORT ExceptionOr<void> appendChild(Node& newChild);
</span><ins>+ void replaceAllChildren(Ref<Node>&&);
+ void replaceAllChildren(std::nullptr_t);
</ins><span class="cx">
</span><span class="cx"> // These methods are only used during parsing.
</span><span class="cx"> // They don't send DOM mutation events or handle reparenting.
</span><span class="lines">@@ -62,11 +64,12 @@
</span><span class="cx"> void parserInsertBefore(Node& newChild, Node& refChild);
</span><span class="cx">
</span><span class="cx"> void removeChildren();
</span><ins>+
</ins><span class="cx"> void takeAllChildrenFrom(ContainerNode*);
</span><span class="cx">
</span><span class="cx"> void cloneChildNodes(ContainerNode& clone);
</span><span class="cx">
</span><del>- enum ChildChangeType { ElementInserted, ElementRemoved, TextInserted, TextRemoved, TextChanged, AllChildrenRemoved, NonContentsChildChanged };
</del><ins>+ enum ChildChangeType { ElementInserted, ElementRemoved, TextInserted, TextRemoved, TextChanged, AllChildrenRemoved, NonContentsChildChanged, AllChildrenReplaced };
</ins><span class="cx"> enum ChildChangeSource { ChildChangeSourceParser, ChildChangeSourceAPI };
</span><span class="cx"> struct ChildChange {
</span><span class="cx"> ChildChangeType type;
</span><span class="lines">@@ -120,10 +123,13 @@
</span><span class="cx"> void insertBeforeCommon(Node& nextChild, Node& oldChild);
</span><span class="cx"> void appendChildCommon(Node&);
</span><span class="cx">
</span><del>- void notifyChildInserted(Node& child, ChildChangeSource);
</del><ins>+ void notifyChildInserted(Node& child, const ChildChange&);
</ins><span class="cx"> void notifyChildRemoved(Node& child, Node* previousSibling, Node* nextSibling, ChildChangeSource);
</span><span class="cx">
</span><del>- void updateTreeAfterInsertion(Node& child);
</del><ins>+ enum class ReplacedAllChildren { No, Yes };
+ void updateTreeAfterInsertion(Node& child, ReplacedAllChildren = ReplacedAllChildren::No);
+ static ChildChange changeForChildInsertion(Node&, ChildChangeSource, ReplacedAllChildren = ReplacedAllChildren::No);
+ void rebuildSVGExtensionsElementsIfNecessary();
</ins><span class="cx">
</span><span class="cx"> bool isContainerNode() const = delete;
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoredomElementcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Element.cpp (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Element.cpp 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/Source/WebCore/dom/Element.cpp 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -2035,6 +2035,7 @@
</span><span class="cx"> // For elements, we notify shadowRoot in Element::insertedInto and Element::removedFrom.
</span><span class="cx"> break;
</span><span class="cx"> case AllChildrenRemoved:
</span><ins>+ case AllChildrenReplaced:
</ins><span class="cx"> shadowRoot->didRemoveAllChildrenOfShadowHost();
</span><span class="cx"> break;
</span><span class="cx"> case TextInserted:
</span></span></pre></div>
<a id="trunkSourceWebCoredomNodecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Node.cpp (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Node.cpp 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/Source/WebCore/dom/Node.cpp 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -1507,12 +1507,12 @@
</span><span class="cx"> return setNodeValue(text);
</span><span class="cx"> case ELEMENT_NODE:
</span><span class="cx"> case DOCUMENT_FRAGMENT_NODE: {
</span><del>- auto container = makeRef(downcast<ContainerNode>(*this));
- ChildListMutationScope mutation(container);
- container->removeChildren();
</del><ins>+ auto& container = downcast<ContainerNode>(*this);
</ins><span class="cx"> if (text.isEmpty())
</span><del>- return { };
- return container->appendChild(document().createTextNode(text));
</del><ins>+ container.replaceAllChildren(nullptr);
+ else
+ container.replaceAllChildren(document().createTextNode(text));
+ return { };
</ins><span class="cx"> }
</span><span class="cx"> case DOCUMENT_NODE:
</span><span class="cx"> case DOCUMENT_TYPE_NODE:
</span></span></pre></div>
<a id="trunkSourceWebCoredomRangecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/Range.cpp (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/Range.cpp 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/Source/WebCore/dom/Range.cpp 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -1107,11 +1107,8 @@
</span><span class="cx"> return fragment.releaseException();
</span><span class="cx">
</span><span class="cx"> // Step 4: If newParent has children, replace all with null within newParent.
</span><del>- while (auto* child = newParent.firstChild()) {
- auto result = downcast<ContainerNode>(newParent).removeChild(*child);
- if (result.hasException())
- return result.releaseException();
- }
</del><ins>+ if (newParent.hasChildNodes())
+ downcast<ContainerNode>(newParent).replaceAllChildren(nullptr);
</ins><span class="cx">
</span><span class="cx"> // Step 5: Insert newParent into context object.
</span><span class="cx"> auto insertResult = insertNode(newParent);
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/Tools/ChangeLog 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2017-01-17 Chris Dumez <cdumez@apple.com>
+
+ Document title changed twice when setting document.title
+ https://bugs.webkit.org/show_bug.cgi?id=167065
+
+ Reviewed by Darin Adler.
+
+ Add WebKit2GTK API test that was written by Michael Catanzaro.
+
+ * TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebView.cpp:
+ (testWebViewTitleChange):
+ (beforeAll):
+
</ins><span class="cx"> 2017-01-17 Joseph Pecoraro <pecoraro@apple.com>
</span><span class="cx">
</span><span class="cx"> ENABLE(USER_TIMING) Not Defined for Apple Windows or OS X Ports
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsWebKit2GtkTestWebKitWebViewcpp"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebView.cpp (210832 => 210833)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebView.cpp 2017-01-18 00:11:30 UTC (rev 210832)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestWebKitWebView.cpp 2017-01-18 00:14:37 UTC (rev 210833)
</span><span class="lines">@@ -947,6 +947,52 @@
</span><span class="cx"> g_assert_cmpint(naturalSize.height, ==, 615);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+class WebViewTitleTest: public WebViewTest {
+public:
+ MAKE_GLIB_TEST_FIXTURE(WebViewTitleTest);
+
+ static void titleChangedCallback(WebKitWebView* view, GParamSpec*, WebViewTitleTest* test)
+ {
+ test->m_webViewTitles.append(webkit_web_view_get_title(view));
+ }
+
+ WebViewTitleTest()
+ {
+ g_signal_connect(m_webView, "notify::title", G_CALLBACK(titleChangedCallback), this);
+ }
+
+ Vector<CString> m_webViewTitles;
+};
+
+static void testWebViewTitleChange(WebViewTitleTest* test, gconstpointer)
+{
+ g_assert_cmpint(test->m_webViewTitles.size(), ==, 0);
+
+ test->loadHtml("<head><title>Page Title</title></head>", nullptr);
+ test->waitUntilLoadFinished();
+ g_assert_cmpint(test->m_webViewTitles.size(), ==, 1);
+ g_assert_cmpstr(test->m_webViewTitles[0].data(), ==, "Page Title");
+
+ test->loadHtml("<head><title>Another Page Title</title></head>", nullptr);
+ test->waitUntilLoadFinished();
+ g_assert_cmpint(test->m_webViewTitles.size(), ==, 3);
+ /* Page title should be immediately unset when loading a new page. */
+ g_assert_cmpstr(test->m_webViewTitles[1].data(), ==, "");
+ g_assert_cmpstr(test->m_webViewTitles[2].data(), ==, "Another Page Title");
+
+ test->loadHtml("<p>This page has no title!</p>", nullptr);
+ test->waitUntilLoadFinished();
+ g_assert_cmpint(test->m_webViewTitles.size(), ==, 4);
+ g_assert_cmpstr(test->m_webViewTitles[3].data(), ==, "");
+
+ test->loadHtml("<script>document.title = 'one'; document.title = 'two'; document.title = 'three';</script>", nullptr);
+ test->waitUntilLoadFinished();
+ g_assert_cmpint(test->m_webViewTitles.size(), ==, 7);
+ g_assert_cmpstr(test->m_webViewTitles[4].data(), ==, "one");
+ g_assert_cmpstr(test->m_webViewTitles[5].data(), ==, "two");
+ g_assert_cmpstr(test->m_webViewTitles[6].data(), ==, "three");
+}
+
</ins><span class="cx"> static void serverCallback(SoupServer* server, SoupMessage* message, const char* path, GHashTable*, SoupClientContext*, gpointer)
</span><span class="cx"> {
</span><span class="cx"> if (message->method != SOUP_METHOD_GET) {
</span><span class="lines">@@ -984,6 +1030,7 @@
</span><span class="cx"> IsPlayingAudioWebViewTest::add("WebKitWebView", "is-playing-audio", testWebViewIsPlayingAudio);
</span><span class="cx"> WebViewTest::add("WebKitWebView", "background-color", testWebViewBackgroundColor);
</span><span class="cx"> WebViewTest::add("WebKitWebView", "preferred-size", testWebViewPreferredSize);
</span><ins>+ WebViewTitleTest::add("WebKitWebView", "title-change", testWebViewTitleChange);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void afterAll()
</span></span></pre>
</div>
</div>
</body>
</html>