<!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>[203460] 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/203460">203460</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2016-07-20 11:01:26 -0700 (Wed, 20 Jul 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>CSSStyleDeclaration.setProperty() should be able to unset "important" on a property
https://bugs.webkit.org/show_bug.cgi?id=159959
Reviewed by Alexey Proskuryakov.
Source/WebCore:
CSSStyleDeclaration.setProperty() should be able to unsert "important"
on a property as per the latest specification:
- https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty
- https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute
Firefox and Chrome match the specification here but WebKit was ignoring calls
to setProperty() if there is already an "important" property wit this name
and if the new property does not have the "important" flag set.
This behavior was added a long time ago via Bug 60007. However, it does not
match the latest specification or other browsers.
Test: fast/css/CSSStyleDeclaration-setProperty-unset-important.html
* css/StyleProperties.cpp:
(WebCore::MutableStyleProperties::addParsedProperty):
Drop code that was added via Bug 60007 as this behavior no longer matches the
specification or other browsers. The layout test added in Bug 60007 fails in
other browsers and was updated in this patch to match the specification.
LayoutTests:
* fast/css/CSSStyleDeclaration-setProperty-unset-important-expected.txt: Added.
* fast/css/CSSStyleDeclaration-setProperty-unset-important.html: Added.
Add layout test coverage.
* fast/css/important-js-override.html:
The test covered our 'wrong' behavior and was failing in Firefox / Chrome.
I updated the test to match the behavior in the specification. The test
now passed in Chrome and Firefox.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsfastcssimportantjsoverridehtml">trunk/LayoutTests/fast/css/important-js-override.html</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorecssStylePropertiescpp">trunk/Source/WebCore/css/StyleProperties.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssCSSStyleDeclarationsetPropertyunsetimportantexpectedtxt">trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-unset-important-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssCSSStyleDeclarationsetPropertyunsetimportanthtml">trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-unset-important.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (203459 => 203460)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-07-20 17:55:53 UTC (rev 203459)
+++ trunk/LayoutTests/ChangeLog 2016-07-20 18:01:26 UTC (rev 203460)
</span><span class="lines">@@ -1,3 +1,19 @@
</span><ins>+2016-07-20 Chris Dumez <cdumez@apple.com>
+
+ CSSStyleDeclaration.setProperty() should be able to unset "important" on a property
+ https://bugs.webkit.org/show_bug.cgi?id=159959
+
+ Reviewed by Alexey Proskuryakov.
+
+ * fast/css/CSSStyleDeclaration-setProperty-unset-important-expected.txt: Added.
+ * fast/css/CSSStyleDeclaration-setProperty-unset-important.html: Added.
+ Add layout test coverage.
+
+ * fast/css/important-js-override.html:
+ The test covered our 'wrong' behavior and was failing in Firefox / Chrome.
+ I updated the test to match the behavior in the specification. The test
+ now passed in Chrome and Firefox.
+
</ins><span class="cx"> 2016-07-20 Commit Queue <commit-queue@webkit.org>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r203423.
</span></span></pre></div>
<a id="trunkLayoutTestsfastcssCSSStyleDeclarationsetPropertyunsetimportantexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-unset-important-expected.txt (0 => 203460)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-unset-important-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-unset-important-expected.txt 2016-07-20 18:01:26 UTC (rev 203460)
</span><span class="lines">@@ -0,0 +1,15 @@
</span><ins>+Test that CSSStyleDeclaration.setProperty() can unset the 'important' flag
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+div.style.setProperty('color', 'red', 'important')
+PASS div.style.getPropertyValue('color') is "red"
+PASS div.style.getPropertyPriority('color') is "important"
+div.style.setProperty('color', 'green', null)
+PASS div.style.getPropertyValue('color') is "green"
+PASS div.style.getPropertyPriority('color') is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssCSSStyleDeclarationsetPropertyunsetimportanthtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-unset-important.html (0 => 203460)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-unset-important.html (rev 0)
+++ trunk/LayoutTests/fast/css/CSSStyleDeclaration-setProperty-unset-important.html 2016-07-20 18:01:26 UTC (rev 203460)
</span><span class="lines">@@ -0,0 +1,19 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/js-test-pre.js"></script>
+<script>
+description("Test that CSSStyleDeclaration.setProperty() can unset the 'important' flag");
+
+var div = document.createElement("div");
+evalAndLog("div.style.setProperty('color', 'red', 'important')");
+shouldBeEqualToString("div.style.getPropertyValue('color')", "red");
+shouldBeEqualToString("div.style.getPropertyPriority('color')", "important");
+
+evalAndLog("div.style.setProperty('color', 'green', null)");
+shouldBeEqualToString("div.style.getPropertyValue('color')", "green");
+shouldBeEqualToString("div.style.getPropertyPriority('color')", "");
+</script>
+<script src="../../resources/js-test-post.js"></script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssimportantjsoverridehtml"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/fast/css/important-js-override.html (203459 => 203460)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/important-js-override.html 2016-07-20 17:55:53 UTC (rev 203459)
+++ trunk/LayoutTests/fast/css/important-js-override.html 2016-07-20 18:01:26 UTC (rev 203460)
</span><span class="lines">@@ -1,8 +1,8 @@
</span><del>-<div id="a" style="background-color: green !important">The background of this element should be green. It is </div>
</del><ins>+<div id="a" style="background-color: red !important">The background of this element should be green. It is </div>
</ins><span class="cx"> <script>
</span><span class="cx"> if (window.testRunner)
</span><span class="cx"> testRunner.dumpAsText();
</span><span class="cx"> var a = document.getElementById("a");
</span><del>- a.style.backgroundColor = "red";
</del><ins>+ a.style.backgroundColor = "green";
</ins><span class="cx"> a.innerHTML += a.style.backgroundColor;
</span><span class="cx"> </script>
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (203459 => 203460)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-07-20 17:55:53 UTC (rev 203459)
+++ trunk/Source/WebCore/ChangeLog 2016-07-20 18:01:26 UTC (rev 203460)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2016-07-20 Chris Dumez <cdumez@apple.com>
+
+ CSSStyleDeclaration.setProperty() should be able to unset "important" on a property
+ https://bugs.webkit.org/show_bug.cgi?id=159959
+
+ Reviewed by Alexey Proskuryakov.
+
+ CSSStyleDeclaration.setProperty() should be able to unsert "important"
+ on a property as per the latest specification:
+ - https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-setproperty
+ - https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel-cased-attribute
+
+ Firefox and Chrome match the specification here but WebKit was ignoring calls
+ to setProperty() if there is already an "important" property wit this name
+ and if the new property does not have the "important" flag set.
+
+ This behavior was added a long time ago via Bug 60007. However, it does not
+ match the latest specification or other browsers.
+
+ Test: fast/css/CSSStyleDeclaration-setProperty-unset-important.html
+
+ * css/StyleProperties.cpp:
+ (WebCore::MutableStyleProperties::addParsedProperty):
+ Drop code that was added via Bug 60007 as this behavior no longer matches the
+ specification or other browsers. The layout test added in Bug 60007 fails in
+ other browsers and was updated in this patch to match the specification.
+
</ins><span class="cx"> 2016-07-20 Commit Queue <commit-queue@webkit.org>
</span><span class="cx">
</span><span class="cx"> Unreviewed, rolling out r203423.
</span></span></pre></div>
<a id="trunkSourceWebCorecssStylePropertiescpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/StyleProperties.cpp (203459 => 203460)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/css/StyleProperties.cpp 2016-07-20 17:55:53 UTC (rev 203459)
+++ trunk/Source/WebCore/css/StyleProperties.cpp 2016-07-20 18:01:26 UTC (rev 203460)
</span><span class="lines">@@ -883,12 +883,7 @@
</span><span class="cx"> return setProperty(property);
</span><span class="cx"> return false;
</span><span class="cx"> }
</span><del>-
- // Only add properties that have no !important counterpart present
- if (!propertyIsImportant(property.id()) || property.isImportant())
- return setProperty(property);
-
- return false;
</del><ins>+ return setProperty(property);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> String StyleProperties::asText() const
</span></span></pre>
</div>
</div>
</body>
</html>