<!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>[203475] 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/203475">203475</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2016-07-20 16:07:30 -0700 (Wed, 20 Jul 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>Stop using valueToStringWithNullCheck() in JSCSSStyleDeclaration::putDelegate()
https://bugs.webkit.org/show_bug.cgi?id=159982
Reviewed by Ryosuke Niwa.
Source/WebCore:
valueToStringWithNullCheck() treats null as the null String() which is
legacy / non standard behavior. The specification says we should treat
null as the empty string:
- https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute
Therefore, we should be using valueToStringTreatingNullAsEmptyString() instead.
In practice, there is no web-exposed behavior change because
MutableStyleProperties::setProperty() removes the property wether the
value is the null String or the empty String.
This behavior is correct since the specification says that we should
remove the property if the value is the empty string:
- https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty (step 4)
I added test coverage to make sure we behave according to specification.
This test is passing in Firefox, Chrome and in WebKit (before and after
my change).
Test: fast/css/CSSStyleDeclaration-property-setter.html
* bindings/js/JSCSSStyleDeclarationCustom.cpp:
(WebCore::JSCSSStyleDeclaration::putDelegate):
LayoutTests:
Add layout test coverage for JSCSSStyleDeclaration::putDelegate(),
covering cases like setting to null or the empty String, to make
sure we behave according to specification.
* fast/css/CSSStyleDeclaration-property-setter-expected.txt: Added.
* fast/css/CSSStyleDeclaration-property-setter.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorebindingsjsJSCSSStyleDeclarationCustomcpp">trunk/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssCSSStyleDeclarationpropertysetterexpectedtxt">trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssCSSStyleDeclarationpropertysetterhtml">trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (203474 => 203475)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-07-20 23:06:35 UTC (rev 203474)
+++ trunk/LayoutTests/ChangeLog 2016-07-20 23:07:30 UTC (rev 203475)
</span><span class="lines">@@ -1,5 +1,19 @@
</span><span class="cx"> 2016-07-20 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><ins>+ Stop using valueToStringWithNullCheck() in JSCSSStyleDeclaration::putDelegate()
+ https://bugs.webkit.org/show_bug.cgi?id=159982
+
+ Reviewed by Ryosuke Niwa.
+
+ Add layout test coverage for JSCSSStyleDeclaration::putDelegate(),
+ covering cases like setting to null or the empty String, to make
+ sure we behave according to specification.
+
+ * fast/css/CSSStyleDeclaration-property-setter-expected.txt: Added.
+ * fast/css/CSSStyleDeclaration-property-setter.html: Added.
+
+2016-07-20 Chris Dumez <cdumez@apple.com>
+
</ins><span class="cx"> Fix null handling of HTMLFrameElement.marginWidth / marginHeight
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=159987
</span><span class="cx">
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssCSSStyleDeclarationpropertysetterexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter-expected.txt (0 => 203475)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter-expected.txt 2016-07-20 23:07:30 UTC (rev 203475)
</span><span class="lines">@@ -0,0 +1,29 @@
</span><ins>+Test that setting CSS properties via CSSStyleDeclaration behaves as expected
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS div.style.cssText is ""
+div.style.backgroundColor = 'red'
+PASS div.style.cssText is "background-color: red;"
+PASS div.style.getPropertyValue('background-color') is "red"
+PASS div.style.length is 1
+
+div.style.backgroundColor = ''
+PASS div.style.cssText is ""
+PASS div.style.getPropertyValue('background-color') is ""
+PASS div.style.length is 0
+
+div.style.backgroundColor = 'red'
+PASS div.style.cssText is "background-color: red;"
+PASS div.style.getPropertyValue('background-color') is "red"
+PASS div.style.length is 1
+
+div.style.backgroundColor = null
+PASS div.style.cssText is ""
+PASS div.style.getPropertyValue('background-color') is ""
+PASS div.style.length is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssCSSStyleDeclarationpropertysetterhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter.html (0 => 203475)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter.html (rev 0)
+++ trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter.html 2016-07-20 23:07:30 UTC (rev 203475)
</span><span class="lines">@@ -0,0 +1,37 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description("Test that setting CSS properties via CSSStyleDeclaration behaves as expected");
+
+var div = document.createElement("div");
+shouldBeEqualToString("div.style.cssText", "");
+evalAndLog("div.style.backgroundColor = 'red'");
+shouldBeEqualToString("div.style.cssText", "background-color: red;");
+shouldBeEqualToString("div.style.getPropertyValue('background-color')", "red");
+shouldBe("div.style.length", "1");
+
+debug("");
+// Setting to empty string should delete the property.
+evalAndLog("div.style.backgroundColor = ''");
+shouldBeEqualToString("div.style.cssText", "");
+shouldBeEqualToString("div.style.getPropertyValue('background-color')", "");
+shouldBe("div.style.length", "0");
+
+debug("");
+evalAndLog("div.style.backgroundColor = 'red'");
+shouldBeEqualToString("div.style.cssText", "background-color: red;");
+shouldBeEqualToString("div.style.getPropertyValue('background-color')", "red");
+shouldBe("div.style.length", "1");
+
+debug("");
+// Setting to null should delete the property (null is treated as the empty string).
+evalAndLog("div.style.backgroundColor = null");
+shouldBeEqualToString("div.style.cssText", "");
+shouldBeEqualToString("div.style.getPropertyValue('background-color')", "");
+shouldBe("div.style.length", "0");
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (203474 => 203475)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-07-20 23:06:35 UTC (rev 203474)
+++ trunk/Source/WebCore/ChangeLog 2016-07-20 23:07:30 UTC (rev 203475)
</span><span class="lines">@@ -1,5 +1,36 @@
</span><span class="cx"> 2016-07-20 Chris Dumez <cdumez@apple.com>
</span><span class="cx">
</span><ins>+ Stop using valueToStringWithNullCheck() in JSCSSStyleDeclaration::putDelegate()
+ https://bugs.webkit.org/show_bug.cgi?id=159982
+
+ Reviewed by Ryosuke Niwa.
+
+ valueToStringWithNullCheck() treats null as the null String() which is
+ legacy / non standard behavior. The specification says we should treat
+ null as the empty string:
+ - https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute
+
+ Therefore, we should be using valueToStringTreatingNullAsEmptyString() instead.
+
+ In practice, there is no web-exposed behavior change because
+ MutableStyleProperties::setProperty() removes the property wether the
+ value is the null String or the empty String.
+
+ This behavior is correct since the specification says that we should
+ remove the property if the value is the empty string:
+ - https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty (step 4)
+
+ I added test coverage to make sure we behave according to specification.
+ This test is passing in Firefox, Chrome and in WebKit (before and after
+ my change).
+
+ Test: fast/css/CSSStyleDeclaration-property-setter.html
+
+ * bindings/js/JSCSSStyleDeclarationCustom.cpp:
+ (WebCore::JSCSSStyleDeclaration::putDelegate):
+
+2016-07-20 Chris Dumez <cdumez@apple.com>
+
</ins><span class="cx"> Fix null handling of HTMLFrameElement.marginWidth / marginHeight
</span><span class="cx"> https://bugs.webkit.org/show_bug.cgi?id=159987
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebCorebindingsjsJSCSSStyleDeclarationCustomcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp (203474 => 203475)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp 2016-07-20 23:06:35 UTC (rev 203474)
+++ trunk/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp 2016-07-20 23:07:30 UTC (rev 203475)
</span><span class="lines">@@ -326,7 +326,7 @@
</span><span class="cx"> if (!propertyInfo.propertyID)
</span><span class="cx"> return false;
</span><span class="cx">
</span><del>- String propValue = valueToStringWithNullCheck(exec, value);
</del><ins>+ String propValue = valueToStringTreatingNullAsEmptyString(exec, value);
</ins><span class="cx"> if (propertyInfo.hadPixelOrPosPrefix)
</span><span class="cx"> propValue.append("px");
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>