Bug Tracker

Opened 14 years ago

Closed 14 years ago

#2071 closed bug (fixed)

1.2.2b kills IE on accordion demo page

Reported by: joern Owned by:
Priority: major Milestone: 1.2.2
Component: core Version: 1.2.1
Keywords: Cc:
Blocked by: Blocking:

Description

I get weird errors in IE 6/7 on the accordion demo page, starting with an illegal argument, sometimes also a out of memory error.

Debugger says the illegal argument error is thrown in line 1077 (elem[ name ] = value;), with name="height" and value="NaNpx". Looks like a wrong height is calculated during an animation, resulting in a NaN.

To reproduce: Checkout trunk/plugins/accordion, update lib/jquery.js to latest, load accordion/demo/ in IE and click on top left accordion.

Works fine with 1.2.1.

Attachments (2)

css.diff (1.6 KB) - added by brandon 14 years ago.
css.2.diff (2.1 KB) - added by brandon 14 years ago.

Download all attachments as: .zip

Change History (10)

Changed 14 years ago by brandon

Attachment: css.diff added

comment:1 Changed 14 years ago by brandon

The css.diff is the patch I'm currently testing locally to fix some width/height issues. Can you test to see if it fixes this ticket as well?

Changed 14 years ago by brandon

Attachment: css.2.diff added

comment:2 Changed 14 years ago by brandon

The height issue is fixed but I consistently get the "out of memory error" at page unload. It seems the unbind code is using to much of the limited memory during the unload event.

comment:3 Changed 14 years ago by brandon

Actually it doesn't even just happen at unload. If I run the unload code at any time on the demo, I get the "outer of memory error".

comment:4 Changed 14 years ago by joern

I can confirm that - height issue fixed, memory issue still there.

The unload handler gets all elements first:

jQuery(window).bind("unload", function() {
	jQuery("*").add(document).unbind();
});

Could it help to somehow reduce the selection? If I replace the "*" with eg. "a" on the demo page, the error doesn't occur anymore - but it leaks a lot of memory.

1.2.1 leaks a lot, too, but we handled that well enough in earlier versions. I've lost track of all the changes, but maybe we can backtrack some of them to see where the regression occured.

comment:5 Changed 14 years ago by brandon

We used to keep a global cache of elements that had events bound to them. That is what we would use to fix the memory leak. However, if I remember correctly (and I know John does) it was causing a slew of its own issues in regards to memory and slow downs. We definitely need to find a way to limit the scope of the final unbind call.

comment:6 Changed 14 years ago by brandon

John mentioned just going directly to the expando data cache and killing all the handlers that way. I think I'll give this a try when I get the chance ... just not sure when that will be. Feel free to try it out as well.

comment:7 Changed 14 years ago by davidserduke

I'm not sure how that can work. The cache doesn't hold a reference to the elem does it? Or are you thinking about adding that?

comment:8 Changed 14 years ago by davidserduke

Resolution: fixed
Status: newclosed

The memory bug appears to be fixed with [4226]. If there is still an issue please reopen with a new test case.

Note: See TracTickets for help on using tickets.