Bug Tracker

Ticket #10067 (closed bug: notabug)

Opened 3 years ago

Last modified 2 years ago

Firing $.ready on document.readyState === 'interactive' too

Reported by: skaurus Owned by: mikesherov
Priority: low Milestone:
Component: event Version: 1.6.2
Keywords: 1.8-discuss Cc:
Blocking: Blocked by:

Description (last modified by rwaldron) (diff)

Sorry if dup.

So; imagine situation: FF5, page with ever loading iframe (any other resourse will do I think), jQuery executed after DOMContentLoaded fired (asynchronous loading). On $.bindReady stage document.readyState consistently (a lot of Ctrl+F5) equals to 'interactive', so $.ready wont fire ever, right?

It makes panda sooo sad)

Change History

comment:1 Changed 3 years ago by rwaldron

  • Priority changed from undecided to low
  • Resolution set to wontfix
  • Status changed from new to closed
  • Component changed from unfiled to event
  • Milestone changed from None to 1.6.3

jQuery provides a solid, consistent document ready handling system that has been battle tested for the last 5 years on upwards of 24 million websites.

comment:2 Changed 3 years ago by skaurus <skaurus@…>

WTF is that?

I provided a number of steps that leads to bug clearly, that's easy to see just looking to $.bindReady source.

The only thing that needs proving is that FF5 can have document.readyState === 'interactive' after DOMContentLoaded fired.

But this is 1) W3C compliant -  http://www.whatwg.org/specs/web-apps/current-work/multipage/dom.html says '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.' Looks like FF5 adopted this; 2) easily proved.

But instead of investigation or providing technical reasons why current behaviour is right or why it can't be reliably fixed you just give me some marketing. No thanks.

There, have a test case:  http://jsfiddle.net/skaurus/xGtUf/1/

comment:3 Changed 3 years ago by skaurus <skaurus@…>

Also Yansky from this stackoverflow question  http://stackoverflow.com/questions/3665561/document-readystate-of-interactive-vs-ondomcontentloaded states that he was given information by one of mozilla developers that 'DOMContentLoaded event equates to the document.interactive readystate'

comment:4 Changed 3 years ago by rwaldron

So, what benefit would jQuery get by adding a lot of extra code to handle something that is "equivalent" to what it already does?

comment:5 Changed 3 years ago by skaurus <skaurus@…>

Don't you see - in testcase jQuery.ready never ever fire? Although DOMContentLoaded did?

Lot of code possibky would be || document.readyState === "interactive".

comment:6 Changed 3 years ago by skaurus <skaurus@…>

The only possible way to jQuery to discover that DOMContentLoaded already fired, is check for document.readyState === "complete", line 454.

Possibly before FF5 after DOMContentLoaded it indeed always was equal to complete.

But now after DOMContentLoaded and before onload it's interactive, so check for "did DOMContentLoaded already fired?" on line 454 fails, then jQuery setups listeners for DOMContentLoaded, which will never fire, because it's already fired!

comment:7 Changed 3 years ago by rwaldron

  • Status changed from closed to reopened
  • Resolution wontfix deleted

My apologies, my initial resolution was short sighted. Thanks for your patience

comment:8 Changed 3 years ago by rwaldron

comment:9 Changed 3 years ago by skaurus <skaurus@…>

Thank you :)

Is this really low priority though?

comment:10 Changed 3 years ago by dmethvin

  • Status changed from reopened to open
  • Milestone changed from 1.6.3 to 1.7

I am not clear on the case where this would happen. The test case shows something uncommon, loading jQuery after the doc is ready.

comment:11 Changed 3 years ago by skaurus@…

It would happen when jQuery loaded asynchronously, it's a rarity, but it's good practice. Async js loading generaly advised for higher perfomance.

comment:12 Changed 3 years ago by rwaldron

We've made concessions for RequireJS's needs this has yet to be an issue

comment:13 Changed 3 years ago by eduardocereto

Would this be enough to solve the issue?

from this:

