977554 - Error in parsing value for 'animation-timing-function'. Declaration dropped. browser.css:345

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Alice0775 White]

>browser.css:345

  #bookmarked-notification-anchor[notification="finish"] > #bookmarked-notification {
    background-image: url("chrome://browser/skin/places/bookmarks-notification-finish.png");
    animation: animation-bookmarkAdded 800ms;
>>  animation-timing-function: ease ease ease linear;
  }

Comment 1

[:Gijs (he/him)]

Darrin, what was your intent for the timing function here?

Comment 2

[:Gijs (he/him)]

Attachment: bookmark animation for Australis added syntax error,

Comment 3

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8384909 [details] [diff] [review]
bookmark animation for Australis added syntax error,

Review of attachment 8384909 [details] [diff] [review]:
-----------------------------------------------------------------

good to fix this syntax error, but we need to figure out if all of these values are necessary.

Comment 4

[Darrin Henein [:darrin]]

(In reply to :Gijs Kruitbosch from comment #1)
> Darrin, what was your intent for the timing function here?

Most of the CSS for the animation was taken directly from shorlander's animation spec. It seems that we only need the first two as the animation only affects 2 properties (transform and opacity) and even then, both would be 'ease', so in theory we should only need one. Maybe Stephen can comment on his intentions?

Comment 5

[:Gijs (he/him)]

(In reply to Darrin Henein [:darrin] from comment #4)
> (In reply to :Gijs Kruitbosch from comment #1)
> > Darrin, what was your intent for the timing function here?
> 
> Most of the CSS for the animation was taken directly from shorlander's
> animation spec. It seems that we only need the first two as the animation
> only affects 2 properties (transform and opacity) and even then, both would
> be 'ease', so in theory we should only need one. Maybe Stephen can comment
> on his intentions?

Unlike for transitions, the timing functions refer to intervals between keyframes, not to properties. See:

http://jsbin.com/niner/2/edit

the square remains square (although the width and height should not be the same if they were eased differently), but you can instead see a 'bump' in the middle of the animation as it switches transition function. Just checked with shorlander, and this should just be using 'ease' throughout all the animation intervals.

Comment 6

[:Gijs (he/him)]

Attachment: bookmark animation for Australis added syntax error,

Checked with Stephen, this should be what we want.

Comment 7

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8384978 [details] [diff] [review]
bookmark animation for Australis added syntax error,

Review of attachment 8384978 [details] [diff] [review]:
-----------------------------------------------------------------

perfect, thanks.

Comment 8

[:Gijs (he/him)]

remote:   https://hg.mozilla.org/integration/fx-team/rev/bac0e16887b6

Comment 9

[Carsten Book [:Tomcat]]

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

Comment 10

[:Gijs (he/him)]

Comment on attachment 8384978 [details] [diff] [review]
bookmark animation for Australis added syntax error,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis
User impact if declined: spurious CSS errors
Testing completed (on m-c, etc.): locally, on m-c
Risk to taking this patch (and alternatives if risky): none (CSS only change)
String or IDL/UUID changes made by this patch: none

Comment 11

[:Gijs (he/him)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/89e6c53ad85c