Bug Tracker

Opened 10 years ago

Closed 10 years ago

Last modified 8 years ago

#4889 closed enhancement (fixed)

"isReady" never gets set in FF3.5 if jQuery loaded after domready

Reported by: shadedecho Owned by:
Priority: major Milestone: 1.4
Component: event Version: 1.3.2
Keywords: isReady ready bug firefox Cc:
Blocked by: Blocking:

Description

I've got a test script to show this behavior here:

http://test.getify.com/dynloadjquery/

Load FF3.5 on windows, clear your cache, then go to that page. Click "load jQuery", wait a few seconds to make sure it's there, then click "test isReady". It'll report false.

On FF3, the same test procedure would show "true". Same with IE7.

So I think this is a FF3.5 related bug with jQuery's dom ready checking.

Change History (21)

comment:1 Changed 10 years ago by shadedecho

scratch that on FF3.0... my mistake. But it definitely still says "true" in IE7. But "false" in it seems most other browsers.

So, shouldn't jQuery be able to be loaded dynamically onto a page and have this value reliably set? and set to true?

comment:2 Changed 10 years ago by shadedecho

Also, that example was appending to the BODY element. Seems "true" in all IE browsers, false everywhere else.

This example appends to the HEAD, exact same behavior.

http://test.getify.com/dynloadjquery/index2.html

comment:3 Changed 10 years ago by shadedecho

OK, Brandon just pointed me at this:

http://groups.google.com/group/jquery-dev/browse_thread/thread/05b843d512e9ae4c/88854daa08dc8d9b?lnk=raot#88854daa08dc8d9b

So, am I to understand that we have no way of reliably getting the value consistent across browsers in this use case? If jQuery is loaded dynamically, then the value is gonna be true in IE and false in every other browser!?

Ugh, that's really bad. It means that jQuery can't be loaded dynamically if there's any code that relies on $(document).ready(). That's pretty severely limiting. :(

comment:4 in reply to:  3 Changed 10 years ago by john

Resolution: wontfix
Status: newclosed
Type: bugenhancement

Replying to shadedecho:

Unfortunately, that's the hand we've been dealt. You can always load it dynamically and call jQuery.ready() to simulate the page load, instead.

In the meantime, if a technique is spotted for handling this in all browsers then I'm definitely game for adding it in (we can re-open the ticket and implement it right away).

comment:5 Changed 10 years ago by shadedecho

Resolution: wontfix
Status: closedreopened

Unfortunately, to reliably call "ready", you have to be certain that it's after page load, or you'll probably mess up the page. This creates a chicken-and-the-egg problem, obviously, since one would ideally like to rely on jquery to figure that out, not have to manually do so.

The biggest problem is that you cannot write a plugin or code (that relies on the DOM) which robustly works in both static and dynamic cases. Either you assume it's always loaded before page load, statically, and you wrap in "$(document).ready(...)". Or you assume it'll NEVER be loaded before page load, and you don't. But what about wanting to write code that'll work in either case? You can't. Big bummer. :(


I was looking at some code implemented in the latest release of SWFObject, which seems to do what we are talking about. If you look at the source of v2.2, starting around line 83, they implement some logic which allows swfobject to "wait" for things if the DOM is not ready, but they have hacks in place to set it to ready if swfobject is added to the page dynamically after load -- the same use case we are dealing with.

http://code.google.com/p/swfobject/source/browse/tags/swfobject_2_2/src/swfobject.js

The logic in question is:

if ((typeof doc.readyState != UNDEF && doc.readyState == "complete") || (typeof doc.readyState == UNDEF && (doc.getElementsByTagName("body")[0] || doc.body))) {
     callDomLoadFunctions(); 
}

callDomLoadFunctions() of course sets the internal state variable "isDomLoaded" to true (which allows the rest of the DOM-dependent library code to execute), so this logic is in place for when swfobject is being executed after being added to the page dynamically, after the domready has long since originally fired.

Is there any hope with this logic?

comment:6 Changed 10 years ago by shadedecho

I would also say that, even more important that FIXING this problem, I think it's important that jQuery have a consistent state in all browsers.

Right now, IE gets set to true, the others stay at false. This is really bad. If we determine there's simply no way to get it to true in all browsers reliably, can we at least consider falling back to false in all browsers (including IE) so that we don't have this cross-browser inconsistency?

comment:7 Changed 10 years ago by shadedecho

I've gone ahead and tried patching jQuery with the above logic taken from SWFObject's code, and my testing so far shows it to be working well. Here's the code I put at the very end of jquery.js:

(function(global,$){
  var doc = global.document, UNDEF = "undefined";
  if (
    (typeof doc.readyState != UNDEF && doc.readyState == "complete") 
    || 
    (typeof doc.readyState == UNDEF && (doc.getElementsByTagName("body")[0] || doc.body))
  ) 
    $.ready();
})(window,jQuery);

Of course, I've spaced out all that code to make it more readable, it can probably be shrunk down. But the essential idea is it tests the current environment to see if certain conditions are present, and if so, it assumes then that the dom is already present and accounted for and ready to go, and it just flags jQuery as such.

I've done simple tests in IE7/8, FF3, FF3.5, and Chrome 2 (all on windows), by dynamically adding jQuery to the page, and it seems to flag the library as domready as desired. The same code however doesn't seem to prematurely fire if the patched jquery.js is loaded during dom load with a big page still loading.

The only other detail which might be helpful is a try/catch around that code, so that if some weird condition exists where such typeof checks or what not are error-prone, at least the calls would just sliently fail and the library would continue on as normal.

Of course, there needs to be some more rigorous testing, not only in terms of testing scenarios, but also more browser testing. Maybe testswarm could help? :)

