1033881 - CSS Animations should not dispatch events when the animation-name doesn't match

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Brian Birtles (:birtles)]

In bug 1004377, I made CSS animations start dispatching events when the corresponding @keyframes rule is empty. This basically consisted of removing some occasions where we skipped event dispatch when the array of keyframes was empty.

However, when we encounter an animation-name that doesn't match any @keyframes rule we would still generate an ElementAnimation with an empty set of keyframes. Prior to bug 1004377 we would not dispatch events in this case but now we do.

The spec is pretty clear that we shouldn't:

  Any animation for which both a valid keyframe rule and a non-zero duration are defined will run and generate events; this includes animations with empty keyframe rules.
  http://dev.w3.org/csswg/css-animations/#events

The non-zero duration condition is wrong but the rest makes sense. I've tested Chrome and IE and they don't dispatch events in this case but they also don't dispatch events for an empty keyframe rule which the spec clearly requires.

Bug 1031319 fixes this behavior for when animation-name is none or empty but not for non-matching names.

Comment 1

[Brian Birtles (:birtles)]

Attachment: part 1 - Don't generate animations when the animation-name doesn't match

When animation-name does not match a keyframes rule, we should not dispatch
animation events as per:

  "Any animation for which both a valid keyframe rule and a non-zero duration
  are defined will run and generate events; this includes animations with empty
  keyframe rules."
  http://dev.w3.org/csswg/css-animations/#events

Since bug 1004377, however, we started dispatching events in this case because
we no longer ignore animations whose set of keyframes is empty.

This patch checks for a matching keyframes rule in BuildAnimations and if one is
not found, no corresponding animation is generated.

Comment 2

[Brian Birtles (:birtles)]

Attachment: part 2 - Add a test to test_element-get-animation-players.html which checks for animation-name: missing

Depending on which lands first, this or bug 1032573, this patch might have to be landed separately

Comment 3

[Brian Birtles (:birtles)]

On further thought, I'm not entirely sure what the correct behavior should be if the referred-to @keyframes rule is added later. Should the animation start then or not? The spec doesn't seem to answer this. Might be a question for www-style.

Comment 4

[Brian Birtles (:birtles)]

(In reply to Brian Birtles (:birtles) from comment #3)
> On further thought, I'm not entirely sure what the correct behavior should
> be if the referred-to @keyframes rule is added later. Should the animation
> start then or not? The spec doesn't seem to answer this. Might be a question
> for www-style.

http://lists.w3.org/Archives/Public/www-style/2014Jul/0044.html

Note that attachment 8449911 [details] [diff] [review] from this bug makes us do (b) in that mail, the same as Chrome.

Comment 5

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

Comment on attachment 8449911 [details] [diff] [review]
part 1 - Don't generate animations when the animation-name doesn't match

r=dbaron

Comment 6

[Brian Birtles (:birtles)]

Attachment: part 2 - Add tests to test_element-get-animation-players.html which checks for animation-name: missing

Comment 7

[Brian Birtles (:birtles)]

Try (part 1 only): https://tbpl.mozilla.org/?tree=Try&rev=0d4fdf6243f4

Comment 8

[Brian Birtles (:birtles)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4aa67e18ed7f

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4aa67e18ed7f

Comment 10

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

Comment on attachment 8452125 [details] [diff] [review]
part 2 - Add tests to test_element-get-animation-players.html which checks for animation-name: missing

It would probably be good to remove the @keyframes notyet rule from the stylesheet when you're done with it.

r=dbaron

Comment 11

[Brian Birtles (:birtles)]

(In reply to David Baron [:dbaron] (UTC-7) (needinfo? for questions) from comment #10)
> It would probably be good to remove the @keyframes notyet rule from the
> stylesheet when you're done with it.

Fixed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/40a3b13a460b

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/40a3b13a460b