-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Add document.{parsed,contentLoaded,loaded} promises #1936
base: main
Are you sure you want to change the base?
Conversation
source
Outdated
data-x="concept-document-loaded-promise">loaded promise</span> all to distinct new promises.</p> | ||
|
||
<p class="note">This can occur due to <code | ||
data-x="dom-document-open">document.open()</code>.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that the only reason? And would that still happen if we fixed #1698?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it would, unless you change more of the document.open/write/close model.
Mozilla would be interested in implementing this, but can't give you an ETA. |
I will implement this in Blink behind a flag to get feedback from developers. Adding any property to document is fraught because it can shadow bindings on the global in inline event handlers. We'll try to gather feedback if not data on that. |
@dominiccooney good point, why not just mark them |
Great! I'll mark them [Unscopeable], and with two implementers in support, I'll start writing tests next week. |
I'm not certain I like the idea of adding three new attributes on |
Some more discussion in #whatwg at http://logs.glob.uno/?c=freenode%23whatwg&s=31%20Oct%202016&e=31%20Oct%202016#c1010329 with @rniwa. It emerged that he's mostly concerned about So if we can come up with an alternate name that would help. Some suggestions were But apart from that, @rniwa is indifferent to the API, so we shouldn't really take this as a sign of support. |
How about a common prefix for these (nice for autocomplete etc), like:
|
We haven't done that for other promise-returning properties, and it hurts usability. Worth considering though. |
Introduced in whatwg/html#1936.
Tests up at web-platform-tests/wpt#4171. [Unscopable] added. Ready for editor review. |
I'm a bit confused by the document.open handling here. Say I have a DOMContentLoaded event listener which calls document.open(). What will the state of the various promises involved be after https://html.spec.whatwg.org/#the-end step 4? As far as I can tell, the document will be in the "loading" state but its "content loaded promise" will be resolved... |
Ah, yes, I thought that the order between events and promise operations did not matter due to microtask handling, but you're right and have found a case where it does. I think the fix is to resolve the promise first there, before firing the event. Then if firing the event causes it (and the others) to reset to a new promise via Will fix and add such a test case. |
Briefly discussed this with @jakearchibald and he didn't feel it would cause too much confusion, but adding a note for reference: When speaking to developers about I think loading states as promises add value to the platform, but am concerned about the confusion the |
Do you have an alternative term? |
How about |
@rniwa that's taken (the promise for Do web developers know when The In #127 (comment) |
@zcorpan very few developers know this stuff. I made it a question at Chrome Dev Summit https://youtu.be/aWGU4qLtHEw?t=1h40m28s When asked "What comes first?
|
Btw I like the general rule of following event names for equivalent promises, but given developer confusion around this, and @rniwa's concerns (although I don't share them), |
I agree with @jakearchibald that |
I'll switch the tests and spec to document.parsed then. Thanks for the feedback all! |
This closes #127, although that thread also contains discussion about adding other "loaded" promises for various elements, which will move elsewhere. For a discussion of the names, including why jQuery's "ready" naming was not used, see #127 (comment).
Nods. My reference to it is simply for the end point of parsing. Chrome stop in the middle of parsing and start painting in the middle of it indeed. I just can't make any good sense out of that... Thanks for pointing out the event loop. Does 'rendering' mean 'paint' or 'layout' or either? |
Both, basically. And parsing routinely queues tasks to update the node tree, at which point the browser could decide to render. |
@hexalys I wasn't intending to be dismissive. I thought I'd provided enough evidence in a form you could verify, and no further info was needed. I've updated it to show the document ready state at the top of the page: https://progressive-render.glitch.me/. Can you see (like I can in Chrome, Edge, Safari and Firefox) that the
Sure there is. At the top of your document: <style>html { display: none; }</style> Then at the bottom of your document: <style>html { display: block; }</style> (or change the value with JavaScript if you want to reveal the content some other time) |
Here's the server code for the demo, in case it isn't clear: https://glitch.com/edit/#!/progressive-render?path=server.js |
Here's the demo running in Firefox 3 (2008): Seems like Firefox didn't support Here's IE8 (2009): Note that |
@jakearchibald I am speaking of a more practical sense in the context of an average page, with only a few scripts and early DOM manipulations, like my example; which validates my case of a unique Chrome exception "in practice". Ask yourself. How are you suppose to insert critical dynamic content into a page if you can’t predict consistently whether the page is already shown or not? That's a non-starter for me, in the way I think about showing a page, or the way I think about the best performance and experience for my users. The CSS display method is one hack. Thanks for mentioning! I might use that if I must. But I am hoping for a more standard long term solution. I'd be foolish to expect getting very far in convincing Chrome to change any of this in the short term. |
@hexalys you made claims about the spec and implementations that have been shown to be false. I realise that can be embarrassing, especially since you made those claims so aggressively, and boasted about your experience. You've seen the evidence. Instead of trying to find ways to dismiss it, do some more research of your own. Throttle your connection, browse the web, observe the details in dev tools. Consider that you may not be infallible.
This has nothing to do with chunked transfer encoding.
The best way is to use your frontend skills to make a stable progressive render. I've written about this before: https://jakearchibald.com/2014/dont-use-flexbox-for-page-layout/.
Using CSS to control the display of content isn't a hack. That's what it's made for.
After all these examples, in multiple browsers, over 10 years, you're still clinging to this being a recent Chrome thing? I believe you may have an example that has changed in Chrome from your perspective (in a way that's fully spec compliant). But you cannot flip a coin once and conclude "it's always heads!", especially when others have demonstrated otherwise. |
First, I’m usually wrong, so I’m sorry if I have errantly believed that the document's I received a report (in jonathantneal/document-promises#14) that browsers are handling The reports states that, in Safari, executing lots of code on Reviewing this PR again, I could not figure out what should happen when lots of code is executed from |
Per spec, https://html.spec.whatwg.org/multipage/parsing.html#the-end step 7 fires the two events one right after the other. Doing stuff in the readystatechange event listeners should not affect "load" firing, apart from the listeners needing to finish running before it fires. As far as I can tell, that's what Firefox implements. I can't speak for other implementations... In terms of what this PR does, it's resolving the promise right before firing the readystatechange event. What that means in terms of when the promise handlers run depends on whether there are readystatechange listeners. If there are, it will run after the first such listener, afaict, because that will be the next microtask checkpoint. If there are not, it will run after the first load listener, because that will be the next microtask checkpoint. If there are no listeners for either event, they will run after this all unwinds, I think. Maybe we want an explicit microtask checkpoint after we resolve the promise and before we fire readystatechange? |
It should probably resolve after firing. I think that's the usual pattern for these kind of things. |
After firing just the readystatechange, or also after firing "load"? |
Sprinkling microtask checkpoints to different places would be a bit unfortunate. Breaks the generic concept of microtasks. I think we've managed to avoid that quite well. (script element execution is a special case). In my mind if one thinks there is a missing microtask checkpoint, it usually means certain tasks should be split to two pieces (and then the checkpoint would naturally happen at the end of the first task). But not sure how that applies in this case. |
In this case, that would look like separate tasks for firing the "readystatechange" and "load" events or something like that. Then other tasks could run in between those, but that might be ok. Another option is to move the firing of "load" into the readystate change, since now we explicitly have it take different actions based on what state is being changed to, and resolve the promise after both are fired. |
Yeah, I guess conceptually there could be two tasks, but those would be in a way guaranteed to run sequentially and no tasks between them, so in practice implementation could have just one task and an extra checkpoint. That is quite close to how I've been thinking script element handling. |
No tasks from the same task source. Other task sources could run in between. |
If they're in the same task, I'd expect to first fire all events, and then resolve any promises. We should document that pattern somewhere. |
@hexalys Just wanted to point out that on Safari 12.1, I get the same issue as on Chrome (the flash of one button instead of 2 on first few paints) about 90% of the time when loading from cache but 0% of the time when loading from network (forced via dev tools), and this is identical to what I experience on Chrome. I can confirm I don't experience this issue on Firefox, however. It's not Chrome-specific, but might have to do with code common between Webkit and Blink (which started as a Webkit fork). In either case, it's probably more appropriate to file bugs against Safari and Chrome than to discuss it here. |
|
Hey all. I'm curious, what is the status of this? Has it stalled? |
Is there anything I could do to help move this along? |
A preffed-off implementation in a browser engine would help move this along. |
This closes #127, although that thread also contains discussion about
adding other "loaded" promises for various elements, which will move
elsewhere. For a discussion of the names, including why jQuery's "ready"
naming was not used, see
#127 (comment).
I have heard rumblings of implementer interest for this, which is why I put in the effort. However, I'd like to make sure we get confirmed review from 2+ implementers before I proceed to write tests. So:
Also /cc @jakearchibald and @tabatkins as they have both championed this in https://lists.w3.org/Archives/Public/public-whatwg-archive/2014Mar/0091.html and in #127, respectively.
Finally, I'd like to add some developer-facing examples of using these. Let's discuss some ideas for that over in #127 though, not here.
💥 Error: Wattsi server error 💥
PR Preview failed to build. (Last tried on Jan 15, 2021, 7:57 AM UTC).
More
PR Preview relies on a number of web services to run. There seems to be an issue with the following one:
🚨 Wattsi Server - Wattsi Server is the web service used to build the WHATWG HTML spec.
🔗 Related URL
If you don't have enough information above to solve the error by yourself (or to understand to which web service the error is related to, if any), please file an issue.