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.

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

Clarify history.pushState/replaceState behavior with history navigation, bfcache #6207

Open
rakina opened this issue Dec 10, 2020 · 16 comments
Assignees
Labels
needs tests Moving the issue forward requires someone to write tests topic: history

Comments

@rakina
Copy link
Member

rakina commented Dec 10, 2020

We recently found an interesting case with history navigation to/from entries created with history.pushState or history.replaceState.

  1. Navigate to A.html
  2. pushState B.html, we're still showing the page to A.html but the URL in the address bar is set to B.html (expected)
  3. Navigate to C.html
  4. Do a back navigation, we'll load B.html (expected)
  5. Do a back navigation, we will still show the page loaded for B.html, but the URL in the address bar changes to A.html (not expected, according to spec - see below)
    (Also, if you change step 4 to do a back-2-steps navigation it will load the contents of A.html, and if you go forward it will still show A.html but with the URL bar showing B.html)

Here's a repro page. I think according to the current spec, step 5 we would load the contents of A.html, but looks like in Chrome, Firefox, and Safari we will stay with the contents of B.html page. Steps according to the spec:

  • Step 1 wil create a new entry "entry1"
  • Step 2 will create a new entry "entry2" with URL and history update steps
  • Step 3 will navigate away. The entry created in step 2 might retain its document if it's bfcached, otherwise the document will get discarded. Let's assume the latter case for now.
  • Step 4 will navigate to the "entry2" created in step 2. This will run traverse the history, and since the document for that is already discarded we will run navigate that loads B.html
  • Step 5 will navigate to the "entry1" created in step 1. The document for "entry1" should also already be discarded at this point so we will run navigate that loads A.html (instead of only updating the URL bar like what happens in Chrome/Firefox/Safari)

With bfcache, the expectation is different. On step 4, if the document is still around, we will just load the bfcached A.html document (but the URL bar is set to B.html). On step 5, the document is also still alive so we will stay in A.html + set the URL bar to A.html. This is the same as the current non-bfcache behavior in Chrome/Firefox/Safari.

I'm wondering if the current non-bfcache behavior is desirable, and if so whether we should update the spec to allow this. If we decide that the currently-implemented behavior is wrong, this means the behavior with vs without bfcache will be different (not sure if that's a big deal).

cc navigation/history-related folks @annevk @csreis @domenic @cdumez @altimin
(see bug where we originally found this).

@annevk
Copy link
Member

annevk commented Dec 10, 2020

This sounds a lot like #4488 and https://www.w3.org/Bugs/Public/show_bug.cgi?id=27188. Personally I think the behavior makes sense, but as discussed A.html needs to be given the appropriate events so it can make the document appear as expected. (Unfortunately back then no tests or browser bugs would be filed if the specification was deemed acceptable...)

cc @smaug---- @petervanderbeken

@rakina
Copy link
Member Author

rakina commented Dec 10, 2020

If we want to allow this, we might need to add a "document ID" or something like that to session history entry, and modify the traverse the history steps to only trigger a navigation/new load if the document ID differs. This is what Chrome does with Document Sequence Numbers.

The behavior is pretty not obvious to me, so maybe we also want to add clarifying notes in the spec. Not sure if this can have side effects in other parts of the spec too. cc @jakearchibald in case this affects #5767 (maybe not, if we only need to add the document ID part)

@smaug----
Copy link

I think the behavior is somewhat obvious, as much as it can be given the weird behavior pushState with an URL causes.

@fergald
Copy link

fergald commented Apr 21, 2021

Let's say there is a block of history entries that all came from the same document via pushState and the current entry is outside of this block. If I history-navigate back into that block, I might get a bfcached page or load a new one. The question is what happens if I then history-navigate to another page in that block.

1 If the page came out of bfcache, that navigation will just be a URL/state change (this is exactly the same as if I had never left the block)
2 if it didn't come out of bfcache we do a navigation to that the URL (looks like it won't get the serialized state) and the same for any other pages I go to within the block.

If I history-navigate to more and more of those pages I will turn that previously contiguous block that shared a document into a block of entries each with distinct documents.

