Closed Bug 977554 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: Theme, defect)

30 Branch
x86_64
Windows 7
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 30
Tracking Status
firefox29 --- fixed
firefox30 --- fixed

People

(Reporter: alice0775, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P3-])

Attachments

(1 file, 1 obsolete file)

>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;
  }
Darrin, what was your intent for the timing function here?
Flags: needinfo?(dhenein)
Whiteboard: [Australis:P3-]
Attachment #8384909 - Flags: review?(jaws)
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
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.
Attachment #8384909 - Flags: review?(jaws) → review+
(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?
Flags: needinfo?(shorlander)
(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.
Flags: needinfo?(shorlander)
Flags: needinfo?(dhenein)
Checked with Stephen, this should be what we want.
Attachment #8384978 - Flags: review?(jaws)
Attachment #8384909 - Attachment is obsolete: true
Comment on attachment 8384978 [details] [diff] [review]
bookmark animation for Australis added syntax error,

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

perfect, thanks.
Attachment #8384978 - Flags: review?(jaws) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/bac0e16887b6
Whiteboard: [Australis:P3-] → [Australis:P3-][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/bac0e16887b6
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P3-][fixed-in-fx-team] → [Australis:P3-]
Target Milestone: --- → Firefox 30
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
Attachment #8384978 - Flags: approval-mozilla-aurora?
Attachment #8384978 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
QA Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.