From d280c831b695ad8eb00fbcbd9af2de8bf625823e Mon Sep 17 00:00:00 2001
From: Domenic Denicola <d@domenic.me>
Date: Mon, 24 May 2021 18:04:48 -0400
Subject: [PATCH] Fix handling of navigations before/during load

Several types of navigations have a special handling, where if they happen "before load", they are converted into "replace" navigations (instead of "default" navigations). However, this special handling had several interoperability problems and the specification was somewhat broken due to a bad definition of "completely loaded". The result here mostly aligns with WebKit and Blink behavior.

Notable changes:

* "completely loaded" gets flipped only after the load event has run. It is mostly used to determine "default" vs. "replace" navigations, and in almost all cases browsers treat navigations before or during the load event as getting the special "replace" treatment. This includes the below cases, as well as <iframe> src="" assignment (which is otherwise untouched). The other thing this improves is that it ensures load events on iframe/embed/object elements are only fired (or at least queued) after the corresponding load event on the contained Document.

* location.assign() gets special handling before/during the load event; previously this special handling was only applied to the location setters. (This allows us to roll "Location-object-setter navigate" into "Location-object navigate".)

* Location APIs now override the special handling and force "default" navigation if transient user activation is present, not just if running specifically in a click handler.

* All form submissions can get converted to "replace" if they target a non-completely loaded document. Previously this special handling was only applied to form submissions initiated by the formElement.submit() API.

Notable non-changes (backed by tests):

* Following hyperlinks never varies its "replace" vs. "default" behavior based on completely loaded status. This matches all browsers

* window.open() navigating an existing browsing context never varies its "replace" vs. "default" behavior based on completely loaded status. This matches Blink and WebKit.

Closes #6579. Closes #3247.
---
 source | 80 +++++++++++++++++++++++-----------------------------------
 1 file changed, 32 insertions(+), 48 deletions(-)

diff --git a/source b/source
index ef3bf3395d8..5c49ee31ecb 100644
--- a/source
+++ b/source
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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>
 
@@ -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>
@@ -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>
@@ -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>
@@ -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>
@@ -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>
@@ -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>
@@ -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,
@@ -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
@@ -112674,6 +112658,8 @@ document.body.appendChild(text);
        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>Completely finish loading</span> the <code>Document</code>.</p></li>
+
        <li><p><span>Queue the navigation timing entry</span> for <var>document</var>.</p></li>
       </ol>
      </li>
@@ -112705,8 +112691,6 @@ document.body.appendChild(text);
    <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>