[jQuery] A bug and an improvement...
First off, jQuery has (so far) been excellent to work with, and I wish
I had discovered it before investing time in some of the other
JavaScript toolkits that are available!
So, the bug:
I the process of running through the examples I have noticed that
Firefox's leak monitor extension reports a leak in the same bit of
code from jquery.js. I searched and apparently someone else has
reported it on the list:
http://jquery.com/pipermail/discuss_jquery.com/2006-May/005401.html
The basic premise is that any time "addEventListener" is called a
corresponding "removeEventListener" call must occur or there will be a
leak. I tested the fix from the post above and that indeed does the
trick. I haven't noticed a bug report on this, though...so, anyone
object to me filing one?
And for the improvement...
I was actually working on my own solution for a set of
"addOnLoad/addOnUnload" functions, and I came across this post by Dean
Edwards (http://dean.edwards.name/weblog/2006/06/again/) and also this
one by Jesse Skinner
(http://www.thefutureoftheweb.com/blog/2006/6/adddomloadevent), but I
didn't like the IE hack at all, and tried to find some other way...
I'm not sure if anyone else has tried this, but a simple
document.attachEvent will work instead of the messy script tag hack.
Here is what is currently in jQuery:
...
// If IE is used, use the excellent hack by Matthias Miller
// http://www.outofhanwell.com/blog/index.php?title=the_window_onload_problem_revisited
} else if ( jQuery.browser.msie ) {
// Only works if you document.write() it
document.write("<scr" + "ipt id=__ie_init defer=true " +
"src=//:><\/script>");
// Use the defer script hack
var script = document.getElementById("__ie_init");
script.onreadystatechange = function() {
if ( this.readyState != "complete" ) return;
this.parentNode.removeChild( this );
jQuery.ready();
};
// Clear from memory
script = null;
// If Safari is used
} else if ( jQuery.browser.safari ) {
...
And this is my proposed "fix", with no hacks :)
...
} else if ( jQuery.browser.msie ) {
// Only works if you document.write() it
var __ie_init = function() {
if ( document.readyState != "complete" ) return;
document.detachEvent("onreadystatechange", __ie_init);
jQuery.ready();
}
document.attachEvent("onreadystatechange", __ie_init);
// If Safari is used
} else if ( jQuery.browser.safari ) {
...
I've tested this myself with Drip and it works with no leaks. The
readyState and detachEvent could even be added in to ready to include
this new code as well as the Mozilla/Gecko fix from the other post:
$.ready = function() {
if (jQuery.browser.msie && document.readyState != "complete") return;
if( jQuery.browser.mozilla || jQuery.browser.opera ) {
document.removeEventListener( "DOMContentLoaded", $.ready, null );
} else if ( jQuery.browser.msie ) {
document.detachEvent("onreadystatechange", $.ready);
}
if ( $.$$timer ) {
clearInterval( $.$$timer );
$.$$timer = null;
for ( var i = 0; i < $.$$ready.length; i++ ) {
$.apply( document, $.$$ready[i] );
}
$.$$ready = null;
}
};
And the "hack" could simply be changed to this (which is almost
identical to the Mozilla/Opera addEventListener):
...
} else if ( jQuery.browser.msie ) {
document.attachEvent("onreadystatechange", jQuery.ready);
// If Safari is used
} else if ( jQuery.browser.safari ) {
...
Well, hope this made sense. Feel free to let me know if you have any
comments (or if this doesn't work for you). Thanks, and hopefully
this can make it in to a future version!
Adam
_______________________________________________
jQuery mailing list
discuss@jquery.com
http://jquery.com/discuss/