Check document.readyState === "interactive" on $(document).ready #2100

Closed
stackia opened this Issue Feb 17, 2015 · 10 comments

Comments

Projects
None yet
5 participants
@stackia

stackia commented Feb 17, 2015

https://github.com/jquery/jquery/blob/master/src/core/ready.js#L81

As the comment mentioned, IE9 will probably give an incorrect "interactive" readyState. But that only affects IE9. Should "interactive" be accepted on other browsers? I'm using require.js to load my scripts and exactly jQuery and many other scripts are loaded after DOMContentLoaded but before fully loaded. Scripts depending on $(document).ready have to wait until load event is triggered.

Currently I use document.readyState === "interactive" ? init() : $(document).ready(init); to get around this problem.

https://github.com/ded/domready/blob/master/src/ready.js#L16
This is another implementation of domReady which accept "interactive" readyState on browsers excluding IE.

Is there any reason why jQuery doesn't do that?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Feb 17, 2015

Member

To recast the question in the form of a bug/feature, I think it would be whether jQuery should support being lazy-loaded so that it would run .ready() handlers before window.onload fires. You can read through http://bugs.jquery.com/ticket/10067 for background. It seems like we could use the .doScroll hack that domready uses.

Member

dmethvin commented Feb 17, 2015

To recast the question in the form of a bug/feature, I think it would be whether jQuery should support being lazy-loaded so that it would run .ready() handlers before window.onload fires. You can read through http://bugs.jquery.com/ticket/10067 for background. It seems like we could use the .doScroll hack that domready uses.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Feb 17, 2015

Member

It seems like we could use the .doScroll hack that domready uses.

What doScroll hack? ;) It was needed for IE<8, now the code looks like that.

Member

mgol commented Feb 17, 2015

It seems like we could use the .doScroll hack that domready uses.

What doScroll hack? ;) It was needed for IE<8, now the code looks like that.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Feb 17, 2015

Member

For reference, this is the mentioned hack (last commit before dropping IE<8):

jquery/src/core/ready.js

Lines 125 to 152 in a467f86

// If IE and not a frame
// continually check to see if the document is ready
var top = false;
try {
top = window.frameElement == null && document.documentElement;
} catch ( e ) {}
if ( top && top.doScroll ) {
(function doScrollCheck() {
if ( !jQuery.isReady ) {
try {
// Use the trick by Diego Perini
// http://javascript.nwbox.com/IEContentLoaded/
top.doScroll("left");
} catch ( e ) {
return setTimeout( doScrollCheck, 50 );
}
// detach all dom ready events
detach();
// and execute any waiting functions
jQuery.ready();
}
})();
}

Member

mgol commented Feb 17, 2015

For reference, this is the mentioned hack (last commit before dropping IE<8):

jquery/src/core/ready.js

Lines 125 to 152 in a467f86

// If IE and not a frame
// continually check to see if the document is ready
var top = false;
try {
top = window.frameElement == null && document.documentElement;
} catch ( e ) {}
if ( top && top.doScroll ) {
(function doScrollCheck() {
if ( !jQuery.isReady ) {
try {
// Use the trick by Diego Perini
// http://javascript.nwbox.com/IEContentLoaded/
top.doScroll("left");
} catch ( e ) {
return setTimeout( doScrollCheck, 50 );
}
// detach all dom ready events
detach();
// and execute any waiting functions
jQuery.ready();
}
})();
}

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Feb 17, 2015

Member

I meant the check for .doScroll() they use in domready. It turns out that method went away in IE11, so by using that as an indirect feature check you only penalize IE9 and IE10. At least that's what I think it would mean.

Member

dmethvin commented Feb 17, 2015

I meant the check for .doScroll() they use in domready. It turns out that method went away in IE11, so by using that as an indirect feature check you only penalize IE9 and IE10. At least that's what I think it would mean.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Feb 17, 2015

Member

Ah, the library domready, not our core/ready. ;) Sorry!

Member

mgol commented Feb 17, 2015

Ah, the library domready, not our core/ready. ;) Sorry!

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Oct 17, 2015

Member

@dmethvin To sum it up, we're thinking we could check if "interactive" as long as we use a scroll hack to determine interactive where not supported?

