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.

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

Fix handling of navigations before/during load #6714

Merged
merged 3 commits into from
Jun 23, 2021
Merged

Fix handling of navigations before/during load #6714

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d280c83
Fix handling of navigations before/during load
domenic May 24, 2021
9a405d3
Deal with pageshow more correctly
domenic Jun 17, 2021
eb006c9
Editorial tweaks to those nested steps
domenic Jun 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 49 additions & 79 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23719,9 +23719,17 @@ document.body.appendChild(wbr);</code></pre>
<var>request</var>'s <span data-x="concept-request-referrer">referrer</span> to "<code
data-x="">no-referrer</code>".</p></li>

<li><p>Let <var>historyHandling</var> be "<code data-x="hh-replace">replace</code>" if
<var>windowType</var> is not "<code data-x="">existing or none</code>"; otherwise, "<code
data-x="hh-default">default</code>".</p></li>
<li>
<p>Let <var>historyHandling</var> be "<code data-x="hh-replace">replace</code>" if
<var>windowType</var> is not "<code data-x="">existing or none</code>"; otherwise, "<code
data-x="hh-default">default</code>".</p>

<p class="note">Unlike many other types of navigations, following hyperlinks does not have
special "<code data-x="hh-replace">replace</code>" behavior for when documents are not
<span>completely loaded</span>. This is true for both user-initiated instances of following
hyperlinks, as well as script-triggered ones via, e.g., <code
data-x="">aElement.click()</code>.</p>
</li>

<li><p><span>Queue an element task</span> on the <span>DOM manipulation task source</span> given
<var>subject</var> to <span>navigate</span><!--DONAV hyperlink--> <var>target</var> to
Expand Down Expand Up @@ -56344,9 +56352,8 @@ fur

<li><p>If <var>target browsing context</var> is null, then return.</p></li>

<li><p>If <var>form document</var> has not yet <span>completely loaded</span> and the
<var>submitted from <code data-x="dom-form-submit">submit()</code> method</var> flag is set, then
set <var>historyHandling</var> to "<code data-x="hh-replace">replace</code>".</p></li>
<li><p>If <var>form document</var> has not yet <span>completely loaded</span>, then set
<var>historyHandling</var> to "<code data-x="hh-replace">replace</code>".</p></li>

<li>
<p>If the value of <var>method</var> is <span
Expand Down Expand Up @@ -84671,37 +84678,9 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
<li><p><i>End</i>: Return <var>output</var>.</p></li>
</ol>

<p>A <code>Location</code> object has an associated <dfn><code>Location</code>-object-setter
navigate</dfn> algorithm, which given a <var>url</var>, runs these steps:</p>

<ol>
<li><p>Let <var>historyHandling</var> be "<code data-x="hh-replace">replace</code>".</p></li>

<li>
<p>If any of the following conditions are met, then set <var>historyHandling</var> to "<code
data-x="hh-default">default</code>":</p>

<ul class="brief">
<li>This <code>Location</code> object's <span>relevant <code>Document</code></span> has
<span>completely loaded</span>, or</li>

<li>In the <span data-x="concept-task">task</span> in which the algorithm is running, an
<span>activation behavior</span> is currently being processed whose <code
data-x="event-click">click</code> event's <code data-x="dom-Event-isTrusted">isTrusted</code>
attribute is true, or</li>

<li>In the <span data-x="concept-task">task</span> in which the algorithm is running, the event
listener for a <code data-x="event-click">click</code> event, whose <code
data-x="dom-Event-isTrusted">isTrusted</code> attribute is true, is being handled.</li>
</ul>
</li>

<li><p><span><code>Location</code>-object navigate</span>, given <var>url</var> and
<var>historyHandling</var>.</p></li>
</ol>

