Fix historical and recent breakage in session history traversal

This fixes several related issues surrounding the "update the session history with the new page" and "traverse the history" algorithms. * A mistaken attempted refactoring in #6039 got the wrong Document for the newDocument variable in "traverse the history". To fix this, we explicitly pass in the new document to the algorithm from all its call sites. (Discussed in #6197.) * As discussed more extensively in #6197, the same-URL and entry update/reload conditions in "update the session history with the new page" were not correctly triggering the new-document clause in "traverse the history", despite replacing the document. This was partially due to #6039, although the phrasing before #6039 was extremely ambiguous, so #6039 was mostly a transition from "unclear" to "clearly wrong". * This fixes the "easy bug" discussed in #6202, where the same-URL case was using the wrong URL to determine sameness. That bug was also introduced in #6039. The harder bug, as well as the action-at-a-distance nature of the same-URL check, remain tracked in #6202. * For years, the spec has required deleting the future session history entries after performing a navigation with history handling of "replace", e.g. via location.replace(). This is not correct; a "replace" navigation does not delete future session history entries. Instead it just replaces the current entry, leaving future entries alone. * The latter point makes the handling of same-URL navigations almost identical to "replace" navigations (including non-same-URL "replace" navigations). This change makes this clear, by using the same text for both, but adding a pointer to #6213 which highlights the fact that some browsers treat them slightly differently (and some treat them the same). * Finally, this adds or modifies a few assertions to check that we're in the "default" history handling behavior, so it's clearer to the reader what the non-exceptional path through the spec is. Closes #6197.
whatwg · Dec 11, 2020 · b80e83b · b80e83b
1 parent 07a5b3e
commit b80e83b
Showing 1 changed file with 72 additions and 35 deletions.
diff --git a/source b/source
@@ -85165,16 +85165,13 @@ new PaymentRequest(&hellip;); // Allowed to use
 
   <p>Some of the sections below, to which the above algorithm defers in certain cases, require the
   user agent to <dfn>update the session history with the new page</dfn>, given some <span>navigation
-  params</span> <var>navigationParams</var>. When a user agent is required to do this, it must
-  <span>queue a global task</span> on the <span>networking task source</span>, given the
-  <span>relevant global object</span> of the <code>Document</code> object of the <span>current
-  entry</span> (not the new one), to run the following steps:</p>
+  params</span> <var>navigationParams</var> and a <code>Document</code> <var>newDocument</var>. When
+  a user agent is required to do this, it must <span>queue a global task</span> on the
+  <span>networking task source</span>, given the <span>relevant global object</span> of the
+  <code>Document</code> object of the <span>current entry</span> (not the new one), to run the
+  following steps:</p>
 
   <ol>
-   <li><p>Let <var>newDocument</var> be <var>navigationParams</var>'s <span
-   data-x="navigation-params-browsing-context">browsing context</span>'s <span>active
-   document</span>.</p></li>
-
    <li>
     <p><span data-x="unload a document">Unload</span> the <span
     data-x="she-document">document</span> of the <span>current entry</span>.</p>
@@ -85190,38 +85187,76 @@ new PaymentRequest(&hellip;); // Allowed to use
    <li>
     <dl>
      <dt>If <var>navigationParams</var>'s <span data-x="navigation-params-hh">history
-     handling</span> is "<code data-x="hh-reload">reload</code>" or "<code
-     data-x="hh-entry-update">entry update</code>"</dt>
+     handling</span> is "<code data-x="hh-entry-update">entry update</code>" or "<code
+     data-x="hh-reload">reload</code>"</dt>
 
      <dd>
       <ol>
        <li><p>Replace the <span data-x="she-document">document</span> of the <span>current
-       entry</span>, and any other entries that reference the same <span
+       entry</span>, and of any other entries that reference the same <span
        data-x="she-document">document</span> as that entry, with <var>newDocument</var>.</p></li>
 
-       <li><p><span>Traverse the history</span> to the <span>current entry</span>.</p></li>
+       <li><p><span>Traverse the history</span> to the <span>current entry</span> with <var
+       data-x="traverse-history-hh">historyHandling</var> set to <var>navigationParams</var>'s <span
+       data-x="navigation-params-hh">history handling</span>.</p></li>
       </ol>
      </dd>
 