Member

timmywil commented Oct 17, 2015

@dmethvin To sum it up, we're thinking we could check if "interactive" as long as we use a scroll hack to determine interactive where not supported?

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Oct 17, 2015

Member

Even nicer now. If document.body.doScroll exists we assume that we are in IE8-10 and do not allow our .ready() to fire on document.readyState === "interactive" because those versions of IE don't work properly with it. In all other cases we allow interactive.

Member

dmethvin commented Oct 17, 2015

Even nicer now. If document.body.doScroll exists we assume that we are in IE8-10 and do not allow our .ready() to fire on document.readyState === "interactive" because those versions of IE don't work properly with it. In all other cases we allow interactive.

@mikesherov

This comment has been minimized.

Show comment
Hide comment
@mikesherov

mikesherov Oct 17, 2015

Member

As the original author of the interactive check, there are other gotchas to consider here. There is also the case of flushed partial content creating an interactive state without the full html being loading. Whether that was oldIE only or not, I don't remember, but need to check that.

Also, there was the case where readyState interactive caused bottom of page scripts that accidentally relied on out of sync operations while combined with ready firing synchronously. Consider the following code:

<html>
<body>
<script>
var foo;

$(function() {
  alert(foo);
});

foo = 'hi';
</script>

This always alerts hi in current jQuery. When I implemented readyState == 'interactive', there were cases where undefined was being alerted. Now, this may no longer be a problem if .ready() always fires async, but at the time, jQuery deferred were synchronous.

Just food for thought, and edge cases to consider.

Member

mikesherov commented Oct 17, 2015

As the original author of the interactive check, there are other gotchas to consider here. There is also the case of flushed partial content creating an interactive state without the full html being loading. Whether that was oldIE only or not, I don't remember, but need to check that.

Also, there was the case where readyState interactive caused bottom of page scripts that accidentally relied on out of sync operations while combined with ready firing synchronously. Consider the following code:

<html>
<body>
<script>
var foo;

$(function() {
  alert(foo);
});

foo = 'hi';
</script>

This always alerts hi in current jQuery. When I implemented readyState == 'interactive', there were cases where undefined was being alerted. Now, this may no longer be a problem if .ready() always fires async, but at the time, jQuery deferred were synchronous.

Just food for thought, and edge cases to consider.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Oct 17, 2015

Member

Thanks @mikesherov, fortunately I know we have at least one good unit test for this. The question is whether IE11 or Edge still has this bug, I believe those were the only places where interactive caused problems.

Member

dmethvin commented Oct 17, 2015

Thanks @mikesherov, fortunately I know we have at least one good unit test for this. The question is whether IE11 or Edge still has this bug, I believe those were the only places where interactive caused problems.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Oct 18, 2015

Member

Just for reference, the test and the iframe file. Whoa, was that really 2012? Time flies.

Member

dmethvin commented Oct 18, 2015

Just for reference, the test and the iframe file. Whoa, was that really 2012? Time flies.

timmywil added a commit to timmywil/jquery that referenced this issue Oct 28, 2015

timmywil added a commit to timmywil/jquery that referenced this issue Oct 28, 2015

timmywil added a commit to timmywil/jquery that referenced this issue Oct 28, 2015

timmywil added a commit to timmywil/jquery that referenced this issue Oct 29, 2015

@timmywil timmywil closed this in #2678 Oct 29, 2015

@dmethvin dmethvin modified the milestones: 1.12/2.2, 3.0.0 Jan 7, 2016

dracos added a commit to dracos/jquery that referenced this issue Jan 13, 2016

@cssmagic cssmagic referenced this issue in cssmagic/ChangeLog May 18, 2016

Open

jQuery #5

nateberkopec added a commit to nateberkopec/turbolinks that referenced this issue May 3, 2017

Fix DOM Ready check in IE10
Apparently IE10 can make the document "interactive" too early.

See jquery/jquery#2100

nateberkopec added a commit to nateberkopec/turbolinks that referenced this issue May 3, 2017

Fix DOM Ready check in IE10
Apparently IE10 can make the document "interactive" too early.

See jquery/jquery#2100

@jquery jquery locked as resolved and limited conversation to collaborators Jun 19, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.