Bug Tracker

Opened 11 years ago

Closed 9 years ago

#2186 closed bug (invalid)

Custom Animation function does not correctly handle unit that is an empty string

Reported by: openg Owned by:
Priority: minor Milestone: 1.2.4
Component: effects Version: 1.2.2
Keywords: Cc:
Blocked by: Blocking:



Custom Animation function does not correctly handle the "unit" parameter if passed in an empty string

External Behavior

I first discovered this when I attempted to do something simple like this:

$("#someDiv").animate({color: "red"}, 1000);

Nothing happens when executing that. (I forgot that jQuery doesn't support animation tweening between color values outside of plugins, but there is still a bug =)


My first clue of what the problem was when my Firebug console spat out an error:

Expected color but found 'redpx'. Error in parsing value for property 'color'. Declaration dropped.

Digging into the code, in the uncompressed version of jQuery 1.2.2, we have:

2935    animate: function( prop, speed, easing, callback ) {
2936        var optall = jQuery.speed(speed, easing, callback);
2938        return this[ optall.queue === false ? "each" : "queue" ](function(){

2963            jQuery.each( prop, function(name, val){
2964                var e = new jQuery.fx( self, opt, name );
2966                if ( /toggle|show|hide/.test(val) )
2967                    e[ val == "toggle" ? hidden ? "show" : "hide" : val ]( prop );
2968                else {
2969                    var parts = val.toString().match(/^([+-]=)?([\d+-.]+)(.*)$/),
2970                        start = e.cur(true) || 0;
2972                    if ( parts ) {
2988                    } else
2989                        e.custom( start, val, "" );
2990                }
2991            });
3122  jQuery.fx.prototype = {
3145      // Start an animation from one number to another
3146      custom: function(from, to, unit){
3147          this.startTime = (new Date()).getTime();
3148          this.start = from;
3149          this.end = to;
3150          this.unit = unit || this.unit || "px";

The problem is in lines 2989 and 3150.

On line 1989, an empty string is passed in as unit to custom.

On line 3150, the empty string is evaluated as falsey, this.unit is undefined, thus a unit of "px" is used.

Simply changing line 2989 to pass in a space instead of the empty string: e.custom( start, val, " ") fixed the issue and demonstrated that this indeed is the problem.

Note that although this bug was discovered when I tried to animate color in a way that wasn't meant to be used, this bug will manifest itself any time the code path executes line 2989.

Change History (3)

comment:1 Changed 11 years ago by davidserduke

Milestone: 1.2.3

Do you have a test case you expect to pass that fails? If so please post it. I've only done a bit with animate() but it is my understanding that "px" is the default for jQuery. So if nothing is passed in (or "" in this case) it will choose "px" and that is by design.

I could certainly be wrong so feel free to post a test case that displays the unwanted behavior.


comment:2 Changed 11 years ago by flesler

need: ReviewTest Case

comment:3 Changed 9 years ago by dmethvin

Resolution: invalid
Status: newclosed

Since .animate() is documented to animate only numeric properties, this is a case of bad input to the method. I'll close it since it's not a bug.

Note: See TracTickets for help on using tickets.