Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixes #10067 Introducing.... jQuery.quickReady #736

Closed
wants to merge 8 commits into
from
View
@@ -384,6 +384,9 @@ jQuery.extend({
// the ready event fires. See #6781
readyWait: 1,
+ // should we fire ready on readyState "interactive" ?
+ quickReady: true,
+
// Hold (or release) the ready event
holdReady: function( hold ) {
if ( hold ) {
@@ -395,6 +398,12 @@ jQuery.extend({
// Handle when the DOM is ready
ready: function( wait ) {
+ // user wasn't necessarily given the chance to set jQuery.quickReady before bindReady
+ // so we check here for quickReady instead
+ if ( !jQuery.quickReady && document.readyState === "interactive" ) {
+ return;
+ }
+
// Either a released hold or an DOMready/load event and not yet ready
if ( (wait === true && !--jQuery.readyWait) || (wait !== true && !jQuery.isReady) ) {
// Make sure body exists, at least, in case IE gets a little overzealous (ticket #5443).
@@ -429,9 +438,9 @@ jQuery.extend({
// Catch cases where $(document).ready() is called after the
// browser event has already occurred.
- if ( document.readyState === "complete" ) {
+ if ( document.readyState !== "loading" ) {
// Handle it asynchronously to allow scripts the opportunity to delay ready
- return setTimeout( jQuery.ready, 1 );
+ setTimeout( jQuery.ready, 1 );
@dmethvin

dmethvin Apr 13, 2012

Owner

Do you really want to fall through this? Trying to wrap my head around this since we're also in a !== "loading" situation now.

@mikesherov

mikesherov Apr 13, 2012

Member

yes, I do. I can't do the check for complete || (quickReady && interactive) here because there is no way to set jQuery.quickReady before bindReady when jQuery is loaded asynchronously. So I just check if it's not loading. This allows the user to set quickReady before the setTimeouted function fires. However, if they DON'T set quickReady, and we were returning here, the DomContentLoaded and window.load handlers would never be bound, and ready would never fire.

@dmethvin

dmethvin Apr 13, 2012

Owner

Great explanation. I figured it was okay, seemed too obvious to be a bug.

}
// Mozilla, Opera and webkit nightlies currently support this event
@@ -924,14 +933,15 @@ rootjQuery = jQuery(document);
// Cleanup functions for the document ready method
if ( document.addEventListener ) {
DOMContentLoaded = function() {
+ jQuery.quickReady = true;
document.removeEventListener( "DOMContentLoaded", DOMContentLoaded, false );
jQuery.ready();
};
} else if ( document.attachEvent ) {
DOMContentLoaded = function() {
// Make sure body exists, at least, in case IE gets a little overzealous (ticket #5443).
- if ( document.readyState === "complete" ) {
+ if ( document.readyState === "complete" || ( jQuery.quickReady && document.readyState === "interactive" ) ) {
document.detachEvent( "onreadystatechange", DOMContentLoaded );
jQuery.ready();
}
@@ -0,0 +1,32 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
+<html>
+<head>
+<meta http-equiv="content-type" content="text/html; charset=utf-8">
+<title>Test case for jQuery ticket #10067</title>
+<script type="text/javascript">
+
+// ready should fire callback after the iframe fires the callback
+setTimeout(function () {
+ el = document.createElement('script');
+ el.type = 'text/javascript';
+ el.onload = function () {
+ jQuery.quickReady = false;
+ jQuery(document).ready(function () {
+ // unfortunately, Opera 11.6 and lower has a bug where
@mikesherov

mikesherov Apr 16, 2012

Member

@miketaylr, thought you might be interested in this...

@miketaylr

miketaylr Apr 16, 2012

So this bug doesn't exist in Opera Next, presumably?

@mikesherov

mikesherov Apr 16, 2012

Member

I have not tested in next.

@miketaylr

miketaylr Apr 16, 2012

It would be helpful, if you get a moment. -> http://www.opera.com/browser/next/

@mikesherov

mikesherov Apr 16, 2012

Member

No problem. Will do shortly, and report back.

@mikesherov

mikesherov Apr 16, 2012

Member

@miketaylr, still happening in Opera Next. To summarize: when a sub-resource like an IFRAME takes a while to load but the DOM of the page is ready, the other browsers report "interactive" as the readyState, while Opera reports "complete".

Specifically, http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html#current-document-readiness prescribes the following non-normatively:

document . readyState
Returns "loading" while the Document is loading, "interactive" once it is finished parsing but still loading sub-resources, and "complete" once it has loaded.

The readystatechange event fires on the Document object when this value changes.
+ // document.readyState is "complete" before all subresources
+ // are loaded, so we need this check here for tests to pass
+ if ( document.readyState !== "complete" ) {
+ window.parent.iframeCallback(false);
+ }
+ });
+ }
+ document.getElementsByTagName('head')[0].appendChild(el);
+ el.src = "../include_js.php";
+}, 1000);
+</script>
+</head>
+<body>
+<!-- long loading iframe -->
+<iframe src="longLoad.php?sleep=3&return=true" style="width: 1px; height: 1px"></iframe>
+</body>
+</html>
@@ -0,0 +1,30 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
+<html>
+<head>
+<meta http-equiv="content-type" content="text/html; charset=utf-8">
+<title>Test case for jQuery ticket #10067</title>
+<script type="text/javascript">
+// browsers that implement the non-standard event API will load the iframe
+// before loading up jQuery, so quickReady has no effect here
+if( document.attachEvent ){
+ window.parent.iframeCallback(true);
+} else {
+ setTimeout(function () {
+ el = document.createElement('script');
+ el.type = 'text/javascript';
+ el.onload = function () {
+ jQuery(document).ready(function () {
+ window.parent.iframeCallback(true);
+ });
+ }
+ document.getElementsByTagName('head')[0].appendChild(el);
+ el.src = "../include_js.php";
+ }, 1000);
+}
+</script>
+</head>
+<body>
+<!-- long loading iframe -->
+<iframe src="longLoad.php?sleep=30&return=false" style="width: 1px; height: 1px"></iframe>
+</body>
+</html>
@@ -0,0 +1,6 @@
+<?php
+sleep((int)$_GET['sleep']);
+?>
+<script type="text/javascript">
+window.parent.parent.iframeCallback(<?php echo $_GET['return'];?>);
+</script>
@@ -0,0 +1,17 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
+<html>
+<head>
+<meta http-equiv="content-type" content="text/html; charset=utf-8">
+<title>Test case for jQuery ticket #10067</title>
+<script type="text/javascript" src="../include_js.php"></script>
+</head>
+<body>
+<script type="text/javascript">
+jQuery(document).ready(function () {
+ window.parent.iframeCallback(true);
+});
+</script>
+<!-- long loading iframe -->
+<iframe src="longLoad.php?sleep=10&return=false" style="width: 1px; height: 1px"></iframe>
+</body>
+</html>
@@ -21,7 +21,7 @@
</div>
<script>
jQuery(function() {
- window.parent.supportCallback( jQuery( "body" ).css( "backgroundColor" ), jQuery.support );
+ window.parent.iframeCallback( jQuery( "body" ).css( "backgroundColor" ), jQuery.support );
});
</script>
</body>
@@ -3,7 +3,7 @@
<body>
<script src="../include_js.php"></script>
<script>
- jQuery(function() { window.parent.supportCallback( document.compatMode, jQuery.support.boxModel ) });
+ jQuery(function() { window.parent.iframeCallback( document.compatMode, jQuery.support.boxModel ) });
</script>
</body>
</html>
@@ -1,7 +0,0 @@
-<html>
- <head>
- <script src="../include_js.php"></script>
- </head>
- <body>
- </body>
-</html>
@@ -7,14 +7,11 @@
background: url('http://s1.postimage.org/2d2r8xih0/body_background.png');
}
</style>
- <script src="../../../src/core.js"></script>
- <script src="../../../src/callbacks.js"></script>
- <script src="../../../src/deferred.js"></script>
- <script src="../../../src/support.js"></script>
+ <script src="../include_js.php"></script>
</head>
<body>
<script>
- window.parent.supportCallback();
+ window.parent.iframeCallback();
</script>
</body>
</html>
View
@@ -143,14 +143,37 @@ function url(value) {
});
function loadFixture() {
- var src = "./data/" + fileName + ".html?" + parseInt( Math.random()*1000, 10 ),
+ var src = url("./data/" + fileName + ".html"),
iframe = jQuery("<iframe />").css({
width: 500, height: 500, position: "absolute", top: -600, left: -600, visibility: "hidden"
}).appendTo("body")[0];
iframe.contentWindow.location = src;
return iframe;
}
};
+
+ this.testIframeWithCallback = function( title, fileName, func ) {
+
+ test( title, function() {
+ var iframe;
+
+ stop();
+ window.iframeCallback = function() {
+ var self = this,
+ args = arguments;
+ setTimeout(function() {
+ window.iframeCallback = undefined;
+ iframe.remove();
+ func.apply( self, args );
+ func = function() {};
+ start();
+ }, 0 );
+ };
+ iframe = jQuery( "<div/>" ).append(
+ jQuery( "<iframe/>" ).attr( "src", url("./data/" + fileName + ".html") )
+ ).appendTo( "body" );
+ });
+ }
}());
// Sandbox start for great justice
View
@@ -2794,6 +2794,26 @@ test("fixHooks extensions", function() {
jQuery.event.fixHooks.click = saved;
});
+testIframeWithCallback( "jQuery.ready sync load", "event/syncReady", function( isOk ) {
+ expect(1);
+ ok( isOk, "jQuery loaded synchronously fires ready before all sub-resources are loaded" );
+});
+
+// async loaded tests expect jQuery to be loaded as a single file
+// if we're not doing PHP concat, then we fall back to document.write
+// which breaks order of execution on async loaded files
+if ( hasPHP ) {
+ testIframeWithCallback( "jQuery.ready async load with quickReady true", "event/asyncQuickReadyTrue", function( isOk ) {
+ expect(1);
+ ok( isOk, "jQuery loaded asynchronously with quickReady true fires ready before all sub-resources are loaded" );
+ });
+
+ testIframeWithCallback( "jQuery.ready async load with quickReady false", "event/asyncQuickReadyFalse", function( isOk ) {
+ expect(1);
+ ok( isOk, "jQuery loaded asynchronously with quickReady false fires ready after all sub-resources are loaded" );
+ });
+}
+
(function(){
// This code must be run before DOM ready!
var notYetReady, noEarlyExecution,
View
@@ -1,37 +1,10 @@
module("support", { teardown: moduleTeardown });
-function supportIFrameTest( title, url, noDisplay, func ) {
-
- if ( noDisplay !== true ) {
- func = noDisplay;
- noDisplay = false;
- }
-
- test( title, function() {
- var iframe;
-
- stop();
- window.supportCallback = function() {
- var self = this,
- args = arguments;
- setTimeout( function() {
- window.supportCallback = undefined;
- iframe.remove();
- func.apply( self, args );
- start();
- }, 0 );
- };
- iframe = jQuery( "<div/>" ).css( "display", noDisplay ? "none" : "block" ).append(
- jQuery( "<iframe/>" ).attr( "src", "data/support/" + url + ".html" )
- ).appendTo( "body" );
- });
-}
-
-supportIFrameTest( "proper boxModel in compatMode CSS1Compat (IE6 and IE7)", "boxModelIE", function( compatMode, boxModel ) {
+testIframeWithCallback( "proper boxModel in compatMode CSS1Compat (IE6 and IE7)", "support/boxModelIE", function( compatMode, boxModel ) {
ok( compatMode !== "CSS1Compat" || boxModel, "boxModel properly detected" );
});
-supportIFrameTest( "body background is not lost if set prior to loading jQuery (#9238)", "bodyBackground", function( color, support ) {
+testIframeWithCallback( "body background is not lost if set prior to loading jQuery (#9238)", "support/bodyBackground", function( color, support ) {
expect( 2 );
var i,
passed = true,
@@ -56,7 +29,7 @@ supportIFrameTest( "body background is not lost if set prior to loading jQuery (
ok( passed, "Same support properties" );
});
-supportIFrameTest( "A background on the testElement does not cause IE8 to crash (#9823)", "testElementCrash", function() {
+testIframeWithCallback( "A background on the testElement does not cause IE8 to crash (#9823)", "support/testElementCrash", function() {
expect(1);
ok( true, "IE8 does not crash" );
});