Skip to content

Core: use interactive to evaluate dom ready #2678

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 29, 2015
Merged

Core: use interactive to evaluate dom ready #2678

merged 1 commit into from
Oct 29, 2015

Conversation

timmywil
Copy link
Member

  • Now that dom ready is always synchronous, we don't have to deal with ensuring sync execution order.
  • I actually couldn't reproduce the IE9 issue where DOM below scripts is not actually interactive, but I ran into a different issue with IE10 (I think it was mostly a test issue, but it had to do with document.write behaving differently so I didn't mess with it). Either way, I just kept the proposed solution of checking doScroll, effectively excluding IE9-10.

Fixes gh-2100

@timmywil timmywil changed the title WIP: Core: use interactive to evaluate dom ready Core: use interactive to evaluate dom ready Oct 28, 2015
@timmywil
Copy link
Member Author

@dmethvin @mikesherov Would like feedback from you two specifically, given your familiarity with the old tickets.

@Krinkle
Copy link
Member

Krinkle commented Oct 28, 2015

Hm.. so in the old code if we run after DOMContentLoaded (readyState interactive) but before window-load (readyState complete) then it wouldn't trigger immediately but fall back to window-load.

The commit mentions interactive but the code does not. I assume !== loading is used as proxy for == interactive here. We may want to document in the code why that is.

On a related note: I tried to work on a polyfill for a readyState promise (whatwg/html#127) and ran into a bug on Android 2.3 where interactive is reported as loaded (and there is no complete). See https://gist.github.com/Krinkle/fc0b4d4626f2409219ee. I guess that's why we use !== loading?

@timmywil
Copy link
Member Author

@Krinkle Yes, I believe that was why originally. I used it here just because it's shorter.

@dmethvin
Copy link
Member

The logic LGTM. I'd put a support comment there, something to the effect of:

// Support: IE9-10 only
// Older IE signals "interactive" before scripts are ready

@timmywil timmywil merged commit dabd5ba into jquery:master Oct 29, 2015
@timmywil timmywil deleted the ready-gh-2100 branch October 29, 2015 14:26
@dmethvin dmethvin mentioned this pull request Dec 31, 2015
Closed
Totktonada added a commit to Totktonada/zepto that referenced this pull request Oct 30, 2016
@Totktonada
More reliable way to examine DOM readiness on IE
24274f5 
This commit is revisiting of the following commit.

commit 790d6c2
Author: sgtwilko <null@sgtwilko.f9.co.uk>
Date:   Wed Oct 2 10:47:09 2013 +0100

    Ready fires on IE10 before the document is ready.

    Added additional check for document.body into the ready check.
    Changing the readyRE regex does resolve the issue on IE, but breaks
    Chrome.  Adding this check fixes IE and doesn't seem to have any
    problems on Chrome/Firefox.

More info about the original problem with IE can be found in related
jQuery tracker [1].

This commit solves the particular case when IE <= 10 gives non-null
`document.body` in "interactive" `document.readyState`, but DOM isn't
fully parsed at the moment. The case was catched in [2]. This commit
adapt current jQuery's approach to detect IE9 & IE 10, see [3].

Related zepto trackers: madrobby#733, madrobby#916.

[1]: https://bugs.jquery.com/ticket/12282
[2]: https://bugs.jquery.com/ticket/12282#comment:20
[3]: jquery/jquery#2678

Build size
----------

Calculated for a build with the default modules set:

```
zepto.js      58760 -> 58726 ( +6 bytes)
zepto.min.js  26411 -> 26427 (+16 bytes)
zepto.min.gz   9804 ->  9799 ( -5 bytes)
```
@Totktonada Totktonada mentioned this pull request Oct 30, 2016
Merged
@jmbowman jmbowman mentioned this pull request Mar 7, 2018
Merged
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@timmywil @Krinkle @dmethvin @jquerybot