-     <dt>If the navigation was initiated with a <span>URL</span> that <span
-     data-x="concept-url-equals">equals</span> <var>newDocument</var>'s <span
-     data-x="concept-document-url">URL</span></dt>
+     <dt>Otherwise, if the navigation was initiated with a <span>URL</span> that <span
+     data-x="concept-url-equals">equals</span> <var>navigationParams</var>'s <span
+     data-x="navigation-params-browsing-context">browsing context</span>'s <span>active
+     document</span>'s <span data-x="concept-document-url">URL</span></dt>
 
      <dd>
       <ol>
-       <li><p>Replace the <span>current entry</span> with a new <span>session history entry</span>
-       whose <span data-x="she-url">URL</span> is <var>newDocument</var>'s <span
-       data-x="concept-document-url">URL</span> and <span data-x="she-document">document</span> is
-       <var>newDocument</var>.</p></li>
+       <li>
+        <p>Let <var>newEntry</var> be a new <span>session history entry</span> whose <span
+        data-x="she-url">URL</span> is <var>newDocument</var>'s <span
+        data-x="concept-document-url">URL</span> and <span data-x="she-document">document</span> is
+        <var>newDocument</var>.</p>
+
+        <p class="XXX">Some browsers copy over the serialized state of <span>current entry</span>,
+        but this is inconsistent. See <a href="https://github.com/whatwg/html/issues/6213">issue
+        #6213</a> for more discussion on this.</p>
+       </li>
+
+       <li><p>Insert <var>newEntry</var> after the <span>current entry</span> in
+       <var>navigationParams</var>'s <span data-x="navigation-params-browsing-context">browsing
+       context</span>'s <span>session history</span>.</p></li>
 
-       <li><p><span>Traverse the history</span> to the <span>current entry</span>.</p></li>
+       <li><p><span>Traverse the history</span> to <var>newEntry</var> with <var
+       data-x="traverse-history-hh">historyHandling</var> set to "<code
+       data-x="hh-replace">replace</code>".</p></li>
+      </ol>
+     </dd>
+
+     <dt>Otherwise, if <var>navigationParams</var>'s <span data-x="navigation-params-hh">history
+     handling</span> is "<code data-x="hh-replace">replace</code>"</dt>
+
+     <dd>
+      <ol>
+       <li><p>Let <var>newEntry</var> be a new <span>session history entry</span> whose <span
+        data-x="she-url">URL</span> is <var>newDocument</var>'s <span
+        data-x="concept-document-url">URL</span> and <span data-x="she-document">document</span> is
+        <var>newDocument</var>.</p></li>
+
+       <li><p>Insert <var>newEntry</var> after the <span>current entry</span> in
+       <var>navigationParams</var>'s <span data-x="navigation-params-browsing-context">browsing
+       context</span>'s <span>session history</span>.</p></li>
+
+       <li><p><span>Traverse the history</span> to <var>newEntry</var> with
+       <var data-x="traverse-history-hh">historyHandling</var> set to "<code
+       data-x="hh-replace">replace</code>".</p></li>
       </ol>
      </dd>
 
      <dt>Otherwise</dt>
 
      <dd>
       <ol>
+       <li><p>Assert: <var>navigationParams</var>'s <span data-x="navigation-params-hh">history
+       handling</span> is "<code data-x="hh-default">default</code>".</p></li>
+
        <li>
         <p>Remove all the entries in the <span>session history</span> after the <span>current
         entry</span>. If the <span>current entry</span> is the last entry in the session history,
@@ -85236,9 +85271,7 @@ new PaymentRequest(&hellip;); // Allowed to use
        data-x="concept-document-url">URL</span> and <span data-x="she-document">document</span> is
        <var>newDocument</var>.</p></li>
 
-       <li><p><span>Traverse the history</span> to the new entry, with <var
-       data-x="traverse-history-hh">historyHandling</var> set to <var>navigationParams</var>'s <span
-       data-x="navigation-params-hh">history handling</span>.</p></li>
+       <li><p><span>Traverse the history</span> to the new entry.</p></li>
       </ol>
      </dd>
     </dl>
@@ -85311,7 +85344,8 @@ new PaymentRequest(&hellip;); // Allowed to use
 
   <p>After creating the <code>Document</code> object, but before any script execution, certainly
   before the parser <span data-x="stop parsing">stops</span>, the user agent must <span>update the
-  session history with the new page</span> given <var>navigationParams</var>.</p>
+  session history with the new page</span> given <var>navigationParams</var> and the newly-created
+  <code>Document</code>.</p>
 
 
 
@@ -85336,9 +85370,10 @@ new PaymentRequest(&hellip;); // Allowed to use
   character encoding.</p>
 
   <p>Then, with the newly created <code>Document</code>, the user agent must <span>update the