<p>To <dfn><code>Location</code>-object navigate</dfn>, given a <var>url</var> and
<var>historyHandling</var>:</p>
<p>To <dfn><code>Location</code>-object navigate</dfn>, given a <span>URL</span> <var>url</var>
and an optional <span>history handling behavior</span> <var>historyHandling</var> (default "<code
data-x="hh-default">default</code>"):</p>

<ol>
<li><p>Let <var>browsingContext</var> be the <span>current global object</span>'s <span
Expand All @@ -84715,6 +84694,11 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
<code>Document</code></span>, then set <var>historyHandling</var> to "<code
data-x="hh-replace">replace</code>".</p></li>

<li><p>If this <code>Location</code> object's <span>relevant <code>Document</code></span> is not
yet <span>completely loaded</span>, and the <span data-x="concept-incumbent-global">incumbent
global object</span> does not have <span>transient activation</span>, then set
<var>historyHandling</var> to "<code data-x="hh-replace">replace</code>".</p></li>

<li><p><span>Navigate</span><!--DONAV location--> <var>browsingContext</var> to <var>url</var>,
with <var><span>exceptionsEnabled</span></var> set to true, <var
data-x="navigation-hh">historyHandling</var> set to <var>historyHandling</var>, and the
Expand Down Expand Up @@ -84744,7 +84728,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
<li><p><span data-x="parse a url">Parse</span> the given value relative to the <span>entry
settings object</span>. If that failed, throw a <code>TypeError</code> exception.</p></li>

<li><p><span><code>Location</code>-object-setter navigate</span> given the <span>resulting URL
<li><p><span><code>Location</code>-object navigate</span> given the <span>resulting URL
record</span>.</p></li>
</ol>

Expand Down Expand Up @@ -84813,7 +84797,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
<li><p>If <var>copyURL</var>'s <span data-x="concept-url-scheme">scheme</span> is not an
<span>HTTP(S) scheme</span>, then terminate these steps.</p></li>

<li><p><span><code>Location</code>-object-setter navigate</span> to <var>copyURL</var>.</p></li>
<li><p><span><code>Location</code>-object navigate</span> to <var>copyURL</var>.</p></li>
</ol>

<p>The <dfn attribute for="Location"><code data-x="dom-location-host">host</code></dfn>
Expand Down Expand Up @@ -84863,7 +84847,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
state</span> as <span data-x="basic url parser state override"><i>state
override</i></span>.</p></li>

<li><p><span><code>Location</code>-object-setter navigate</span> to <var>copyURL</var>.</p></li>
<li><p><span><code>Location</code>-object navigate</span> to <var>copyURL</var>.</p></li>
</ol>

<p>The <dfn attribute for="Location"><code data-x="dom-location-hostname">hostname</code></dfn>
Expand Down Expand Up @@ -84906,7 +84890,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
state</span> as <span data-x="basic url parser state override"><i>state
override</i></span>.</p></li>

<li><p><span><code>Location</code>-object-setter navigate</span> to <var>copyURL</var>.</p></li>
<li><p><span><code>Location</code>-object navigate</span> to <var>copyURL</var>.</p></li>
</ol>

<p>The <dfn attribute for="Location"><code data-x="dom-location-port">port</code></dfn>
Expand Down Expand Up @@ -84952,7 +84936,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
state</span> as <span data-x="basic url parser state override"><i>state
override</i></span>.</p></li>

<li><p><span><code>Location</code>-object-setter navigate</span> to <var>copyURL</var>.</p></li>
<li><p><span><code>Location</code>-object navigate</span> to <var>copyURL</var>.</p></li>
</ol>

<p>The <dfn attribute for="Location"><code data-x="dom-location-pathname">pathname</code></dfn>
Expand Down Expand Up @@ -85004,7 +84988,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
state</span> as <span data-x="basic url parser state override"><i>state
override</i></span>.</p></li>

<li><p><span><code>Location</code>-object-setter navigate</span> to <var>copyURL</var>.</p></li>
<li><p><span><code>Location</code>-object navigate</span> to <var>copyURL</var>.</p></li>
</ol>

<p>The <dfn attribute for="Location"><code data-x="dom-location-search">search</code></dfn>
Expand Down Expand Up @@ -85062,7 +85046,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
</ol>
</li>

<li><p><span><code>Location</code>-object-setter navigate</span> to <var>copyURL</var>.</p></li>
<li><p><span><code>Location</code>-object navigate</span> to <var>copyURL</var>.</p></li>
</ol>

<p>The <dfn attribute for="Location"><code data-x="dom-location-hash">hash</code></dfn>
Expand Down Expand Up @@ -85109,7 +85093,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
state</span> as <span data-x="basic url parser state override"><i>state
override</i></span>.</p></li>

<li><p><span><code>Location</code>-object-setter navigate</span> to <var>copyURL</var>.</p></li>
<li><p><span><code>Location</code>-object navigate</span> to <var>copyURL</var>.</p></li>
</ol>

<p class="note">Unlike the equivalent API for the <code>a</code> and <code>area</code> elements,
Expand All @@ -85136,7 +85120,7 @@ interface <dfn>Location</dfn> { // but see also <a href="#the-location-interface
<code>DOMException</code>.</p></li>

<li><p><span><code>Location</code>-object navigate</span> given the <span>resulting URL
record</span> and "<code data-x="hh-default">default</code>".</p></li>
record</span>.</p></li>
</ol>

<p>When the <dfn method for="Location"><code
Expand Down Expand Up @@ -112651,62 +112635,48 @@ document.body.appendChild(text);

<li>
<p><span>Queue a global task</span> on the <span>DOM manipulation task source</span> given the
<code>Document</code>'s <span>relevant global object</span> to run the following substeps:</p>
<code>Document</code>'s <span>relevant global object</span> to run the following steps:</p>

<ol>
<li><p><span>Update the current document readiness</span> to "<code
data-x="">complete</code>".</p></li>

<li>
<p><i>Load event</i>: If the <code>Document</code> object's <span
data-x="concept-document-bc">browsing context</span> is non-null, then:</p>
<li><p>If the <code>Document</code> object's <span data-x="concept-document-bc">browsing
context</span> is null, then abort these steps.</p></li>

<ol>
<li><p>Set the <code>Document</code>'s <span>load timing info</span>'s <span>load event start
time</span> to the <span>current high resolution time</span> given the
<code>Document</code>'s <span>relevant global object</span>.</p></li>

<li><p><span data-x="concept-event-fire">Fire an event</span> named <code
data-x="event-load">load</code> at the <code>Document</code> object's <span>relevant global
object</span>, with <var>legacy target override flag</var> set.</p></li>
<li><p>Let <var>window</var> be the <code>Document</code>'s <span>relevant global
object</span>.</p></li>

<li><p>Set the <code>Document</code>'s <span>load timing info</span>'s <span>load event end
time</span> to the <span>current high resolution time</span> given the
<code>Document</code>'s <span>relevant global object</span>.</p></li>
<li><p>Set the <code>Document</code>'s <span>load timing info</span>'s <span>load event start
time</span> to the <span>current high resolution time</span> given <var>window</var>.</p></li>

<li><p><span>Queue the navigation timing entry</span> for <var>document</var>.</p></li>
</ol>
</li>
</ol>
</li>
<li><p><span data-x="concept-event-fire">Fire an event</span> named <code
data-x="event-load">load</code> at <var>window</var>, with <var>legacy target override
flag</var> set.</p></li>

<li>
<p>If the <code>Document</code> object's <span data-x="concept-document-bc">browsing
context</span> is non-null, then <span>queue a global task</span> on the <span>DOM manipulation
task source</span> given the <code>Document</code>'s <span>relevant global object</span> to run
these steps:</p>
<li><p>Set the <code>Document</code>'s <span>load timing info</span>'s <span>load event end
time</span> to the <span>current high resolution time</span> given <var>window</var>.</p></li>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If load and pageshow happen in the same task, should this measure both or only the load event cost? cc @yoavweiss

It seems this could be gamed by just moving the work to pageshow instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't worry too much about gaming here, as work can already be rescheduled from onload to run in a consecutive task. Are there tests covering that onload and pageshow are now on the same task and onload runs before pageshow?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the same-taskness wouldn't be testable, but I realized we can test it with microtasks. I'll tack that onto my tests PR.

I'm pretty sure the ordering is already tested somewhere but I'll test that too.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, no, it's not testable, I forgot. When the event listeners fire, the stack is empty, so microtasks will run whether it's a single task or multiple tasks scheduled one after the other.

The ordering is already tested in https://wpt.fyi/results/html/syntax/parsing/the-end.html?label=master&label=experimental&aligned&q=the-end .


<ol>
<li><p>If the <code>Document</code>'s <span>page showing</span> flag is true, then return
(i.e. don't fire the event below).</p></li> <!-- i don't see how this could be, but just
to be sure... -->
<li><p>Assert: <code>Document</code>'s <span>page showing</span> is false.</p></li>

<li><p>Set the <code>Document</code>'s <span>page showing</span> flag to true.</p></li>

<li><p><span data-x="concept-event-fire">Fire an event</span> named <code
data-x="event-pageshow">pageshow</code> at the <code>Document</code> object's <span>relevant
global object</span>, using <code>PageTransitionEvent</code>, with the <code
data-x="event-pageshow">pageshow</code> at <var>window</var>, using
<code>PageTransitionEvent</code>, with the <code
data-x="dom-PageTransitionEvent-persisted">persisted</code> attribute initialized to false, and
<var>legacy target override flag</var> set.</p></li>

<li><p><span>Completely finish loading</span> the <code>Document</code>.</p></li>

<li><p><span>Queue the navigation timing entry</span> for the <code>Document</code>.</p></li>
</ol>
</li>

<li><p>If the <code>Document</code>'s <span>print when loaded</span> flag is set, then run the
<span>printing steps</span>.</p></li>

<li><p>The <code>Document</code> is now <dfn>ready for post-load tasks</dfn>.</p></li>

<li><p><span>Completely finish loading</span> the <code>Document</code>.</p></li>
</ol>

<p>When the user agent is to <dfn>abort a parser</dfn>, it must run the following steps:</p>
Expand Down