Title: [203475] trunk
- Revision
- 203475
- Author
- cdu...@apple.com
- Date
- 2016-07-20 16:07:30 -0700 (Wed, 20 Jul 2016)
Log Message
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (203474 => 203475)
--- trunk/LayoutTests/ChangeLog 2016-07-20 23:06:35 UTC (rev 203474)
+++ trunk/LayoutTests/ChangeLog 2016-07-20 23:07:30 UTC (rev 203475)
@@ -1,5 +1,19 @@
2016-07-20 Chris Dumez <cdu...@apple.com>
+ 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 <cdu...@apple.com>
+
Fix null handling of HTMLFrameElement.marginWidth / marginHeight
https://bugs.webkit.org/show_bug.cgi?id=159987
Added: trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter-expected.txt (0 => 203475)
--- 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)
@@ -0,0 +1,29 @@
+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
+
Added: trunk/LayoutTests/fast/css/CSSStyleDeclaration-property-setter.html (0 => 203475)
--- 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)
@@ -0,0 +1,37 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<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=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (203474 => 203475)
--- 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)
@@ -1,5 +1,36 @@
2016-07-20 Chris Dumez <cdu...@apple.com>
+ 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 <cdu...@apple.com>
+
Fix null handling of HTMLFrameElement.marginWidth / marginHeight
https://bugs.webkit.org/show_bug.cgi?id=159987
Modified: trunk/Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp (203474 => 203475)
--- 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)
@@ -326,7 +326,7 @@
if (!propertyInfo.propertyID)
return false;
- String propValue = valueToStringWithNullCheck(exec, value);
+ String propValue = valueToStringTreatingNullAsEmptyString(exec, value);
if (propertyInfo.hadPixelOrPosPrefix)
propValue.append("px");
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
