Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First attempt at fixing #7912: $.fx.prototype.cur() is incompatible with cssHooks #165

Closed
wants to merge 8 commits into from

Conversation

louisremi
Copy link
Contributor

The complete bug report is to be found here: http://bugs.jquery.com/ticket/7912

// Empty strings and "auto" are converted to 0,
// complex values such as "rotate(1rad)" are returned as is,
// simple values such as "10px" are parsed to Float.
return r === "" || r === "auto"? 0 : isNaN( parsed = parseFloat(r) )? r : parsed;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using $.isNaN here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$.isNaN mostly handles the case of isNaN("")==false which doesn't apply here since parseFloat("") returns NaN and the "" case is checked earlier anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jQuery.isNaN() is only faster for values != NaN, and parsFloat returns exactly NaN when it fails.

// Empty strings and "auto" are converted to 0,
// complex values such as "rotate(1rad)" are returned as is,
// simple values such as "10px" are parsed to Float.
return r === "" || r === "auto" ? 0 : isNaN( parsed = parseFloat(r) ) ? r : parsed;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I edited this comment multiple times. Sorry if you read it in between <<

This code won't work in IE6. Just try running the test suite there and it will fail all over the place, because IE6 also returns undefined and null . I suggest this line instead
jQuery.isNaN( parsed = parseFloat( r ) ) ? !r || r === "auto" ? 0 : r : parsed;
It's switched around to make the common case (r is something number-like) faster. If r is falsy or auto it returns 0 else it returns the unparsed value.

P.S. You seem to have a strange character in there after the ||

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comments and tests!

(Aptana is driving me nuts, I'm currently searching for another IDE)

@louisremi
Copy link
Contributor Author

As discussed earlier, I'm using isNaN instead of jQuery.isNaN because it is faster in this case.

@timmywil
Copy link
Member

lrbabe, did you see jitter's updated comment?

@jitter
Copy link
Contributor

jitter commented Feb 18, 2011

Landed b9f5e2b

@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants