- Revision
- 203753
- Author
- cdu...@apple.com
- Date
- 2016-07-26 16:37:52 -0700 (Tue, 26 Jul 2016)
Log Message
Align CSSKeyframesRule with the specification
https://bugs.webkit.org/show_bug.cgi?id=160219
Reviewed by Darin Adler.
Source/WebCore:
Align CSSKeyframesRule with the specification:
- https://drafts.csswg.org/css-animations/#interface-csskeyframesrule
In particular, the parameter to insertRule() / appendRule() /
deleteRule() / findRule() should be mandatory. Both Firefox and Chrome
agree with the specification here.
Also, the CSSKeyframesRule.name attribute should not be nullable.
Chrome agrees with the specification. However, Firefox, has the
attribute nullable. This patch aligns our behavior with Chrome and
the specification.
Tests: animations/CSSKeyframesRule-name-null.html
animations/CSSKeyframesRule-parameters.html
* css/CSSKeyframesRule.h:
(WebCore::StyleRuleKeyframes::name):
(WebCore::StyleRuleKeyframes::setName):
* css/CSSKeyframesRule.idl:
LayoutTests:
Add layout test coverage.
* animations/CSSKeyframesRule-name-null-expected.txt: Added.
* animations/CSSKeyframesRule-name-null.html: Added.
* animations/CSSKeyframesRule-parameters-expected.txt: Added.
* animations/CSSKeyframesRule-parameters.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (203752 => 203753)
--- trunk/LayoutTests/ChangeLog 2016-07-26 23:36:52 UTC (rev 203752)
+++ trunk/LayoutTests/ChangeLog 2016-07-26 23:37:52 UTC (rev 203753)
@@ -1,3 +1,17 @@
+2016-07-26 Chris Dumez <cdu...@apple.com>
+
+ Align CSSKeyframesRule with the specification
+ https://bugs.webkit.org/show_bug.cgi?id=160219
+
+ Reviewed by Darin Adler.
+
+ Add layout test coverage.
+
+ * animations/CSSKeyframesRule-name-null-expected.txt: Added.
+ * animations/CSSKeyframesRule-name-null.html: Added.
+ * animations/CSSKeyframesRule-parameters-expected.txt: Added.
+ * animations/CSSKeyframesRule-parameters.html: Added.
+
2016-07-26 Myles C. Maxfield <mmaxfi...@apple.com>
[iPhone] Playing a video on tudou.com plays only sound, no video
Added: trunk/LayoutTests/animations/CSSKeyframesRule-name-null-expected.txt (0 => 203753)
--- trunk/LayoutTests/animations/CSSKeyframesRule-name-null-expected.txt (rev 0)
+++ trunk/LayoutTests/animations/CSSKeyframesRule-name-null-expected.txt 2016-07-26 23:37:52 UTC (rev 203753)
@@ -0,0 +1,13 @@
+Tests that the parameters are mandatory in CSSKeyframesRule API.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS keyframes.__proto__ is CSSKeyframesRule.prototype
+PASS keyframes.name is "test1"
+PASS keyframes.name = null did not throw exception.
+PASS keyframes.name is "null"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/animations/CSSKeyframesRule-name-null.html (0 => 203753)
--- trunk/LayoutTests/animations/CSSKeyframesRule-name-null.html (rev 0)
+++ trunk/LayoutTests/animations/CSSKeyframesRule-name-null.html 2016-07-26 23:37:52 UTC (rev 203753)
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@keyframes test1 {
+ from { left: 10px; }
+ to { left: 20px; }
+ }
+</style>
+</head>
+<body>
+<script src=""
+<script>
+description("Tests that the parameters are mandatory in CSSKeyframesRule API.");
+
+var keyframes = document.styleSheets.item(0).cssRules.item(0);
+shouldBe("keyframes.__proto__", "CSSKeyframesRule.prototype");
+shouldBeEqualToString("keyframes.name", "test1");
+shouldNotThrow("keyframes.name = null");
+shouldBeEqualToString("keyframes.name", "null");
+</script>
+<script src=""
+</body>
+</html>
Added: trunk/LayoutTests/animations/CSSKeyframesRule-parameters-expected.txt (0 => 203753)
--- trunk/LayoutTests/animations/CSSKeyframesRule-parameters-expected.txt (rev 0)
+++ trunk/LayoutTests/animations/CSSKeyframesRule-parameters-expected.txt 2016-07-26 23:37:52 UTC (rev 203753)
@@ -0,0 +1,14 @@
+Tests that the parameters are mandatory in CSSKeyframesRule API.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS keyframes.__proto__ is CSSKeyframesRule.prototype
+PASS keyframes.insertRule() threw exception TypeError: Not enough arguments.
+PASS keyframes.appendRule() threw exception TypeError: Not enough arguments.
+PASS keyframes.deleteRule() threw exception TypeError: Not enough arguments.
+PASS keyframes.findRule() threw exception TypeError: Not enough arguments.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/animations/CSSKeyframesRule-parameters.html (0 => 203753)
--- trunk/LayoutTests/animations/CSSKeyframesRule-parameters.html (rev 0)
+++ trunk/LayoutTests/animations/CSSKeyframesRule-parameters.html 2016-07-26 23:37:52 UTC (rev 203753)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+@keyframes test1 {
+ from { left: 10px; }
+ to { left: 20px; }
+ }
+</style>
+</head>
+<body>
+<script src=""
+<script>
+description("Tests that the parameters are mandatory in CSSKeyframesRule API.");
+
+var keyframes = document.styleSheets.item(0).cssRules.item(0);
+shouldBe("keyframes.__proto__", "CSSKeyframesRule.prototype");
+
+shouldThrow("keyframes.insertRule()");
+shouldThrow("keyframes.appendRule()");
+shouldThrow("keyframes.deleteRule()");
+shouldThrow("keyframes.findRule()");
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (203752 => 203753)
--- trunk/Source/WebCore/ChangeLog 2016-07-26 23:36:52 UTC (rev 203752)
+++ trunk/Source/WebCore/ChangeLog 2016-07-26 23:37:52 UTC (rev 203753)
@@ -1,3 +1,30 @@
+2016-07-26 Chris Dumez <cdu...@apple.com>
+
+ Align CSSKeyframesRule with the specification
+ https://bugs.webkit.org/show_bug.cgi?id=160219
+
+ Reviewed by Darin Adler.
+
+ Align CSSKeyframesRule with the specification:
+ - https://drafts.csswg.org/css-animations/#interface-csskeyframesrule
+
+ In particular, the parameter to insertRule() / appendRule() /
+ deleteRule() / findRule() should be mandatory. Both Firefox and Chrome
+ agree with the specification here.
+
+ Also, the CSSKeyframesRule.name attribute should not be nullable.
+ Chrome agrees with the specification. However, Firefox, has the
+ attribute nullable. This patch aligns our behavior with Chrome and
+ the specification.
+
+ Tests: animations/CSSKeyframesRule-name-null.html
+ animations/CSSKeyframesRule-parameters.html
+
+ * css/CSSKeyframesRule.h:
+ (WebCore::StyleRuleKeyframes::name):
+ (WebCore::StyleRuleKeyframes::setName):
+ * css/CSSKeyframesRule.idl:
+
2016-07-26 Myles C. Maxfield <mmaxfi...@apple.com>
[iPhone] Playing a video on tudou.com plays only sound, no video
Modified: trunk/Source/WebCore/css/CSSKeyframesRule.h (203752 => 203753)
--- trunk/Source/WebCore/css/CSSKeyframesRule.h 2016-07-26 23:36:52 UTC (rev 203752)
+++ trunk/Source/WebCore/css/CSSKeyframesRule.h 2016-07-26 23:37:52 UTC (rev 203753)
@@ -50,8 +50,8 @@
void wrapperAppendKeyframe(Ref<StyleKeyframe>&&);
void wrapperRemoveKeyframe(unsigned);
- String name() const { return m_name; }
- void setName(const String& name) { m_name = AtomicString(name); }
+ const AtomicString& name() const { return m_name; }
+ void setName(const AtomicString& name) { m_name = name; }
size_t findKeyframeIndex(const String& key) const;
Modified: trunk/Source/WebCore/css/CSSKeyframesRule.idl (203752 => 203753)
--- trunk/Source/WebCore/css/CSSKeyframesRule.idl 2016-07-26 23:36:52 UTC (rev 203752)
+++ trunk/Source/WebCore/css/CSSKeyframesRule.idl 2016-07-26 23:37:52 UTC (rev 203753)
@@ -27,14 +27,13 @@
*/
interface CSSKeyframesRule : CSSRule {
- attribute DOMString? name;
+ attribute DOMString name;
readonly attribute CSSRuleList cssRules;
- // FIXME: Using "undefined" as default parameter value is wrong.
- void insertRule(optional DOMString rule = "undefined");
- void appendRule(optional DOMString rule = "undefined");
- void deleteRule(optional DOMString key = "undefined");
- CSSKeyframeRule findRule(optional DOMString key = "undefined");
+ void insertRule(DOMString rule);
+ void appendRule(DOMString rule);
+ void deleteRule(DOMString key);
+ CSSKeyframeRule? findRule(DOMString key);
getter CSSKeyframeRule (unsigned long index);
};