-  session history with the new page</span> given <var>navigationParams</var>. User agents may do
-  this before the complete document has been parsed (thus achieving <i>incremental rendering</i>),
-  and must do this before any scripts are to be executed.</p>
+  session history with the new page</span> given <var>navigationParams</var> and the newly-created
+  <code>Document</code>. User agents may do this before the complete document has been parsed (thus
+  achieving <i>incremental rendering</i>), and must do this before any scripts are to be
+  executed.</p>
 
   <p>Error messages from the parse process (e.g., XML namespace well-formedness errors) may be
   reported inline by mutating the <code>Document</code>.</p>
@@ -85385,7 +85420,7 @@ new PaymentRequest(&hellip;); // Allowed to use
 
   <p>After creating the <code>Document</code> object, but potentially before the page has finished
   parsing, the user agent must <span>update the session history with the new page</span> given
-  <var>navigationParams</var>.</p>
+  <var>navigationParams</var> and the newly-created <code>Document</code>.</p>
 
   <p>User agents may add content to the <code>head</code> element of the <code>Document</code>,
   e.g., linking to a style sheet, providing script, or giving the document a <code>title</code>.</p>
@@ -85473,7 +85508,7 @@ new PaymentRequest(&hellip;); // Allowed to use
 
   <p>After creating the <code>Document</code> object, but potentially before the page has finished
   fully loading, the user agent must <span>update the session history with the new page</span> given
-  <var>navigationParams</var>.</p>
+  <var>navigationParams</var> and the newly-created <code>Document</code>.</p>
 
   <p>User agents may add content to the <code>head</code> element of the <code>Document</code>, or
   attributes to the element <var>host element</var>, e.g., to link to a style sheet, to provide a
@@ -85518,7 +85553,7 @@ new PaymentRequest(&hellip;); // Allowed to use
 
   <p>After creating the <code>Document</code> object, but potentially before the page has finished
   fully loading, the user agent must <span>update the session history with the new page</span> given
-  <var>navigationParams</var>.</p>
+  <var>navigationParams</var> and the newly-created <code>Document</code>.</p>
 
   <p>User agents may add content to the <code>head</code> element of the <code>Document</code>, or
   attributes to the <code>embed</code> element, e.g. to link to a style sheet or to give the
@@ -85574,7 +85609,7 @@ new PaymentRequest(&hellip;); // Allowed to use
 
   <p>After creating the <code>Document</code> object, but potentially before the page has been
   completely set up, the user agent must <span>update the session history with the new page</span>
-  given <var>navigationParams</var>.</p>
+  given <var>navigationParams</var> and the newly-created <code>Document</code>.</p>
 
 
 
@@ -85754,16 +85789,16 @@ new PaymentRequest(&hellip;); // Allowed to use
     <p>If <var>entry</var>'s <span data-x="she-document">document</span> is null, then:</p>
 
     <ol>
+     <li><p>Assert: <var>historyHandling</var> is "<code
+     data-x="hh-default">default</code>".</p></li>
+
      <li><p>Let <var>request</var> be a new <span data-x="concept-request">request</span> whose
      <span data-x="concept-request-url">url</span> is <var>entry</var>'s <span
      data-x="she-url">URL</span>.</p></li>
 
      <li><p>If <var>explicitHistoryNavigation</var> is true, then set <var>request</var>'s <span
      data-x="concept-request-history-navigation-flag">history-navigation flag</span>.</p></li>
 
-     <li><p>Assert: <var>historyHandling</var> is not "<code
-     data-x="hh-replace">replace</code>".</p></li>
-
      <li>
       <p><span>Navigate</span><!--DONAV history traversal after eviction--> the <span>browsing
       context</span> to <var>request</var> with <var data-x="navigation-hh">historyHandling</var>
@@ -85796,7 +85831,9 @@ new PaymentRequest(&hellip;); // Allowed to use
    data-x="she-document">document</span>.</p></li>
 
    <li><p>If <var>newDocument</var> is different than the <span>current entry</span>'s <span
-   data-x="she-document">document</span>, then:</p>
+   data-x="she-document">document</span>, or <var>historyHandling</var> is "<code
+   data-x="hh-entry-update">entry update</code>" or "<code data-x="hh-reload">reload</code>",
+   then:</p>
 
     <ol>
      <li><p>Remove any <span data-x="concept-task">tasks</span> queued by the <span>history traversal