I will continue my own testing, but I'd like to know what jQuery thinks about this possible patch?

comment:8 Changed 10 years ago by john

Component: coreevent
Priority: blockermajor

So I did some more research and it looks like Firefox 3.5 adds in document.readyState support: https://bugzilla.mozilla.org/show_bug.cgi?id=347174

Thus the code addition only needs to be:

if ( doc.readyState == "complete" ) {
  return jQuery.ready();
}

I'll look in to adding this right away.

comment:9 Changed 10 years ago by john

Resolution: fixed
Status: reopenedclosed

Fixed in SVN rev [6481].

comment:10 Changed 10 years ago by shadedecho

I've added a couple of test pages for this patch.

  1. http://test.getify.com/dynloadjquery/index3.html

just like the first two tests, this one loads the patched jquery dynamically on button click, and then clicking the "test isReady" shows that it was set properly even in non-IE browsers.

  1. http://test.getify.com/dynloadjquery/index4.html

this test now instead loads the patched jquery statically, in the HEAD of a big (500kb) file, and sets up a document.ready handler to color some p-tags on domready.

  1. http://test.getify.com/dynloadjquery/index5.html

just like the previous test, but this one adds jquery (and the domready handler) at the end of the body tag.


So far, in my testing, this patch is performing as expected.

comment:11 Changed 10 years ago by shadedecho

John- I know the initial bug report was only about FF3.5, but will the code you added only help FF3.5 or will it also help Chrome, FF3, Opera, and all the other non-IE browsers that suffer the same problem? My patch seems to do so.

comment:12 in reply to:  11 Changed 10 years ago by john

Replying to shadedecho:

John- I know the initial bug report was only about FF3.5, but will the code you added only help FF3.5 or will it also help Chrome, FF3, Opera, and all the other non-IE browsers that suffer the same problem? My patch seems to do so.

My patch will work for Opera, Safari, Chrome, and IE - Firefox was the missing link. It won't work for Firefox 3.0 (and older).

Your patch won't help the situation at all - checking to see if the body element exists will fire the event way too early. It's probably better to just not run the ready than to throw exceptions.

I'm hoping developers will be aware of how this works in all the browsers that they support - I definitely plan on emphasizing that in the release notes.

At this point this is the best that we can hope for (unless we can find a technique that'll work well for older Firefoxes).

comment:13 Changed 10 years ago by shadedecho

Interesting... thanks for the input on FF3 (and below) vs FF3.5.

In my test cases above (index4 and index5), I ran them in FF3 and they worked properly. I'm curious if maybe I didn't construct a proper test case for delaying domready. I thought creating a large body (500kb) would do the trick, but apparently my test is flawed, since it works, and yet you say it shouldn't work (should fire too early).

Any thoughts on why, or how I can make a better test case where my patch in fact breaks in FF3 and below?


Also, do you think there's any hope of getting Mozilla to backport the document.readyState from 3.5 to a 3.0.13 patch?

comment:14 Changed 10 years ago by john

A good way to test DOM ready is to push a large HTML file down using PHP and do a flush(); sleep(3); half way through the file. That'll cause a ton of DOM ready techniques to fail prematurely (as the browser will temporarily construct a DOM, only to be updated when more content comes in).

Unfortunately, there's little hope for Firefox backporting a feature like this.

comment:15 Changed 10 years ago by ecmanaut

I did the above test at http://johan.dev.mashlogic.com/dynloadjquery/ and from what I see, it works just excellent on FF2.0.0.20/Mac, FF3.0.12/Mac, FF3.5.1/Mac, Safari 4.0.2/Mac, Opera 10.00 beta 2/Mac, IE7/WinXP, IE8/Vista, FF3.5/Vista, FF3.0/XP, FF3.0/Linux and uzbl/Linux, at least, with shadedecho's patch.

The flush plus delay is 5 seconds, and does trigger a pre-DOMContentLoad DOM, at least in the Mac browsers of above sample.

I am also interested in a cross-browser $(document).ready(fn), cross-invocation-mode (inline, or dynamically added after the DOM is ready), where in the latter case, the callback is just fired off immediately.

comment:16 Changed 10 years ago by ecmanaut

Ah, Chromium 3.0.196/Linux likes it too. (I'll quit the spammage, though.)

comment:17 Changed 10 years ago by shadedecho

john- correct me if i'm wrong, but it seems like what we need to test would be to force out the pre-dom-ready event (with the flush/delay as you suggest), and then, in that delay period, add jquery to the page, and see whether the patch I provided with the "body" checks will work properly or not in FF (and others).

The key question at hand is will my patch give false positives in that scenario and trigger jquery to think the dom is ready before it really is.

I will attempt the scenario you suggest and see how the two patches in question behave. Thanks.

comment:18 Changed 9 years ago by SlexAxton

#4196 is a duplicate of this ticket.

comment:19 Changed 9 years ago by Loren

Here's a sample to complete for loading JQuery with JavaScript for use with widgets. This allows the widget to use the jQuery include file from the header if it already exists, or insert one if needed.

http://jsbin.com/acido4/4/edit

comment:20 Changed 9 years ago by shadedecho

AFAICT, the code you posted fails in IE because IE does *not* block the window.onload event for dynamically loaded scripts, as does other browsers.

comment:21 Changed 9 years ago by Loren <loren@…>

I've rewritten using document.addEventListener to load before window.onload in Firefox and Opera. What needs to be added for Chrome?

Also loads before window.onload in IE using setTimeout. http://neighborhood.org/core/sample/jquery/append-to-head.htm

Note: See TracTickets for help on using tickets.