if ( document.readyState === "complete" ) {

to this:

if ( /complete|interactive/.test(document.readyState) ) {

comment:14 Changed 3 years ago by skaurus@…

I believe it would.

Now I use

if (document.readyState === "interactive") jQuery.ready();

after loading my scripts, and it works perfectly.

comment:15 Changed 3 years ago by janne.aukia@…

This problem shows in practice on Webkit (Safari/Chrome) when DOMContentLoaded has fired before the jQuery.ready() is set up. This makes Webkit wait until window.load before $(document).ready fires.

In practice, in our application, this made $(document).ready fire 5 seconds slower on Safari/Chrome compared to Firefox! So for some sites and apps, this bug can cause serious slowdown while being difficult to debug.

comment:16 Changed 3 years ago by mikesherov

Correct me if I'm wrong, but simplest patch seems to be:

document.readystate === "complete"

To

document.readystate !== "loading"

Heck, we even save a byte ;). Writing the unit test shouldnt be terrible either.

comment:17 Changed 3 years ago by rwaldron

  • Keywords 1.8-discuss added
  • Owner set to mikesherov
  • Status changed from open to assigned

comment:18 Changed 3 years ago by mikesherov

ok, so writing the unit test IS terrible. But I making solid progress... early results seem to indicate that the change is fine.

comment:19 Changed 3 years ago by dmethvin

  • Milestone changed from 1.7 to 1.8

comment:20 Changed 3 years ago by mikesherov

  • Description modified (diff)

-1, I've been working on this ticket, and tested this in the general case, and it seems to work. It's also one of the simplest changes to make, while very difficult to test. However, I don't know if this would break an expectation that when $.ready fired it meant you could interact with the content of IFRAMEs on the page. Also, it just seems like this is a bit too much of an edge case to change the current behavior.

comment:21 Changed 3 years ago by jaubourg

+1, We just need to document that the dom of iframes is not guaranteed to be ready in that instance.

comment:22 Changed 3 years ago by jzaefferer

+0, Try to figure if its a bug and handle as such. Too complicated for a vote really.

comment:23 Changed 3 years ago by dmethvin

  • Description modified (diff)

+0, Changes in .ready() make me worried when we can't test them easily. So what is the specific benefit to this, and how commonlly is it needed?

comment:24 Changed 3 years ago by timmywil

  • Description modified (diff)

+1, I'm ok with adding interactive if it does not create bugs

comment:25 Changed 3 years ago by rwaldron

  • Description modified (diff)

+1, Despite my previous opposition, I'd be interested in giving this a go

comment:26 Changed 3 years ago by dmethvin

  • Status changed from assigned to closed
  • Resolution set to fixed

comment:27 Changed 3 years ago by Mike Sherov

Fix #10067. Create jQuery.quickReady; closes gh-736.

Allows us to get to the ready state sooner by not waiting for iframes to load. If that causes backcompat pain, use jQuery.quickReady = false as prescribed by your developer.

Changeset: 54fab3174c7844959b374e98b453c048b60de0d0

comment:28 Changed 2 years ago by dmethvin

We tried, it didn't work. We learned a lot. Like, never listen to skaurus.

 https://github.com/jquery/jquery/pull/907

comment:29 Changed 2 years ago by mikesherov

Well, it doesn't work in IE9/IE10, and Android 2.1. Perhaps at a future date, we may be intrepid enough to try this again? I'm a glutton for punishment.

comment:30 Changed 2 years ago by gleidsondg@…

ON IE 9.0.8 with jquery 1.8 ready state never arrives

comment:31 Changed 2 years ago by smith

Does this need to be reopened? #12282 had a regression that reverted the changes made above.

comment:32 Changed 2 years ago by mikesherov

It's not going to get fixed at least until IE9.0 and IE10 die. So we'll reopen this in 2020 when a fix can be reasonably applied.

comment:33 Changed 2 years ago by dmethvin

  • Status changed from closed to reopened
  • Resolution fixed deleted
  • Milestone 1.8 deleted

comment:34 Changed 2 years ago by dmethvin

  • Status changed from reopened to closed
  • Resolution set to notabug
Note: See TracTickets for help on using tickets.