While the solution appears more elegant, the document.onreadystateshange 
property is not as reliable as it should be (according to dean edwards): 
http://dean.edwards.name/weblog/2005/09/busted/#comment2529

I say we stick with what we know with confidence works.

-blair

Adam Schlag wrote:
> 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
> [email protected]
> http://jquery.com/discuss/


_______________________________________________
jQuery mailing list
[email protected]
http://jquery.com/discuss/

Reply via email to