Bug Tracker


Opened 6 years ago

Closed 6 years ago

Last modified 5 years ago

#7912 closed bug (fixed)

$.fx.prototype.cur() is incompatible with cssHooks

Reported by: anonymous Owned by:
Priority: high Milestone: 1.5.1
Component: effects Version: 1.5
Keywords: Cc:
Blocked by: Blocking:


To calculate the start value of a property being animated, $.fn.animate() uses $.fx.prototype.cur(). This function then tries to access the DOM property first and, should it fail, falls back to $.css(), which is cssHooks aware. Unfortunately, the value returned by $.css() is invariably parsed as a Float, which breaks values for properties such as color, transform, gradient, etc.

var r = parseFloat( jQuery.css( this.elem, this.prop ) );
return r || 0;

Currently, cssHooks workaround this problem by re-calculating the start value on the very first step of the animation, see color.js and transform.js examples. Obviously, this solution is neither clean nor efficient.

Before returning the value, cur() could simply do the following:

var r = jQuery.css( this.elem, this.prop ),
  parsed = parseFloat(r);
return isNaN(parsed)? r : parsed;

But there's probably a good reason for it to currently return 0 when parseFloat fails. I need help finding this reason so that I can fix this bug.



Change History (9)

comment:1 Changed 6 years ago by rwaldron

  • Component changed from unfiled to css
  • Keywords needsreview added
  • Priority changed from undecided to high

Thanks for taking the time to contribute to the jQuery project! Please provide a reduced jsFiddle test case to help us assess your ticket!

Additionally, test against the latest jQuery release and the jQuery 0 GIT version to ensure the issue still exists. Be Excellent to eachother!

comment:2 Changed 6 years ago by anonymous

I've opened a pull request with the proposed code and a new unit test: https://github.com/jquery/jquery/pull/165

comment:3 Changed 6 years ago by @…

i think with this little tweak that would be backward-compatible anyway :

var r = jQuery.css( this.elem, this.prop ),
  parsed = parseFloat(r) || 0;
return isNaN(parsed)? r : parsed;

comment:4 Changed 6 years ago by lrbabe

Sorry, the code you propose and the current code would have the exact same effect, the returned value will be 0 for a complex property.

I'm wondering if we were defaulting to 0 because some browsers return 'undefined' or other weird values for properties that have not been set previously...

comment:5 Changed 6 years ago by lrbabe

Of course, the 0 is here as a fallback for "" and "auto" values. Browsers never seem to return undefined...

I've updated my pull request to take this fact into account and I've also added unit tests.

Last edited 6 years ago by lrbabe (previous) (diff)

comment:6 Changed 6 years ago by dmethvin

  • Component changed from css to effects
  • Keywords needsreview removed
  • Milestone changed from 1.next to 1.5.1

comment:7 Changed 6 years ago by snover

  • Status changed from new to open

comment:8 Changed 6 years ago by louisremi

  • Resolution set to fixed
  • Status changed from open to closed

Fixes #7912. Make sure .cur() only returns 0 as fallback value when it needs to ("", auto, undefined, null).

This change makes .cur() more .cssHooks friendly. .cur() now returns the unmodified value by .css() if it isn't a number, number-alike or a value that needs a fallback to 0. This way fx.start doesn't need to be recalculated for complex values.

Changeset: 85d9343271da85fc945bf37a604873eaf247a3a7

comment:9 Changed 6 years ago by jitter

  • Version changed from git to 1.5

Please follow the bug reporting guidlines and use jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

Modify Ticket

Change Properties

E-mail address and user name can be saved in the Preferences.

Note: See TracTickets for help on using tickets.