Skip to main content

Search and Top Navigation

#4328 closed bug (fixed)

Opened March 12, 2009 08:58PM UTC

Closed June 11, 2009 01:07PM UTC

Last modified February 10, 2010 07:09AM UTC

IE6 with fx.off and highlight throws a script error

Reported by: chrisbarr Owned by:
Priority: critical Milestone: 1.8
Component: ui.effects.core Version: 1.7
Keywords: fx.off off highlight ie6 Cc:
Blocked by: Blocking:
Description

I found that when using IE6 with $.fx.off=true set and when a .effect("highlight") is called on a page load or something else other than a click it throws a script error.

It says "invalid property value" for jQuery UI when it hits this point for color animations in jQuery.effects:

fx.elem.style[attr] = "rgb(" + [

Math.max(Math.min(parseInt((fx.pos*(fx.end[0]-fx.start[0]))+fx.start[0],10),255),0),

Math.max(Math.min(parseInt((fx.pos*(fx.end[1]-fx.start[1]))+fx.start[1],10),255),0),

Math.max(Math.min(parseInt((fx.pos*(fx.end[2]-fx.start[2]))+fx.start[2],10),255),0)

].join(",") + ")";

I put together a demo here: http://jsbin.com/omeme/edit

Attachments (0)
Change History (8)

Changed March 18, 2009 01:05AM UTC by scottgonzalez comment:1

milestone: 1.next1.8

Changed March 29, 2009 02:07AM UTC by doublerebel comment:2

Fixed in Ticket 4324 attachment

To avoid redundancy, I have attached my effects core at Ticket 4324. It includes fixes for the color animation bug found in IE6+. I can't say for sure ''why'' this bug happens, but I have the fix.

Please contact me with any questions.

Changed March 30, 2009 03:08AM UTC by epascarello comment:3

why the bug happens?

Well in the case of this bug it is pretty easy to see why it happens in IE.

The state is 1 in the first pass. If never goes into the if statement. IE stores the value as HEX not the rgb tupal. The value of start is just zero and the value of end is a HEX value.

I was working on the bug in the beginning of this week and got this to work


/*
 * jQuery Color Animations
 * Copyright 2007 John Resig
 * Released under the MIT and GPL licenses.
 */

// We override the animation for all of these color styles
$.each(['backgroundColor', 'borderBottomColor', 'borderLeftColor', 'borderRightColor', 'borderTopColor', 'color', 'outlineColor'], function(i,attr){
		$.fx.step[attr] = function(fx) {

				if ( fx.state == 0 ) {
						fx.start = getColor( fx.elem, attr );
						fx.end = getRGB( fx.end );
				}
				else if(fx.state == 1 &&  fx.end.charAt(0) === "#"){  //IE bug where color code is still in hash, not tupal
			        fx.start = [0,0,0];
					fx.end = getRGB( fx.end );
				}

				fx.elem.style[attr] = "rgb(" + [
						Math.max(Math.min( parseInt((fx.pos * (fx.end[0] - fx.start[0])) + fx.start[0],10), 255), 0),
						Math.max(Math.min( parseInt((fx.pos * (fx.end[1] - fx.start[1])) + fx.start[1],10), 255), 0),
						Math.max(Math.min( parseInt((fx.pos * (fx.end[2] - fx.start[2])) + fx.start[2],10), 255), 0)
				].join(",") + ")";
			};
});

I still do not think that is the right solution [even though it does work], just have not had time to go through all of the test cases I can think of.

Changed April 08, 2009 02:57AM UTC by dunghopper comment:4

As epascarello noted, it seems that fx.state == 1 in the first pass. From looking at the jQuery code, this is the case if "gotoEnd" is passed to update() the first time... in other words, if the animation finishes before it starts because the duration was very short, or there was just too much other code to run, blocking the animation.

Since the state is not reliable, I suggest we ignore it and just check fx.start instead (since that is really what we are interested in... if it is not initialized, we need to initialize it regardless of the value of fx.state.)

I suggest:

// We override the animation for all of these color styles
$.each(['backgroundColor', 'borderBottomColor', 'borderLeftColor', 'borderRightColor', 'borderTopColor', 'color', 'outlineColor'], function(i,attr){
		$.fx.step[attr] = function(fx) {
				if ( !fx.start ) {
						fx.start = getColor( fx.elem, attr );
						fx.end = getRGB( fx.end );
				}
				fx.elem.style[attr] = "rgb(" + [
						Math.max(Math.min( parseInt((fx.pos * (fx.end[0] - fx.start[0])) + fx.start[0],10), 255), 0),
						Math.max(Math.min( parseInt((fx.pos * (fx.end[1] - fx.start[1])) + fx.start[1],10), 255), 0),
						Math.max(Math.min( parseInt((fx.pos * (fx.end[2] - fx.start[2])) + fx.start[2],10), 255), 0)
				].join(",") + ")";
			};
});

That still may not be complete, but I think it will prevent many instances of this bug.

Changed April 14, 2009 07:34AM UTC by ZaDarkSide comment:5

I suggest:


// We override the animation for all of these color styles
$.each(['backgroundColor', 'borderBottomColor', 'borderLeftColor', 'borderRightColor', 'borderTopColor', 'color', 'outlineColor'], function(i,attr){
		$.fx.step[attr] = function(fx) {
				fx.start = getColor( fx.elem, attr );
				fx.end = getRGB( fx.end );
				fx.elem.style[attr] = "rgb(" + [
						Math.max(Math.min( parseInt((fx.pos * (fx.end[0] - fx.start[0])) + fx.start[0],10), 255), 0),
						Math.max(Math.min( parseInt((fx.pos * (fx.end[1] - fx.start[1])) + fx.start[1],10), 255), 0),
						Math.max(Math.min( parseInt((fx.pos * (fx.end[2] - fx.start[2])) + fx.start[2],10), 255), 0)
				].join(",") + ")";
			};
});

I just commented:


//if ( fx.state == 0 ) {
	/*...*/
//}

Changed May 08, 2009 03:23AM UTC by wewals comment:6

I just commented:

//if ( fx.state == 0 ) {
	/*...*/
//}

I have looked into this a bit as well. Going with this method will not work as it throws off the easing because start gets reset to the current color each iteration instead of only being set once.

dunghopper's recomendation seems like a good options. I haven't checked into it as much as I would like though.

Changed May 09, 2009 07:57PM UTC by ZaDarkSide comment:7

Replying to [comment:6 wewals]:

The problem is that dunghopper's recomendation still throws an error and the whole script stops, my suggestion was tested and works for me as expected.

Changed June 11, 2009 01:07PM UTC by scottgonzalez comment:8

resolution: → fixed
status: newclosed

Fixed in r2697. The .effect() method in effects.core.js now honors jQuery.fx.off so individual effects don't need to worry about it.