Side navigation
#2071 closed bug (fixed)
Opened December 17, 2007 07:22PM UTC
Closed December 19, 2007 02:05AM UTC
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)
Change History (8)
Changed December 17, 2007 07:33PM UTC by comment:1
Changed December 18, 2007 04:06AM UTC by comment:2
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.
Changed December 18, 2007 04:10AM UTC by comment:3
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".
Changed December 18, 2007 09:25AM UTC by comment:4
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.
Changed December 18, 2007 03:54PM UTC by comment:5
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.
Changed December 18, 2007 04:28PM UTC by comment:6
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.
Changed December 19, 2007 12:08AM UTC by comment:7
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?
Changed December 19, 2007 02:05AM UTC by comment:8
| resolution: | → fixed |
|---|---|
| status: | new → closed |
The memory bug appears to be fixed with [4226]. If there is still an issue please reopen with a new test case.
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?