It actually seems reasonable (in fact what's implemented) that for 2, we would do no more navigations and full loads of pages as we move around inside that block.

@smaug---- Are you saying 2 is obvious or that what we've implemented is obvious? Obviously the right thing?

I cannot find anything in the spec that would reset the entry.document = null when a document is discarded, am I missing something?

If we don't reset to null then we will almost never enter the if of Step 1 of history traversal and if we do reset to null then we will do a full navigate every time in 4 (edited, I said "2" here originally).

@jakearchibald
Copy link
Contributor

jakearchibald commented Apr 21, 2021

The current browser behaviour (rather than the spec behaviour) makes way more sense when it comes to hash navigations:

  1. https://iframe-session-history.glitch.me/.
  2. Click "set cookie: give all responses no-store" and refresh the page. This disables bfcache.
  3. Click 'navigate to hash'.
  4. Click 'navigate to same-origin page'.
  5. Click the browser back button.
  6. Click the browser back button.

The spec says that step 6 should result in another network request, creating a new document. Whereas browsers just change the hash. The spec behaviour doesn't make sense to me, so I'm glad that browsers do what they do.

The browser pushState behaviour is consistent with the hash change behaviour, but I agree it's pretty weird.

Let's say the developer used pushState to allow / to display /article-1 in the URL bar, without changing the document. The code served from / is a full 'app', so it handles the transition to /article-1 with all kinds of fancy animations etc etc.

However, it isn't obvious that the code at /article-1 must also be a full 'app', and be able to handle a transition back to /. But that's what browsers assume right now.

I don't have any better ideas though. It feels like it should load / but with the /article-1 state, but I don't think that would be a web-compatible change now.

@fergald
Copy link

fergald commented Apr 22, 2021

However, it isn't obvious that the code at /article-1 must also be a full 'app', and be able to handle a transition back to /. But that's what browsers assume right now.

It might not be the full app but it also seems reasonable to say, that all URLs that can pushStated should be the full app. It would be weird for a site to have 2 versions of a page, one that is served as not-the-full-app if you go there directly and the other if you get there by clicking around in the app.

I don't know the history of pushState and there doesn't seem to be an explainer so I don't know what was in mind at the time but since all of the impls seem to assume the above, it seems like we should fix the spec. I've also checked that in chrome that both

  • A->pushState(B)->C->Back(1)->Back(1)
  • A->pushState(B)->C->Back(2)->Forward(1)

result in a navigation for the first Back and a URL change only for the next move. So, the impl assumes that that entire block of history is 1 app and it doesn't really matter how you get into it, once you're in it, you're in the app.

@fergald
Copy link

fergald commented Apr 22, 2021

Here's another scenario

A->pushState(B)->C->Back(1)->C->Back(1)->Back(1)

In chrome that last navigation will just change URL and not do a load.

So the history looks like
A
B
C

but at the point of the last navigation A and B's only connection is that when they were created they were the same document so I think having concept of the original document for an entry might be useful.

@jakearchibald
Copy link
Contributor

In #6315 I'm going to spec what browsers currently do.

In this model, a session history entry has a document state. That state is shared between history entries that share the same document.

That means, if the document is discarded, or is updated following a reload, it updates across all the history entries that share the document state.

I followed up in WICG/navigation-api#99 (comment), in case we need to help developer to do the right thing.

@fergald
Copy link

fergald commented Apr 22, 2021

That sounds great. So it seems like the current behaviour and the bfcache behaviour are in line and only the spec is out of line and @rakina's original concerns are not a problem?

@jakearchibald
Copy link
Contributor

That's certainly my view. Others may reject my changes though 😄

@jakearchibald
Copy link
Contributor

Fwiw, I created a video going through this behaviour https://www.youtube.com/watch?v=CY-Zv0bmmRk

@domenic
Copy link
Member

domenic commented Apr 23, 2021

Oh, wow, that video is mind-blowing. I guess we have to spec it, but there's a good amount of non-normative or semi-normative text that will need updating. Ctrl+Fing "contiguous" finds a good number of them.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 13, 2021
Bug: 1107415, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Oct 19, 2021
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 7, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 7, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 16, 2022
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jan 16, 2022
Bug: 1107415, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 9, 2022
Bug: 1107415, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 9, 2022
When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Bug: 1107415, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 9, 2022
When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Bug: 1107415, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 17, 2022
When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Tests pass on Firefox/Safari/Chrome,
except for Safari on non-BFCache case
(just because WebLock doesn't disable BFCache).

Bug: 1107415, 1298336, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
aarongable pushed a commit to chromium/chromium that referenced this issue Feb 18, 2022
When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Tests pass on Firefox/Safari/Chrome,
except for Safari on non-BFCache case
(just because WebLock doesn't disable BFCache).

Bug: 1107415, 1298336, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3199857
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972799}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 18, 2022
When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Tests pass on Firefox/Safari/Chrome,
except for Safari on non-BFCache case
(just because WebLock doesn't disable BFCache).

Bug: 1107415, 1298336, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3199857
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972799}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Feb 18, 2022
When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Tests pass on Firefox/Safari/Chrome,
except for Safari on non-BFCache case
(just because WebLock doesn't disable BFCache).

Bug: 1107415, 1298336, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3199857
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972799}
DanielRyanSmith pushed a commit to DanielRyanSmith/wpt that referenced this issue Feb 28, 2022
When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Tests pass on Firefox/Safari/Chrome,
except for Safari on non-BFCache case
(just because WebLock doesn't disable BFCache).

Bug: 1107415, 1298336, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3199857
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972799}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Mar 22, 2022
…n-BFCache cases, a=testonly

Automatic update from web-platform-tests
[WPT] BFCache: pushState() in BFCache/non-BFCache cases

When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Tests pass on Firefox/Safari/Chrome,
except for Safari on non-BFCache case
(just because WebLock doesn't disable BFCache).

Bug: 1107415, 1298336, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3199857
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972799}

--

wpt-commits: a400637a6ee3b7b6f315be2a17a7b28257663b4b
wpt-pr: 31083
aosmond pushed a commit to aosmond/gecko that referenced this issue Mar 26, 2022
…n-BFCache cases, a=testonly

Automatic update from web-platform-tests
[WPT] BFCache: pushState() in BFCache/non-BFCache cases

When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Tests pass on Firefox/Safari/Chrome,
except for Safari on non-BFCache case
(just because WebLock doesn't disable BFCache).

Bug: 1107415, 1298336, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3199857
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972799}

--

wpt-commits: a400637a6ee3b7b6f315be2a17a7b28257663b4b
wpt-pr: 31083
@jakearchibald jakearchibald self-assigned this Apr 19, 2022
@jakearchibald
Copy link
Contributor

work is ongoing

mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
When doing history navigation to an entry that's created
via pushState, the page should be:

- With BFCache: Restored from BFCache.
- Without BFCache: Loaded from the URL set by `pushState()`.

While this test contradicts the current spec but matches
the desired behavior, and the spec will be
fixed as part of whatwg/html#6315.

Tests pass on Firefox/Safari/Chrome,
except for Safari on non-BFCache case
(just because WebLock doesn't disable BFCache).

Bug: 1107415, 1298336, whatwg/html#6207
Change-Id: I609276fe865fa92409fd7a547777dba222bac36c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3199857
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Cr-Commit-Position: refs/heads/main@{#972799}
NOKEYCHECK=True
GitOrigin-RevId: 760e13ee79efa16c7dfc42e86c12da60e1aed9d6
@annevk
Copy link
Member

annevk commented Feb 14, 2023

Was this resolved by the navigation and history rewrite?

@domenic
Copy link
Member

domenic commented Feb 14, 2023

I'm not sure. The spec might have a sensible answer now, but I'm pretty sure we didn't create WPTs for that answer if so, so leaving this open to track that work seems good.

@annevk annevk added the needs tests Moving the issue forward requires someone to write tests label Feb 14, 2023
@nightpool
Copy link

Hey all, we just ran into this today on our site. Our use-case is something like the following: We have a "main page", where you can view an artist and modals displaying their albums, songs, activity, etc and several "sub pages" for each individual list (e.g. an artist songs list, and artist albums list, etc). These subpages are relatively simple, but for SEO reasons we want to make sure they have the same URL no matter how the user views that content (either through the modal from the "main page" or from the "sub page")

Using pushState, this looks something like:

  1. User loads artist page (navigate /artists/foo)
  2. User loads songs modal (pushState /artists/foo/songs)
  3. User closes song modal (history.back)

This all works fine. However, once the user navigates away, the problems start:

  1. User loads artist page (navigate /artists/foo)
  2. User loads song modal (pushState /artists/foo/songs)
  3. User clicks a song link and navigates to a song page (navigate /foo-song-lyrics)
  4. User clicks back button (browser navigates to /artists/foo/songs).
    • This loads the sub-page, which is unexpected for users, who expect the modal to be loaded
  5. User navigates backwards (popState /artists/foo).
    • This fails entirely because there's no way for the sub page to know how to display the full artist page. In theory I guess we could add specific code to the sub page to handle the popState event, but that doesn't make a lot of sense because so far we haven't needed to do anything at all to the sub page to support this modal work. The only page we've ever loaded, and the only page the user ever expects to load, is the main artist page.

Is there any way we can work around this as-is today? Maybe something with the history state object?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs tests Moving the issue forward requires someone to write tests topic: history
Development

No branches or pull requests

7 participants