Bug Tracker

Opened 9 years ago

Closed 9 years ago

#6718 closed bug (wontfix)

Effect fails in IE with invalid property values

Reported by: andras Owned by:
Priority: undecided Milestone: 1.4.2
Component: effects Version: 1.4.2
Keywords: fail, IE, invalid, property, value, fx Cc:
Blocked by: Blocking:

Description

At line 5902 v1.4.2 I have this line:

fx.elem[ fx.prop ] = fx.now;

I know jquery is not responsable of what the user decided to put in the property. The thing is when an invalid property arrives this step by any reason, IE fails. For example

window.top = NaN

I'd recommend to put a try-catch clause here. I had to do this because an ui component (i really don't know which, cause i'm using many of them on the same script) is trying to set an invalid property value for top.

Thank you

Change History (5)

comment:1 Changed 9 years ago by dmethvin

Resolution: invalid
Status: newclosed

Just adding try/catch around blocks of code is not a good solution; it masks the problem and makes code harder to debug. Please attach a test case.

comment:2 Changed 9 years ago by hungsimol

Resolution: invalid
Status: closedreopened

Testing jQuery UI Position in IE:

var $WRAPPER = $('#wrapper'); $('#div1').position({

my: "left top", at: "left top", of: $WRAPPER

})

jQuery 1.4.2: _default: function( fx ) {

if ( fx.elem.style && fx.elem.style[ fx.prop ] != null ) {

alert(fx.prop + ", " + fx.now + ", " + fx.unit);

fx.elem.style[ fx.prop ] = (fx.prop === "width"
fx.prop === "height" ? Math.max(0, fx.now) : fx.now) + fx.unit;

} else {

fx.elem[ fx.prop ] = fx.now;

}

}

If error: "top, NaN, px" else: "top, 0, px"

comment:3 Changed 9 years ago by hungsimol

fx.now equals NaN because $('#wrapper') in "top".

<div style="height:20px">TOP PADDING</div> <div id="wrapper">

...

</div>

Now, fx.now = 0 and it works! Please fix this problem!!!

jQuery UI Position demo works because: #targetElement { width:240px;height:200px;background-color:#999;margin:30px auto; }

Oh, #targetElement margin top 30px

comment:4 Changed 9 years ago by dmethvin

OK, so the proposal here is to try/catch the problem and then re-throw with a more specific message using jQuery.error()? Simply swallowing the error is right out in my book.

comment:5 Changed 9 years ago by snover

Priority: undecided
Resolution: wontfix
Status: reopenedclosed

For tight loops like fx, making the browser set up a try/catch for every invocation is going to make performance even worse than it already is.

Note: See TracTickets for help on using tickets.