Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for document.readyState === "interactive" #18

Closed
Tbstr opened this issue Jul 20, 2016 · 3 comments
Closed

Check for document.readyState === "interactive" #18

Tbstr opened this issue Jul 20, 2016 · 3 comments

Comments

@Tbstr
Copy link

Tbstr commented Jul 20, 2016

Hey there,

is there a reason for the initial check in line 91 is only for 'complete', not also for 'interactive'. Since 'interactive' marks the state of having sent the DOMContentLoaded event it seems appropriate to me.

In my setup domReady seems to initialize shortly after the DOMContentLoaded event has fired and therefore waits all the way till the window.load event fires. Because of some big images on the site this means a lot of time.

Thanks for your opinion
Tobi

@Tbstr
Copy link
Author

Tbstr commented Jul 20, 2016

Just found issue #1, sorry for that. But a check for IE9 seems like a better solution than the long delay.

Best
Tobi

@jrburke
Copy link
Member

jrburke commented Jul 24, 2016

The issue was reported for IE9, but I am not aware if it was actually fixed in later IE browsers. And traditionally user agent checks usually fail at some point given that there are some IE embedding cases that change the user agent.

I appreciate wanting the faster firing, but in this case I prefer to have a reliable if slower check than fielding issues around it firing incorrectly in some edge cases.

I encourage you just to keep a fork of the code and modify it to the browsers you know you only have to worry about. This code has not really changed in quite a while. I do not expect to do major rewrites of it, so keeping a fork is not likely to suffer from code drift.

@Tbstr
Copy link
Author

Tbstr commented Jul 24, 2016

Alright, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants