Bug Tracker

Modify

Ticket #2186 (closed bug: invalid)

Opened 5 years ago

Last modified 3 years ago

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:
Blocking: Blocked by:

Description

Summary

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 =)

Details

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);
2937
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 );
2965
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;
2971
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

comment:1 Changed 5 years ago by davidserduke

  • Milestone set to 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.

Thanks.

comment:2 Changed 5 years ago by flesler

  • need changed from Review to Test Case
  • Milestone changed from 1.2.3 to 1.2.4

comment:3 Changed 3 years ago by dmethvin

  • Status changed from new to closed
  • Resolution set to invalid

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.

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

View

Add a comment

Modify Ticket

Action
as closed
Author


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

 
Note: See TracTickets for help on using tickets.