The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla21

Status

()

Core
DOM: CSS Object Model
P3
normal
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: jlongster, Assigned: dbaron)

Tracking

({dev-doc-complete})

unspecified
mozilla21
dev-doc-complete
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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`.
(Assignee)

Comment 1

4 years ago
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.
(Reporter)

Comment 2

4 years ago
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

4 years ago
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.
(Assignee)

Comment 4

4 years ago
Created attachment 714728 [details] [diff] [review]
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.
Attachment #714728 - Flags: review?(bzbarsky)
(Assignee)

Updated

4 years ago
Assignee: nobody → dbaron
(Assignee)

Updated

4 years ago
Status: NEW → ASSIGNED
OS: Mac OS X → All
Priority: -- → P3
Hardware: x86 → All
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?
Attachment #714728 - Flags: review?(bzbarsky) → review+

Comment 6

4 years ago
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).
(Assignee)

Comment 7

4 years ago
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

4 years ago
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

4 years ago
^as if there could be...
> The order of the rules inside @keyframes doesn't actually matter.

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

Comment 11

4 years ago
(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.
(Assignee)

Comment 12

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/b9dac8026003
https://hg.mozilla.org/mozilla-central/rev/b9dac8026003
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
https://developer.mozilla.org/en-US/docs/Site_Compatibility_for_Firefox_21
https://developer.mozilla.org/en-US/docs/DOM/CSSKeyframesRule
Keywords: dev-doc-complete
You need to log in before you can comment on or make changes to this bug.