-
Notifications
You must be signed in to change notification settings - Fork 41
add initial version for prerender-until-script #412
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
Conversation
| Preview: |
|
|
||
| By default, a [=navigable=]'s [=navigable/loading mode=] is "`default`". A navigable whose [=navigable/loading mode=] is "`prerender`" is known as a <dfn>prerendering navigable</dfn>. A [=prerendering navigable=] that is also a [=top-level traversable=] is known as a <dfn>prerendering traversable</dfn>. | ||
|
|
||
| Every [=navigable=] has a <dfn for="navigable">scripting mode</dfn>, which is one of the following: |
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.
My naive question would be: Is this flag associated with navigable or something else (e.g. Document)?
So, clarifying the context/reasoning for the placement of this flag (no matter of the specific placement) is helpful.
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.
At first I felt Document was more natural, as that is where most script-related state lives.
However, I think navigable is probably correct, because we want it to survive if one navigable ends up holding multiple Documents in sequence.
A navigable could end up holding multiple Documents in sequence because:
- The initial
about:blankDocument-> "real"Documenttransition. (We could work around this though.) - If our script-pausing machinery is not perfect (e.g. due to inline event handlers?), then a JavaScript redirect could cause the navigable to go elsewhere. (E.g.,
<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" onload="location.href = 'page2.html'">.) <meta name="refresh">- Maybe other things I haven't thought of.
Also, I think it will be much easier in the spec to set the flag on navigable, than to pipe it down into the Document :)
+1 to documenting this though.
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.
Thank you for the discussion! I added a short note trying to cover the points. PTAL :)
prerendering.bs
Outdated
|
|
||
| <h3 id="pausing-script-execution">Pausing script execution</h3> | ||
|
|
||
| If a [=navigable=]'s [=navigable/scripting mode=] is "`paused-until-activation`", the user agent must not execute script in the navigable's [=navigable/active document=]. This includes, but is not limited to executing `<script>` elements. |
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.
This PR is more like the speculation-rules-side plumbing and doesn't define the behavior of "pausing script execution".
So, at least we should clearly label the to-be-defined concepts and centralize the undefined things (probably to the section).
Namely, "Resume script execution" at Line 633.
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.
+1, this will need a lot more work to become a full spec. (Even ignoring inline event handlers.) It would be reasonable to do that in a follow-up PR though, as long as you clearly mark the open issue (e.g., using <p class="XXX">) here and on line 633.
You two probably understand the best patches better than me, since you are working on the implementation. But if I had to guess, for <script>, we might have some queue, and patch https://html.spec.whatwg.org/#execute-the-script-element to push things into a queue during "paused-until-activation" mode, and then flush the queue at activation time. (Or maybe just patch https://html.spec.whatwg.org/#run-a-classic-script and https://html.spec.whatwg.org/#run-a-module-script is better?)
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.
Good catch! Yes I need to add more detailed spec for scripting.
Added some open issues for now, will resolve it in follow-up PRs.
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.
FYI: The current implementation is more like:
- For parser-related scripts, we add another reason to block parser-related scripts in addition to https://html.spec.whatwg.org/C/#has-no-style-sheet-that-is-blocking-scripts.
- For async scripts, we delay https://html.spec.whatwg.org/multipage/scripting.html#mark-as-ready until activation.
This is because, within https://html.spec.whatwg.org/#execute-the-script-element, we can't block the parser gracefully, and thus we need something before https://html.spec.whatwg.org/#execute-the-script-element is called.
However, there have been alternative ways (and alternative interpretation of the implementation) around here, and we have found this is quite complicated (especailly for the long-term view and specification).
For example, the lack of the single unified way for "blocking" scripts in this context made us motivated for figuring out an unified way, but we haven't reached consensus.
So I expect the detailed definition wouldn't be ready at least before TPAC.
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.
Thank you for the information!! So, I think we're going to discuss the blocking/unblocking implementation during TPAC. I will draft changes to scripting first and finalize it after TPAC.
domenic
left a comment
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.
Nice work, this seems like a great start! I am also happy to see @hiroshige-g helping out!!
prerendering.bs
Outdated
| A <dfn>prerender candidate</dfn> is a [=speculative load candidate=] with the following additional [=struct/items=]: | ||
|
|
||
| * <dfn for="prerender candidate">target navigable name hint</dfn>, a [=valid navigable target name or keyword=] or null | ||
| * <dfn for="prerender candidate">scripts paused</dfn>, a [=boolean=], initially false |
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.
Nit: I think a better name for this would be something like "should pause scripts"? Because it causes later machinery to pause the scripts; it's not reflecting whether or not the scripts are paused.
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.
Ah exactly! Revised..
I also change the boolean's name to shouldPauseScripts, but wondering pauseScripts sounds better or not..
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.
I can see arguments for both. "should pause scripts" sounds a bit clearer that it's about a future behavior, but it's a bit confusing since "should" has special meaning in specs that doesn't quite match what we're using here...
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.
nit: I basically prefer "block scripts" / "unblock scripts" over "pause/resume scripts":
- To align with https://html.spec.whatwg.org/multipage/#has-no-style-sheet-that-is-blocking-scripts
- To avoid confusion: we don't pause the parser directly, we don't pause scripts in the middle of a script execution.
But this naming will be also affected by the detailed spec defintion we'll make later, so not blocking.
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.
Thank you all! So then maybe let me follow the naming style of https://html.spec.whatwg.org/multipage/#has-no-style-sheet-that-is-blocking-scripts, and update it after the scripting spec is ready.
How do the current names look to you?
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.
LGTM!
|
|
||
| By default, a [=navigable=]'s [=navigable/loading mode=] is "`default`". A navigable whose [=navigable/loading mode=] is "`prerender`" is known as a <dfn>prerendering navigable</dfn>. A [=prerendering navigable=] that is also a [=top-level traversable=] is known as a <dfn>prerendering traversable</dfn>. | ||
|
|
||
| Every [=navigable=] has a <dfn for="navigable">scripting mode</dfn>, which is one of the following: |
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.
At first I felt Document was more natural, as that is where most script-related state lives.
However, I think navigable is probably correct, because we want it to survive if one navigable ends up holding multiple Documents in sequence.
A navigable could end up holding multiple Documents in sequence because:
- The initial
about:blankDocument-> "real"Documenttransition. (We could work around this though.) - If our script-pausing machinery is not perfect (e.g. due to inline event handlers?), then a JavaScript redirect could cause the navigable to go elsewhere. (E.g.,
<img src="data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABAAEAAAIBRAA7" onload="location.href = 'page2.html'">.) <meta name="refresh">- Maybe other things I haven't thought of.
Also, I think it will be much easier in the spec to set the flag on navigable, than to pipe it down into the Document :)
+1 to documenting this though.
prerendering.bs
Outdated
|
|
||
| <div> | ||
| To <dfn export>start a referrer-initiated navigational prerender</dfn> given a {{Document}} |referrerDoc| and a [=prefetch record=] |prefetchRecord|: | ||
| To <dfn export>start a referrer-initiated navigational prerender</dfn> given a {{Document}} |referrerDoc|, a [=prefetch record=] |prefetchRecord|, and a boolean |scriptsPaused|: |
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.
Related to the above naming discussion, I might call this one |pauseScripts|.
prerendering.bs
Outdated
|
|
||
| <h3 id="pausing-script-execution">Pausing script execution</h3> | ||
|
|
||
| If a [=navigable=]'s [=navigable/scripting mode=] is "`paused-until-activation`", the user agent must not execute script in the navigable's [=navigable/active document=]. This includes, but is not limited to executing `<script>` elements. |
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.
+1, this will need a lot more work to become a full spec. (Even ignoring inline event handlers.) It would be reasonable to do that in a follow-up PR though, as long as you clearly mark the open issue (e.g., using <p class="XXX">) here and on line 633.
You two probably understand the best patches better than me, since you are working on the implementation. But if I had to guess, for <script>, we might have some queue, and patch https://html.spec.whatwg.org/#execute-the-script-element to push things into a queue during "paused-until-activation" mode, and then flush the queue at activation time. (Or maybe just patch https://html.spec.whatwg.org/#run-a-classic-script and https://html.spec.whatwg.org/#run-a-module-script is better?)
|
Thank you for your comments!! ❤️ Trying to resolve as many as I can, and I will make a follow-up PR soon. |
No description provided.