Updating the session history: same-URL case is broken #6202
Comments
|
That sounds right and I like the suggestion of making it more concrete. I think part of the problem with the lifecycle stuff is that there's not clearly defined state on browsing context to manage navigations. E.g., that it's currently navigating and where in that process it's at. |
This all seems very sound to me, and kinda sells me on the idea of adding a step the #navigate like you mentioned. As you mentioned over in #6197 I think it makes sense to fix the "easy" problem as part of that work, and then piggyback this on-top of it after a little more analysis perhaps |
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.
|
@domfarolino noted that this step might be trying to capture |
Closes #6202. Also tidies up the "update the session history with the new page" algorithm more generally, e.g. to be explicit about what session history is being accessed.
Closes #6202. Also tidies up the "update the session history with the new page" algorithm more generally, e.g. to be explicit about what session history is being accessed.
Spinning off from #6197.
I spent some time figuring out what
means. Back in the day, it used to say
so this is another thing I messed up with the current entry vs. newDocument business, discussed a bit in #6197. It should look at navigationParams's browsing context's active document's URL, to capture the old meaning. That's the easy bug.
The harder bug is as follows.
First, what is the point of this step? It's to capture the fact that if JS code does
location.href = location.href, or the user clicks on a link which takes them to the same URL as the current URL, then we actually do a "replace" behavior. (Or at least something very like it?) You can test this yourself by using the JS console, or by creating a page which links to itself: no matter how many times you self-navigate,history.lengthdoes not change, andhistory.back()will kick you off the page entirely.However, the phrasing here is very delicate. "the navigation was initiated with a URL"... well, navigation is often initiated with requests, not with URLs. In particular, the
<a href="current_url">case used to be initiated with a URL, but when we introducedreferrerpolicy="", we switched it to being initiated with a request. So we broke this step, at least by a strict reading. Oops.Also, this step is very action-at-a-distance, grabbing what "navigation was initiated with" waaaay after navigation was done initiating.
I think the best way to fix this would be to change it entirely. In particular, around step 6 of navigate itself, insert something like
It seems like there might be cases where browsingContext's active document's URL changes between early-in-navigate and late-in-navigate, i.e., this might change the semantics a bit beyond just fixing the bug. But if so, I think the new version would be even more correct than the old one. (And I'm not sure there are such cases, as any new navigations that might change the active document probably take precedence and cancel the old ones before they could get to
#update-the-session-history-with-the-new-page.)Thoughts? @domfarolino especially, but @annevk for a third opinion would be great.
The text was updated successfully, but these errors were encountered: