Skip to content
Permalink
Browse files
Require that, at least, document.body exists before firing the ready … 
…event. This may stop jQuery from working on pages that don't create a document.body (such as Safari, when no body exists) but considering that we've required a document.body since 1.3 and haven't broken any pages, this doesn't appear to be a pressing issue. Used the test case written by dmethvin to verify the bug and the fix. Fixes #4320.
  • Loading branch information
@jeresig
jeresig committed Nov 9, 2009
1 parent 24e18d4 commit 262fcf7
Showing 1 changed file with 4 additions and 0 deletions.
4 src/event.js
View file
@@ -791,6 +791,10 @@ jQuery.extend({
ready: function() {
// Make sure that the DOM is not already loaded
if ( !jQuery.isReady ) {
if ( !document.body ) {
return setTimeout( jQuery.ready, 13 );
}

// Remember that the DOM is ready
jQuery.isReady = true;

18 comments on commit 262fcf7

@darwin
Copy link

@darwin darwin commented on 262fcf7 Feb 10, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I can confirm this fixed our IE8 issue with 1.3.2.

@fakedarren
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just interested - why 13ms?

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's about the maximum rate at which most browsers can update their timer. Except for Chrome, if you make it lower then it'll go as fast as it can (down to 1ms). 13ms is a pretty good time to have a consistent rate across browsers without completely decimating the CPU.

@fakedarren
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Thanks for the quick reply!

You mention it's "about" the maximum rate - do you expect a very (very very) small failure rate because of this? Do you have any analysis of it too? What's the difference between IE and others? I both like and dislike the specificity of the delay at the same time. Genuinely interested.

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've actually written about timers extensively:
http://ejohn.org/blog/how-javascript-timers-work/
http://ejohn.org/blog/analyzing-timer-performance/ part 2: http://ejohn.org/blog/revenge-of-the-timers/
http://ejohn.org/blog/javascript-in-chrome/
http://ejohn.org/blog/timer-and-interval-offset/

(This is related to the built-in time, not necessarily timers, but it's also interesting: http://ejohn.org/blog/accuracy-of-javascript-time/)

I'm not entirely sure what you mean by a failure rate - if the body doesn't exist we have to wait until it does - thus we are at the mercy of the browser here and need to use a timer to delay the execution of the callback.

@fakedarren
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, will read up.

What I meant by 'failure rate' is that if you're on a really really slow computer (old macs I guess) at what point is 13ms not enough. Why is that the threshold. Hoping your links will clear that up for me.

Again, thanks for the swift reply.

@padolsey
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@faekdarren, even if it's not enough on the first try, jQuery.ready will simply keep trying at 13ms intervals...

@subtleGradient
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd love to know if there's a specific instance where doscroll works but document.body still doesn't exist.
http://javascript.nwbox.com/IEContentLoaded/

Are there any good unit tests for domready?

Want to make sure there isn't some rare edge case that we haven't considered.

@jdalton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doscroll will not throw errors when its being called from a secondary document (as stated in the tech article linked to on Diego's page). For example if called within an iframe doscroll will not throw errors. That's why IEContentLoaded has a fallback to at least fire before the onload event fires (even if its immediately before). The document.body check is better than nothing but will produce a false positive if you use loading techniques that flush the output buffer early.

@jeresig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@subtleGradient: Actually, I haven't had many problems with the IE technique to speak of - I've actually had more problems with the browser's native DOMContentLoaded implementations firing too early. In that case I just make sure that document.body exists because we depend upon it in core.

@jdalton: Correct, it's never a good idea to rely upon a document.body check alone - which is why it serves as a good backup to the primary checks (DOMContentLoaded and the doscroll techniques).

@GarrettS
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doScroll is not documented to throw an error.

http://msdn.microsoft.com/en-us/library/ms536414(VS.85).aspx

Look down at the bottom, it looks like doScroll might work or might not work after a content update:

When the content of an element changes and causes scroll bars to display, the IHTMLElement2::doScroll method might not work correctly immediately following the content update. When this happens, you can use the IHTMLWindow2::setTimeout method to enable the browser to recognize the dynamic changes that affect scrolling.

I have observed errors from doScroll, but the main documentation page does not guarantee such errors. What if the error is fixed in 9?

@jdalton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Complimentary documentation suggests that methods/properties that require the dom to be fully parsed, doscroll is one of them, will throw an error. It has behaved this way since it was supported in IE5 or so. The long history and bits of documentation make it a pretty safe bet. We can play "what if" until the cows come home. With IE9 still supporting VBScript and other oldies I don't see this as an issue. Also the doscroll technique is always accompanied with various other fallbacks.

@GarrettS
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean this:
http://msdn.microsoft.com/en-us/library/ms531426(VS.85).aspx?

A few methods, such as doScroll, require the primary document to be completely loaded. If these methods are part of an initialization function, they should be handled when the ondocumentready event fires.

That is not the main documentation for doScroll. If the main documentation stated that doScroll would throw an error, then it would be fair to expect that. However, the documentation page for the doScroll method does not state that, and so it is not a reasonable expectation.

Yes I know the error does occur, but it is not specified to occur.

Using event delegation, the dom can be interacted with before onload fires.

jQUery users rely on the 'ready' pattern, so it must be maintained, but they could use delegation and get the added bonus of smaller, more efficient code.

@jdalton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have had this discussion before.

HTC documentation shows a president for throwing errors when the dom is expected to be fully parsed

The innerHTML property of the custom element is available
once the oncontentready event has fired. Therefore, an
event handler should be attached to this event in a
Literal Content component that retrieves the innerHTML
property of the custom element. Otherwise, an error
occurs, which indicates that the innerHTML property is not
yet available.

C++ doc shows similar API throws an error

Returns S_OK if successful, or an error value otherwise.

Or the fact that it has worked for 11 years

I do agree that event delegation is one way around this and jQuery has helper methods for that as well.

@jdalton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

*shows a precedent for throwing errors when the dom is expected to be fully parsed

@GarrettS
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a callback to an ancestor and inspect the target.

function bodyClickHandler(ev) {
  ev = ev||window.event;
  var target = ev.target || ev.srcElement;
  if( target.tagName === "PRE" ) {
    alert("winner");
  }
}

document.body.onclick = bodyClickHandler;

Sometimes you'll want to check the className or other DOM properties. hasClass, contains, findAncestorWithClass functions can be very useful there. There is hasClass functionality built in to jQuery probably almost any library. Most libraries have a contains, though many of those have problems. Even I have a bug filed for APE.dom.contains and there are also failures for contains in some less common cases in Safari 2 that I have been ignoring.

The benefits of delegation are:
faster page initialization - nothing happens until the callback is called
object can be interacted with immediately
usually works with innerHTML changes easliy

A bleeding edge hasClass function might consider using classList:
https://developer.mozilla.org/en/DOM/element.classList

@GarrettS
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RIght - lements are not always rendered completely. Things like
clientWidth might be wrong or table layout might not be done.

This is getting a little bit off-topic here and I know I started it, but you might try posting similar questions on comp.lang.javascript.

@studgeek
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As sort of predicted, this breaks ready() for XUL. I don't see workaround for developers either other than not using ready (but plugins may depend on it).
jquery-xul worked around this by changing the check to ( !document.body && ! document.activeElement). See https://github.com/ilyakharlamov/jquery-xul/blob/b556653a747cbf0ff22d520493288978892deb12/src/core.js.

Please sign in to comment.