Opened 15 years ago
Closed 15 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)
Change History (10)
Changed 15 years ago by
comment:1 Changed 15 years ago by
Changed 15 years ago by
Attachment: | css.2.diff added |
---|
comment:2 Changed 15 years ago by
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 15 years ago by
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 15 years ago by
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 15 years ago by
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 15 years ago by
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 15 years ago by
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 15 years ago by
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?