<!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>[203437] 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/203437">203437</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2016-07-19 18:20:23 -0700 (Tue, 19 Jul 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Align CSSStyleDeclaration.setProperty() with the specification
https://bugs.webkit.org/show_bug.cgi?id=159955
Reviewed by Benjamin Poulain.
Source/WebCore:
Align CSSStyleDeclaration.setProperty() with the specification:
- https://drafts.csswg.org/cssom/#the-cssstyledeclaration-interface
In particular, the following changes were needed:
1. The 'value' parameter should not be optional
2. The 'priority' parameter should treat null as the empty string
rather than the string "null".
3. The 'priority' parameter's default value should be the empty string,
not the string "undefined".
4. CSSStyleDeclaration.setProperty() should return early if 'priority'
is not the empty string and is not an ASCII case-insensitive match
for the string "important".
Chrome matches the specification entirely.
Firefox matches the specification with the exception that it does a
case-sensitive match for "important".
Test: fast/css/CSSStyleDeclaration-setProperty.html
* css/CSSStyleDeclaration.idl:
* css/PropertySetCSSStyleDeclaration.cpp:
(WebCore::PropertySetCSSStyleDeclaration::setProperty):
LayoutTests:
Add layout test coverage.
* fast/css/CSSStyleDeclaration-setProperty-expected.txt: Added.
* fast/css/CSSStyleDeclaration-setProperty.html: Added.
* fast/css/shorthand-priority.html:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsfastcssshorthandpriorityhtml">trunk/LayoutTests/fast/css/shorthand-priority.html</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorecssCSSStyleDeclarationidl">trunk/Source/WebCore/css/CSSStyleDeclaration.idl</a></li>
<li><a href="#trunkSourceWebCorecssPropertySetCSSStyleDeclarationcpp">trunk/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssCSSStyleDeclarationsetPropertyexpectedtxt">trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssCSSStyleDeclarationsetPropertyhtml">trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (203436 => 203437)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-07-20 00:29:21 UTC (rev 203436)
+++ trunk/LayoutTests/ChangeLog 2016-07-20 01:20:23 UTC (rev 203437)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2016-07-19 Chris Dumez <cdumez@apple.com>
+
+ Align CSSStyleDeclaration.setProperty() with the specification
+ https://bugs.webkit.org/show_bug.cgi?id=159955
+
+ Reviewed by Benjamin Poulain.
+
+ Add layout test coverage.
+
+ * fast/css/CSSStyleDeclaration-setProperty-expected.txt: Added.
+ * fast/css/CSSStyleDeclaration-setProperty.html: Added.
+ * fast/css/shorthand-priority.html:
+
</ins><span class="cx"> 2016-07-19 Daniel Bates <dabates@apple.com>
</span><span class="cx">
</span><span class="cx"> CSP: Improve support for multiple policies to more closely conform to the CSP Level 2 spec.
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssCSSStyleDeclarationsetPropertyexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-expected.txt (0 => 203437)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-expected.txt 2016-07-20 01:20:23 UTC (rev 203437)
</span><span class="lines">@@ -0,0 +1,41 @@
</span><ins>+Test the behavior of CSSStyleDeclaration.setProperty()
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+* Not enough parameters
+PASS div.style.setProperty() threw exception TypeError: Not enough arguments.
+PASS div.style.setProperty('color') threw exception TypeError: Not enough arguments.
+
+* Should treat null as empty string for 'value' parameter
+PASS div.style.setProperty('color', null) did not throw exception.
+PASS div.style.getPropertyValue('color') is ""
+PASS div.style.getPropertyPriority('color') is ""
+
+* Should treat null as empty string for 'priority' parameter
+PASS div.style.setProperty('background-color', 'green', null) did not throw exception.
+PASS div.style.getPropertyValue('background-color') is "green"
+PASS div.style.getPropertyPriority('background-color') is ""
+
+* Last parameter should do a case-insensitive match to 'important'
+PASS div.style.setProperty('border-left-color', 'green', 'important') did not throw exception.
+PASS div.style.getPropertyValue('border-left-color') is "green"
+PASS div.style.getPropertyPriority('border-left-color') is "important"
+PASS div.style.setProperty('border-right-color', 'green', 'IMPORTANT') did not throw exception.
+PASS div.style.getPropertyValue('border-right-color') is "green"
+PASS div.style.getPropertyPriority('border-right-color') is "important"
+
+* Invalid 'priority' value, should abort
+PASS div.style.setProperty('border-top-color', 'red', 'invalid') did not throw exception.
+PASS div.style.getPropertyValue('border-top-color') is ""
+PASS div.style.getPropertyPriority('border-top-color') is ""
+PASS div.style.setProperty('border-top-color', 'red', 'important invalid') did not throw exception.
+PASS div.style.getPropertyValue('border-top-color') is ""
+PASS div.style.getPropertyPriority('border-top-color') is ""
+PASS div.style.setProperty('border-top-color', 'red', '!important') did not throw exception.
+PASS div.style.getPropertyValue('border-top-color') is ""
+PASS div.style.getPropertyPriority('border-top-color') is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssCSSStyleDeclarationsetPropertyhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty.html (0 => 203437)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty.html (rev 0)
+++ trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty.html 2016-07-20 01:20:23 UTC (rev 203437)
</span><span class="lines">@@ -0,0 +1,49 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description("Test the behavior of CSSStyleDeclaration.setProperty()");
+
+var div = document.createElement("div");
+debug("* Not enough parameters");
+shouldThrow("div.style.setProperty()");
+shouldThrow("div.style.setProperty('color')");
+
+debug("");
+debug("* Should treat null as empty string for 'value' parameter");
+shouldNotThrow("div.style.setProperty('color', null)");
+shouldBeEqualToString("div.style.getPropertyValue('color')", "");
+shouldBeEqualToString("div.style.getPropertyPriority('color')", "");
+
+debug("");
+debug("* Should treat null as empty string for 'priority' parameter");
+shouldNotThrow("div.style.setProperty('background-color', 'green', null)");
+shouldBeEqualToString("div.style.getPropertyValue('background-color')", "green");
+shouldBeEqualToString("div.style.getPropertyPriority('background-color')", "");
+
+debug("");
+debug("* Last parameter should do a case-insensitive match to 'important'");
+shouldNotThrow("div.style.setProperty('border-left-color', 'green', 'important')");
+shouldBeEqualToString("div.style.getPropertyValue('border-left-color')", "green");
+shouldBeEqualToString("div.style.getPropertyPriority('border-left-color')", "important");
+shouldNotThrow("div.style.setProperty('border-right-color', 'green', 'IMPORTANT')");
+shouldBeEqualToString("div.style.getPropertyValue('border-right-color')", "green");
+shouldBeEqualToString("div.style.getPropertyPriority('border-right-color')", "important");
+
+debug("");
+debug("* Invalid 'priority' value, should abort");
+shouldNotThrow("div.style.setProperty('border-top-color', 'red', 'invalid')");
+shouldBeEqualToString("div.style.getPropertyValue('border-top-color')", "");
+shouldBeEqualToString("div.style.getPropertyPriority('border-top-color')", "");
+shouldNotThrow("div.style.setProperty('border-top-color', 'red', 'important invalid')");
+shouldBeEqualToString("div.style.getPropertyValue('border-top-color')", "");
+shouldBeEqualToString("div.style.getPropertyPriority('border-top-color')", "");
+shouldNotThrow("div.style.setProperty('border-top-color', 'red', '!important')");
+shouldBeEqualToString("div.style.getPropertyValue('border-top-color')", "");
+shouldBeEqualToString("div.style.getPropertyPriority('border-top-color')", "");
+
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssshorthandpriorityhtml"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/css/shorthand-priority.html (203436 => 203437)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/shorthand-priority.html 2016-07-20 00:29:21 UTC (rev 203436)
+++ trunk/LayoutTests/fast/css/shorthand-priority.html 2016-07-20 01:20:23 UTC (rev 203437)
</span><span class="lines">@@ -17,7 +17,7 @@
</span><span class="cx"> e = document.getElementById('test');
</span><span class="cx">
</span><span class="cx"> // Sanity check.
</span><del>-e.style.setProperty("border-bottom-style", "solid", "!important");
</del><ins>+e.style.setProperty("border-bottom-style", "solid", "important");
</ins><span class="cx"> shouldBe("e.style.getPropertyValue('border-bottom-style')", "'solid'");
</span><span class="cx"> shouldBe("e.style.getPropertyPriority('border-bottom-style')", "'important'");
</span><span class="cx">
</span><span class="lines">@@ -27,7 +27,7 @@
</span><span class="cx"> shouldBe("e.style.getPropertyPriority('border')", "''");
</span><span class="cx">
</span><span class="cx"> e.style.border = "";
</span><del>-e.style.setProperty("border", "20px solid green", "!important");
</del><ins>+e.style.setProperty("border", "20px solid green", "important");
</ins><span class="cx"> shouldBe("e.style.getPropertyValue('border')", "'20px solid green'");
</span><span class="cx"> shouldBe("e.style.getPropertyPriority('border')", "'important'");
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (203436 => 203437)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-07-20 00:29:21 UTC (rev 203436)
+++ trunk/Source/WebCore/ChangeLog 2016-07-20 01:20:23 UTC (rev 203437)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2016-07-19 Chris Dumez <cdumez@apple.com>
+
+ Align CSSStyleDeclaration.setProperty() with the specification
+ https://bugs.webkit.org/show_bug.cgi?id=159955
+
+ Reviewed by Benjamin Poulain.
+
+ Align CSSStyleDeclaration.setProperty() with the specification:
+ - https://drafts.csswg.org/cssom/#the-cssstyledeclaration-interface
+
+ In particular, the following changes were needed:
+ 1. The 'value' parameter should not be optional
+ 2. The 'priority' parameter should treat null as the empty string
+ rather than the string "null".
+ 3. The 'priority' parameter's default value should be the empty string,
+ not the string "undefined".
+ 4. CSSStyleDeclaration.setProperty() should return early if 'priority'
+ is not the empty string and is not an ASCII case-insensitive match
+ for the string "important".
+
+ Chrome matches the specification entirely.
+ Firefox matches the specification with the exception that it does a
+ case-sensitive match for "important".
+
+ Test: fast/css/CSSStyleDeclaration-setProperty.html
+
+ * css/CSSStyleDeclaration.idl:
+ * css/PropertySetCSSStyleDeclaration.cpp:
+ (WebCore::PropertySetCSSStyleDeclaration::setProperty):
+
</ins><span class="cx"> 2016-07-19 Daniel Bates <dabates@apple.com>
</span><span class="cx">
</span><span class="cx"> CSP: Improve support for multiple policies to more closely conform to the CSP Level 2 spec.
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSStyleDeclarationidl"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSStyleDeclaration.idl (203436 => 203437)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/CSSStyleDeclaration.idl 2016-07-20 00:29:21 UTC (rev 203436)
+++ trunk/Source/WebCore/css/CSSStyleDeclaration.idl 2016-07-20 01:20:23 UTC (rev 203437)
</span><span class="lines">@@ -38,11 +38,7 @@
</span><span class="cx"> [RaisesException] DOMString removeProperty(optional DOMString propertyName = "undefined");
</span><span class="cx"> DOMString? getPropertyPriority(optional DOMString propertyName = "undefined");
</span><span class="cx">
</span><del>- // FIXME: 'priority' should use [TreatNullAs=EmptyString].
- // FIXME: Using "undefined" as default parameter value is wrong.
- [ObjCLegacyUnnamedParameters, RaisesException] void setProperty(optional DOMString propertyName = "undefined",
- [TreatNullAs=EmptyString] optional DOMString value = "undefined",
- optional DOMString priority = "undefined");
</del><ins>+ [ObjCLegacyUnnamedParameters, RaisesException] void setProperty(DOMString propertyName, [TreatNullAs=EmptyString] DOMString value, [TreatNullAs=EmptyString] optional DOMString priority = "");
</ins><span class="cx">
</span><span class="cx"> readonly attribute unsigned long length;
</span><span class="cx"> getter DOMString item(optional unsigned long index = 0);
</span></span></pre></div>
<a id="trunkSourceWebCorecssPropertySetCSSStyleDeclarationcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp (203436 => 203437)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp 2016-07-20 00:29:21 UTC (rev 203436)
+++ trunk/Source/WebCore/css/PropertySetCSSStyleDeclaration.cpp 2016-07-20 01:20:23 UTC (rev 203437)
</span><span class="lines">@@ -226,7 +226,9 @@
</span><span class="cx"> if (!willMutate())
</span><span class="cx"> return;
</span><span class="cx">
</span><del>- bool important = priority.find("important", 0, false) != notFound;
</del><ins>+ bool important = equalIgnoringASCIICase(priority, "important");
+ if (!important && !priority.isEmpty())
+ return;
</ins><span class="cx">
</span><span class="cx"> ec = 0;
</span><span class="cx"> bool changed = propertyID != CSSPropertyCustom ? m_propertySet->setProperty(propertyID, value, important, contextStyleSheet()) : m_propertySet->setCustomProperty(propertyName, value, important, contextStyleSheet());
</span></span></pre>
</div>
</div>
</body>
</html>