Fix back navigation after POST form submission #1014
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit fixes a bug where navigating back after submitting a POST form would not show the previous page. The bug was introduced in #949. The new cache API is async since it needs to access CacheStorage which is async. In that PR, getCachedSnapshot was changed to be async, but hasCachedSnapshot was not changed. This meant that getCachedSnapshot would always return a promise, which is truthy, and so hasCachedSnapshot which just checked that the return value was not null would always return true. The bug would show up when you tried to navigate back to a page after a post form submission. The form submission would clear the cache, but the restoration visit would think it had a cached snapshot and so would not issue a request. This meant that the page would not be restored and nothing would happen. There's a new test in navigation_tests.js that reproduces this bug. The fix is to make hasCachedSnapshot async and await it in the places where it is used.
kevinmcconnell
approved these changes
Sep 28, 2023
afcapel
added a commit
that referenced
this pull request
Nov 13, 2023
* Revert "Move cache snapshot loading to Visit#start() (#1056)" This reverts commit e07639f. * Revert "Fix back navigation after POST form submission (#1014)" This reverts commit c207f5b. * Revert "Add disk cache store (#949)" This reverts commit f86a376. * Remove redudant test prefix * Bring back test Orginally added by @jayohms in 60cfbee
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit fixes a bug where navigating back after submitting a POST form would not show the previous page. The bug was introduced in #949. The new cache API is async since it needs to access
CacheStoragewhich is async. In that PR,getCachedSnapshotwas changed to be async, buthasCachedSnapshotwas not changed. This meant thatgetCachedSnapshotwould always return a promise, which is truthy, and sohasCachedSnapshotwhich just checked that the return value was not null would always return true.The bug would show up when you tried to navigate back to a page after a post form submission. The form submission would clear the cache, but the restoration visit would think it had a cached snapshot and so would not issue a request. This meant that the page would not be restored and nothing would happen.
There's a new test in navigation_tests.js that reproduces this bug.
The fix is to make
hasCachedSnapshotasync and await it in the places where it is used.