Spec change: https://github.com/w3c/web-animations/commit/be0227af30a9e013ed2f99f6824f9e849af9d262 This is going to involve a lot of renaming test files too, I expect.

MozReview-Commit-ID: GwLLY39l1KE

Comment on attachment 8751578 [details] [diff] [review] Rename KeyframeEffectReadOnly.getFrames() and KeyframeEffect.setFrames() to getKeyframes()/setKeyframes() Hi Olli, can you please review the webidl changes here. Thanks.

Comment on attachment 8751578 [details] [diff] [review] Rename KeyframeEffectReadOnly.getFrames() and KeyframeEffect.setFrames() to getKeyframes()/setKeyframes() Review of attachment 8751578 [details] [diff] [review]: ----------------------------------------------------------------- Are you going to rename below files in a subsequent patch? I think we should rename them in this patch. dom/animation/test/css-transitions/test_keyframeeffect-getframes.html dom/animation/test/css-transitions/file_keyframeeffect-getframes.html dom/animation/test/css-animations/test_keyframeeffect-getframes.html dom/animation/test/css-animations/file_keyframeeffect-getframes.html There are two remainings in comment. Yay! This reminds me that I have to rewrite it! https://dxr.mozilla.org/mozilla-central/rev/3461f3cae78495f100a0f7d3d2e0b89292d3ec02/dom/animation/test/chrome/test_animation_performance_warning.html#176 https://dxr.mozilla.org/mozilla-central/rev/3461f3cae78495f100a0f7d3d2e0b89292d3ec02/dom/animation/test/chrome/test_animation_performance_warning.html#352 ::: testing/web-platform/meta/MANIFEST.json @@ +35222,1 @@ > ], I guess this part is not intentional, but it's OK to me. ::: testing/web-platform/mozilla/meta/MANIFEST.json @@ +7,5 @@ > "wdspec": [] > }, > "local_changes": { > "deleted": [], > + "deleted_reftests": {}, I am not familiar with our test framework for web platform test. Where did this change come from? ::: testing/web-platform/tests/web-animations/keyframe-effect/constructor.html @@ +69,4 @@ > "KeyframeEffectReadOnly constructor in KeyframeTimingOptions"); > > test(function(t) { > var getFrame = function(composite) { nit: here is a survivor. ::: testing/web-platform/tests/web-animations/keyframe-effect/setKeyframes.html @@ +4,1 @@ > <link rel="help" href="https://w3c.github.io/web-animations/#dom-keyframeeffect-setframes"> nit: #dom-keyframeeffect-setkeyframes

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3) > Comment on attachment 8751578 [details] [diff] [review] > Rename KeyframeEffectReadOnly.getFrames() and KeyframeEffect.setFrames() to > getKeyframes()/setKeyframes() > > Review of attachment 8751578 [details] [diff] [review]: > ----------------------------------------------------------------- > > Are you going to rename below files in a subsequent patch? I think we > should rename them in this patch. > > dom/animation/test/css-transitions/test_keyframeeffect-getframes.html > dom/animation/test/css-transitions/file_keyframeeffect-getframes.html > dom/animation/test/css-animations/test_keyframeeffect-getframes.html > dom/animation/test/css-animations/file_keyframeeffect-getframes.html Good idea, will do. > There are two remainings in comment. Yay! This reminds me that I have to > rewrite it! > https://dxr.mozilla.org/mozilla-central/rev/ > 3461f3cae78495f100a0f7d3d2e0b89292d3ec02/dom/animation/test/chrome/ > test_animation_performance_warning.html#176 > https://dxr.mozilla.org/mozilla-central/rev/ > 3461f3cae78495f100a0f7d3d2e0b89292d3ec02/dom/animation/test/chrome/ > test_animation_performance_warning.html#352 Yes, I deliberately left them and filed a bug for removing that comment (bug 1272204). > ::: testing/web-platform/meta/MANIFEST.json > @@ +35222,1 @@ > > ], > > I guess this part is not intentional, but it's OK to me. > > ::: testing/web-platform/mozilla/meta/MANIFEST.json > @@ +7,5 @@ > > "wdspec": [] > > }, > > "local_changes": { > > "deleted": [], > > + "deleted_reftests": {}, > > I am not familiar with our test framework for web platform test. Where did > this change come from? These changes come from running ./mach web-platform-tests --manifest-update > ::: > testing/web-platform/tests/web-animations/keyframe-effect/constructor.html > @@ +69,4 @@ > > "KeyframeEffectReadOnly constructor in KeyframeTimingOptions"); > > > > test(function(t) { > > var getFrame = function(composite) { > > nit: here is a survivor. Thanks! > ::: > testing/web-platform/tests/web-animations/keyframe-effect/setKeyframes.html > @@ +4,1 @@ > > <link rel="help" href="https://w3c.github.io/web-animations/#dom-keyframeeffect-setframes"> > > nit: #dom-keyframeeffect-setkeyframes Thanks!

MozReview-Commit-ID: GwLLY39l1KE

(In reply to Brian Birtles (:birtles) from comment #4) > > ::: testing/web-platform/mozilla/meta/MANIFEST.json > > @@ +7,5 @@ > > > "wdspec": [] > > > }, > > > "local_changes": { > > > "deleted": [], > > > + "deleted_reftests": {}, > > > > I am not familiar with our test framework for web platform test. Where did > > this change come from? > > These changes come from running ./mach web-platform-tests --manifest-update Yes, I guessed so. What I wanted to know is which change caused this. I did not know web platform tests include reftests!

(In reply to Hiroyuki Ikezoe (:hiro) from comment #6) > (In reply to Brian Birtles (:birtles) from comment #4) > > > ::: testing/web-platform/mozilla/meta/MANIFEST.json > > > @@ +7,5 @@ > > > > "wdspec": [] > > > > }, > > > > "local_changes": { > > > > "deleted": [], > > > > + "deleted_reftests": {}, > > > > > > I am not familiar with our test framework for web platform test. Where did > > > this change come from? > > > > These changes come from running ./mach web-platform-tests --manifest-update > > Yes, I guessed so. What I wanted to know is which change caused this. I > did not know web platform tests include reftests! Yes, it does. I just presumed that the update script had been updated and started adding this. Maybe I should just manually drop this line.

Comment on attachment 8751583 [details] [diff] [review] Rename KeyframeEffectReadOnly.getFrames() and KeyframeEffect.setFrames() to getKeyframes()/setKeyframes() r+ for the .webidl.

I've renamed and updated the various MDN pages for this change.