841896 - CSSKeyframesRule should have a `appendRule` method, not `insertRule`

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect, P3)

Description

[James Long (:jlongster)]

Currently a CSSKeyframesRule objects in javascript has a `insertRule` method, like a normal CSSRule object. This is wrong according to the spec:

http://www.w3.org/TR/css3-animations/#DOM-CSSKeyframesRule

MDN succinctly displays this confusion: https://developer.mozilla.org/en-US/docs/DOM/CSSKeyframesRule

It lists the methods at the top, where `appendRule` is defined, but then when describing the methods is uses `insertRule`.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This was a spec change based on my feedback: https://dvcs.w3.org/hg/csswg/rev/08c0c83446dd .  I need to go back and go through all the spec changes at some point.

Comment 2

[James Long (:jlongster)]

So you mean that Firefox is out-of-date with the spec? It looks like the spec change you mention changes "insertRule" to "appendRule".

Opera seems to be the only browser that uses "appendRule", but unfortunately seems to replace the whole keyframe instead of just appending to it.

Comment 3

[Maciej Jaros]

I was thinking about appendChild method and insertBefore and I think this would be more consistent:

// insert and merge styles (same as for += cssText, meaning "color:#333;left:10%" + "color:#999" effectively becomes "color:#999;left:10%")
insertIntoRule(in DOMString keyText, in DOMString cssText);
insertIntoRule(in DOMString keyText, in CSSStyleDeclaration style);

// inserting rules at specific key; replacing upon duplicate key
setRule(in DOMString keyText, in DOMString cssText);
setRule(in DOMString keyText, in CSSStyleDeclaration style);

I think neither insertBefore nore appendRule just doesn't fit in as this is a key based list.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: Rename CSSKeyframesRule.insertRule to appendRule to match spec change.

Note that this does not change the IID of nsIDOMMozCSSKeyframesRule
since neither the method signature nor semantics have changed; only the
name is different.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 714728 [details] [diff] [review]
Rename CSSKeyframesRule.insertRule to appendRule to match spec change.

r=me

But shouldn't there be a way to insert keyframes in the middle too?

Comment 6

[Maciej Jaros]

If there are keys "0%" and "100%" then appendRule with key "50%" should actually insert keyframe in the middle (or set/replace style at 50% to match Presto behavior).

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It appends the rule, but it's still a rule for 50%.  The order of the rules inside @keyframes doesn't actually matter.

Comment 8

[Maciej Jaros]

Sorry, I continued the conversation from GitHub and haven't noticed a missing reference :-). We were talking about Opera/Presto implementation here:
https://github.com/jlongster/css-animations.js/issues/4

Opera is interpreting specs as there can be only one keyframe in the list with given key. It doesn't do any merge or anything, but simply replaces the style under given key. Seems legit to me given current spec. Maybe any of you could suggest spec. change (clarification at least)? :-)

Comment 9

[Maciej Jaros]

^as if there could be...

Comment 10

[Boris Zbarsky [:bzbarsky]]

> The order of the rules inside @keyframes doesn't actually matter.

Ah, that's the key bit.  Makes sense, then.

Comment 11

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Maciej Jaros from comment #8)
> Opera is interpreting specs as there can be only one keyframe in the list
> with given key. It doesn't do any merge or anything, but simply replaces the
> style under given key. Seems legit to me given current spec. Maybe any of
> you could suggest spec. change (clarification at least)? :-)

Yes, that's what the current spec says, but I've proposed changing it, as described here:
https://www.w3.org/Bugs/Public/show_bug.cgi?id=21018 , since I think it's very non-intuitive given how the rest of CSS works.

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9dac8026003

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b9dac8026003

Comment 14

[Kohei Yoshino [:kohei]]

https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_21
https://developer.mozilla.org/en-US/docs/DOM/CSSKeyframesRule