<!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>[203389] 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/203389">203389</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2016-07-18 17:57:18 -0700 (Mon, 18 Jul 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>DocType's publicId / systemId should not be nullable
https://bugs.webkit.org/show_bug.cgi?id=159901
Reviewed by Benjamin Poulain.
LayoutTests/imported/w3c:
Rebaseline now that more checks regarding DocumentType serialization
are passing.
* web-platform-tests/domparsing/xml-serialization-expected.txt:
Source/WebCore:
DocType's publicId / systemId should not be nullable. While they were
not marked as nullable in our IDL, they could be stored as null Strings
in our implementation depending on how the Node was constructed. This
led to subtle bugs where String() != emptyString().
In particular, Node.isEqualNode() would return false when DocumentType
nodes would mismatch because of their publicId / systemId being null
instead of the emptyString.
Serialization would DocumentType nodes would also be wrong when
publicId / systemId were empty Strings instead of null strings. The
new behavior now matches:
- https://www.w3.org/TR/DOM-Parsing/#dfn-concept-serialize-doctype (steps 7-9)
To address these issues, we now always store publicId / systemId as
non-null Strings inside the DocumentType class.
Test: fast/dom/DocumentType/isEqualNode.html
* dom/DocumentType.cpp:
(WebCore::DocumentType::DocumentType):
* editing/MarkupAccumulator.cpp:
(WebCore::MarkupAccumulator::appendDocumentType):
LayoutTests:
Add test coverage for comparison of DocumentType nodes
using isEqualNode(). This tests used to fail and now passes.
The test passes in Firefox and Chrome as well.
* fast/dom/DocumentType/isEqualNode-expected.txt: Added.
* fast/dom/DocumentType/isEqualNode.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsimportedw3cChangeLog">trunk/LayoutTests/imported/w3c/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsimportedw3cwebplatformtestsdomparsingxmlserializationexpectedtxt">trunk/LayoutTests/imported/w3c/web-platform-tests/domparsing/xml-serialization-expected.txt</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoredomDocumentTypecpp">trunk/Source/WebCore/dom/DocumentType.cpp</a></li>
<li><a href="#trunkSourceWebCoreeditingMarkupAccumulatorcpp">trunk/Source/WebCore/editing/MarkupAccumulator.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastdomDocumentTypeisEqualNodeexpectedtxt">trunk/LayoutTests/fast/dom/DocumentType/isEqualNode-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastdomDocumentTypeisEqualNodehtml">trunk/LayoutTests/fast/dom/DocumentType/isEqualNode.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (203388 => 203389)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-07-19 00:47:40 UTC (rev 203388)
+++ trunk/LayoutTests/ChangeLog 2016-07-19 00:57:18 UTC (rev 203389)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2016-07-18 Chris Dumez <cdumez@apple.com>
+
+ DocType's publicId / systemId should not be nullable
+ https://bugs.webkit.org/show_bug.cgi?id=159901
+
+ Reviewed by Benjamin Poulain.
+
+ Add test coverage for comparison of DocumentType nodes
+ using isEqualNode(). This tests used to fail and now passes.
+ The test passes in Firefox and Chrome as well.
+
+ * fast/dom/DocumentType/isEqualNode-expected.txt: Added.
+ * fast/dom/DocumentType/isEqualNode.html: Added.
+
</ins><span class="cx"> 2016-07-18 Jeremy Jones <jeremyj@apple.com>
</span><span class="cx">
</span><span class="cx"> If previous media session interruptions were prevented, still allow subsequent interruptions to try.
</span></span></pre></div>
<a id="trunkLayoutTestsfastdomDocumentTypeisEqualNodeexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/DocumentType/isEqualNode-expected.txt (0 => 203389)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/DocumentType/isEqualNode-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/dom/DocumentType/isEqualNode-expected.txt 2016-07-19 00:57:18 UTC (rev 203389)
</span><span class="lines">@@ -0,0 +1,10 @@
</span><ins>+Tests that isEqualNode() works as expected for DocumentType nodes
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS htmlDocument.doctype.isEqualNode(htmlDoctype) is true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastdomDocumentTypeisEqualNodehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/dom/DocumentType/isEqualNode.html (0 => 203389)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/dom/DocumentType/isEqualNode.html (rev 0)
+++ trunk/LayoutTests/fast/dom/DocumentType/isEqualNode.html 2016-07-19 00:57:18 UTC (rev 203389)
</span><span class="lines">@@ -0,0 +1,15 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../../resources/js-test-pre.js"></script>
+<script>
+description("Tests that isEqualNode() works as expected for DocumentType nodes");
+
+var htmlDoctype = document.implementation.createDocumentType("html", "", "");
+var htmlDocument = document.implementation.createHTMLDocument();
+
+shouldBeTrue("htmlDocument.doctype.isEqualNode(htmlDoctype)");
+</script>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsimportedw3cChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/imported/w3c/ChangeLog (203388 => 203389)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/imported/w3c/ChangeLog 2016-07-19 00:47:40 UTC (rev 203388)
+++ trunk/LayoutTests/imported/w3c/ChangeLog 2016-07-19 00:57:18 UTC (rev 203389)
</span><span class="lines">@@ -1,5 +1,17 @@
</span><span class="cx"> 2016-07-18 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><ins>+ DocType's publicId / systemId should not be nullable
+ https://bugs.webkit.org/show_bug.cgi?id=159901
+
+ Reviewed by Benjamin Poulain.
+
+ Rebaseline now that more checks regarding DocumentType serialization
+ are passing.
+
+ * web-platform-tests/domparsing/xml-serialization-expected.txt:
+
+2016-07-18 Chris Dumez <cdumez@apple.com>
+
</ins><span class="cx"> The 2 first parameters to addEventListener() / removeEventListener() should be mandatory
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=158008
</span><span class="cx">
</span></span></pre></div>
<a id="trunkLayoutTestsimportedw3cwebplatformtestsdomparsingxmlserializationexpectedtxt"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/domparsing/xml-serialization-expected.txt (203388 => 203389)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/imported/w3c/web-platform-tests/domparsing/xml-serialization-expected.txt 2016-07-19 00:47:40 UTC (rev 203388)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/domparsing/xml-serialization-expected.txt 2016-07-19 00:57:18 UTC (rev 203389)
</span><span class="lines">@@ -3,9 +3,9 @@
</span><span class="cx"> PASS Comment: starting with -
</span><span class="cx"> PASS Comment: ending with -
</span><span class="cx"> PASS Comment: containing -->
</span><del>-FAIL DocumentType: empty public and system id assert_equals: expected "<!DOCTYPE html>" but got "<!DOCTYPE html PUBLIC \"\" \"\">"
-FAIL DocumentType: empty system id assert_equals: expected "<!DOCTYPE html PUBLIC \"a\">" but got "<!DOCTYPE html PUBLIC \"a\" \"\">"
-FAIL DocumentType: empty public id assert_equals: expected "<!DOCTYPE html SYSTEM \"a\">" but got "<!DOCTYPE html PUBLIC \"\" \"a\">"
</del><ins>+PASS DocumentType: empty public and system id
+PASS DocumentType: empty system id
+PASS DocumentType: empty public id
</ins><span class="cx"> PASS DocumentType: non-empty public and system id
</span><span class="cx"> PASS DocumentType: 'APOSTROPHE' (U+0027)
</span><span class="cx"> PASS DocumentType: 'QUOTATION MARK' (U+0022)
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (203388 => 203389)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-07-19 00:47:40 UTC (rev 203388)
+++ trunk/Source/WebCore/ChangeLog 2016-07-19 00:57:18 UTC (rev 203389)
</span><span class="lines">@@ -1,3 +1,34 @@
</span><ins>+2016-07-18 Chris Dumez <cdumez@apple.com>
+
+ DocType's publicId / systemId should not be nullable
+ https://bugs.webkit.org/show_bug.cgi?id=159901
+
+ Reviewed by Benjamin Poulain.
+
+ DocType's publicId / systemId should not be nullable. While they were
+ not marked as nullable in our IDL, they could be stored as null Strings
+ in our implementation depending on how the Node was constructed. This
+ led to subtle bugs where String() != emptyString().
+
+ In particular, Node.isEqualNode() would return false when DocumentType
+ nodes would mismatch because of their publicId / systemId being null
+ instead of the emptyString.
+
+ Serialization would DocumentType nodes would also be wrong when
+ publicId / systemId were empty Strings instead of null strings. The
+ new behavior now matches:
+ - https://www.w3.org/TR/DOM-Parsing/#dfn-concept-serialize-doctype (steps 7-9)
+
+ To address these issues, we now always store publicId / systemId as
+ non-null Strings inside the DocumentType class.
+
+ Test: fast/dom/DocumentType/isEqualNode.html
+
+ * dom/DocumentType.cpp:
+ (WebCore::DocumentType::DocumentType):
+ * editing/MarkupAccumulator.cpp:
+ (WebCore::MarkupAccumulator::appendDocumentType):
+
</ins><span class="cx"> 2016-07-18 Jeremy Jones <jeremyj@apple.com>
</span><span class="cx">
</span><span class="cx"> If previous media session interruptions were prevented, still allow subsequent interruptions to try.
</span></span></pre></div>
<a id="trunkSourceWebCoredomDocumentTypecpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/dom/DocumentType.cpp (203388 => 203389)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/dom/DocumentType.cpp 2016-07-19 00:47:40 UTC (rev 203388)
+++ trunk/Source/WebCore/dom/DocumentType.cpp 2016-07-19 00:57:18 UTC (rev 203389)
</span><span class="lines">@@ -31,8 +31,8 @@
</span><span class="cx"> DocumentType::DocumentType(Document& document, const String& name, const String& publicId, const String& systemId)
</span><span class="cx"> : Node(document, CreateOther)
</span><span class="cx"> , m_name(name)
</span><del>- , m_publicId(publicId)
- , m_systemId(systemId)
</del><ins>+ , m_publicId(publicId.isNull() ? emptyString() : publicId)
+ , m_systemId(systemId.isNull() ? emptyString() : systemId)
</ins><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingMarkupAccumulatorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/MarkupAccumulator.cpp (203388 => 203389)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/editing/MarkupAccumulator.cpp 2016-07-19 00:47:40 UTC (rev 203388)
+++ trunk/Source/WebCore/editing/MarkupAccumulator.cpp 2016-07-19 00:57:18 UTC (rev 203389)
</span><span class="lines">@@ -389,17 +389,17 @@
</span><span class="cx">
</span><span class="cx"> result.appendLiteral("<!DOCTYPE ");
</span><span class="cx"> result.append(documentType.name());
</span><del>- if (!documentType.publicId().isNull()) {
</del><ins>+ if (!documentType.publicId().isEmpty()) {
</ins><span class="cx"> result.appendLiteral(" PUBLIC \"");
</span><span class="cx"> result.append(documentType.publicId());
</span><span class="cx"> result.append('"');
</span><del>- if (!documentType.systemId().isNull()) {
</del><ins>+ if (!documentType.systemId().isEmpty()) {
</ins><span class="cx"> result.append(' ');
</span><span class="cx"> result.append('"');
</span><span class="cx"> result.append(documentType.systemId());
</span><span class="cx"> result.append('"');
</span><span class="cx"> }
</span><del>- } else if (!documentType.systemId().isNull()) {
</del><ins>+ } else if (!documentType.systemId().isEmpty()) {
</ins><span class="cx"> result.appendLiteral(" SYSTEM \"");
</span><span class="cx"> result.append(documentType.systemId());
</span><span class="cx"> result.append('"');
</span></span></pre>
</div>
</div>
</body>
</html>