<!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>[203753] 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/203753">203753</a></dd>
<dt>Author</dt> <dd>cdumez@apple.com</dd>
<dt>Date</dt> <dd>2016-07-26 16:37:52 -0700 (Tue, 26 Jul 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>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.</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="#trunkSourceWebCorecssCSSKeyframesRuleh">trunk/Source/WebCore/css/CSSKeyframesRule.h</a></li>
<li><a href="#trunkSourceWebCorecssCSSKeyframesRuleidl">trunk/Source/WebCore/css/CSSKeyframesRule.idl</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsanimationsCSSKeyframesRulenamenullexpectedtxt">trunk/LayoutTests/animations/CSSKeyframesRule-name-null-expected.txt</a></li>
<li><a href="#trunkLayoutTestsanimationsCSSKeyframesRulenamenullhtml">trunk/LayoutTests/animations/CSSKeyframesRule-name-null.html</a></li>
<li><a href="#trunkLayoutTestsanimationsCSSKeyframesRuleparametersexpectedtxt">trunk/LayoutTests/animations/CSSKeyframesRule-parameters-expected.txt</a></li>
<li><a href="#trunkLayoutTestsanimationsCSSKeyframesRuleparametershtml">trunk/LayoutTests/animations/CSSKeyframesRule-parameters.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (203752 => 203753)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog        2016-07-26 23:36:52 UTC (rev 203752)
+++ trunk/LayoutTests/ChangeLog        2016-07-26 23:37:52 UTC (rev 203753)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2016-07-26  Chris Dumez  &lt;cdumez@apple.com&gt;
+
+        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.
+
</ins><span class="cx"> 2016-07-26  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [iPhone] Playing a video on tudou.com plays only sound, no video
</span></span></pre></div>
<a id="trunkLayoutTestsanimationsCSSKeyframesRulenamenullexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/animations/CSSKeyframesRule-name-null-expected.txt (0 => 203753)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,13 @@
</span><ins>+Tests that the parameters are mandatory in CSSKeyframesRule API.
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+PASS keyframes.__proto__ is CSSKeyframesRule.prototype
+PASS keyframes.name is &quot;test1&quot;
+PASS keyframes.name = null did not throw exception.
+PASS keyframes.name is &quot;null&quot;
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
</ins></span></pre></div>
<a id="trunkLayoutTestsanimationsCSSKeyframesRulenamenullhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/animations/CSSKeyframesRule-name-null.html (0 => 203753)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,24 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;style&gt;
+@keyframes test1 {
+    from { left: 10px; }
+    to { left: 20px; }
+  }
+&lt;/style&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;script src=&quot;../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+&lt;script&gt;
+description(&quot;Tests that the parameters are mandatory in CSSKeyframesRule API.&quot;);
+
+var keyframes = document.styleSheets.item(0).cssRules.item(0);
+shouldBe(&quot;keyframes.__proto__&quot;, &quot;CSSKeyframesRule.prototype&quot;);
+shouldBeEqualToString(&quot;keyframes.name&quot;, &quot;test1&quot;);
+shouldNotThrow(&quot;keyframes.name = null&quot;);
+shouldBeEqualToString(&quot;keyframes.name&quot;, &quot;null&quot;);
+&lt;/script&gt;
+&lt;script src=&quot;../resources/js-test-post.js&quot;&gt;&lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkLayoutTestsanimationsCSSKeyframesRuleparametersexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/animations/CSSKeyframesRule-parameters-expected.txt (0 => 203753)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -0,0 +1,14 @@
</span><ins>+Tests that the parameters are mandatory in CSSKeyframesRule API.
+
+On success, you will see a series of &quot;PASS&quot; messages, followed by &quot;TEST COMPLETE&quot;.
+
+
+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
+
</ins></span></pre></div>
<a id="trunkLayoutTestsanimationsCSSKeyframesRuleparametershtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/animations/CSSKeyframesRule-parameters.html (0 => 203753)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/animations/CSSKeyframesRule-parameters.html                                (rev 0)
+++ trunk/LayoutTests/animations/CSSKeyframesRule-parameters.html        2016-07-26 23:37:52 UTC (rev 203753)
</span><span class="lines">@@ -0,0 +1,26 @@
</span><ins>+&lt;!DOCTYPE html&gt;
+&lt;html&gt;
+&lt;head&gt;
+&lt;style&gt;
+@keyframes test1 {
+    from { left: 10px; }
+    to { left: 20px; }
+  }
+&lt;/style&gt;
+&lt;/head&gt;
+&lt;body&gt;
+&lt;script src=&quot;../resources/js-test-pre.js&quot;&gt;&lt;/script&gt;
+&lt;script&gt;
+description(&quot;Tests that the parameters are mandatory in CSSKeyframesRule API.&quot;);
+
+var keyframes = document.styleSheets.item(0).cssRules.item(0);
+shouldBe(&quot;keyframes.__proto__&quot;, &quot;CSSKeyframesRule.prototype&quot;);
+
+shouldThrow(&quot;keyframes.insertRule()&quot;);
+shouldThrow(&quot;keyframes.appendRule()&quot;);
+shouldThrow(&quot;keyframes.deleteRule()&quot;);
+shouldThrow(&quot;keyframes.findRule()&quot;);
+&lt;/script&gt;
+&lt;script src=&quot;../resources/js-test-post.js&quot;&gt;&lt;/script&gt;
+&lt;/body&gt;
+&lt;/html&gt;
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (203752 => 203753)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2016-07-26  Chris Dumez  &lt;cdumez@apple.com&gt;
+
+        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:
+
</ins><span class="cx"> 2016-07-26  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [iPhone] Playing a video on tudou.com plays only sound, no video
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSKeyframesRuleh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSKeyframesRule.h (203752 => 203753)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -50,8 +50,8 @@
</span><span class="cx">     void wrapperAppendKeyframe(Ref&lt;StyleKeyframe&gt;&amp;&amp;);
</span><span class="cx">     void wrapperRemoveKeyframe(unsigned);
</span><span class="cx"> 
</span><del>-    String name() const { return m_name; }    
-    void setName(const String&amp; name) { m_name = AtomicString(name); }
</del><ins>+    const AtomicString&amp; name() const { return m_name; }
+    void setName(const AtomicString&amp; name) { m_name = name; }
</ins><span class="cx">     
</span><span class="cx">     size_t findKeyframeIndex(const String&amp; key) const;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCorecssCSSKeyframesRuleidl"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/css/CSSKeyframesRule.idl (203752 => 203753)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -27,14 +27,13 @@
</span><span class="cx">  */
</span><span class="cx"> 
</span><span class="cx"> interface CSSKeyframesRule : CSSRule {
</span><del>-    attribute DOMString? name;
</del><ins>+    attribute DOMString name;
</ins><span class="cx">     readonly attribute CSSRuleList cssRules;
</span><span class="cx">     
</span><del>-    // FIXME: Using &quot;undefined&quot; as default parameter value is wrong.
-    void insertRule(optional DOMString rule = &quot;undefined&quot;);
-    void appendRule(optional DOMString rule = &quot;undefined&quot;);
-    void deleteRule(optional DOMString key = &quot;undefined&quot;);
-    CSSKeyframeRule findRule(optional DOMString key = &quot;undefined&quot;);
</del><ins>+    void insertRule(DOMString rule);
+    void appendRule(DOMString rule);
+    void deleteRule(DOMString key);
+    CSSKeyframeRule? findRule(DOMString key);
</ins><span class="cx"> 
</span><span class="cx">     getter CSSKeyframeRule (unsigned long index);
</span><span class="cx"> };
</span></span></pre>
</div>
</div>

</body>
</html>