From 4b292d4808e5e811cc39e98eeaaf99989d5ea28c Mon Sep 17 00:00:00 2001
From: Alberto Fernandez-Capel <alberto@hey.com>
Date: Wed, 27 Sep 2023 15:27:24 +0100
Subject: [PATCH] Fix back navigation after POST form submission

This commit fixes a bug where navigating back after submitting a POST
form would not show the previous page. The bug was introduced in
https://github.com/hotwired/turbo/pull/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.
---
 src/core/drive/visit.js                  | 16 ++++++++--------
 src/core/native/browser_adapter.js       |  6 +-----
 src/tests/functional/navigation_tests.js |  6 ++++++
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/core/drive/visit.js b/src/core/drive/visit.js
index d1929b03e..6cf1b52c7 100644
--- a/src/core/drive/visit.js
+++ b/src/core/drive/visit.js
@@ -154,10 +154,10 @@ export class Visit {
     }
   }
 
-  issueRequest() {
+  async issueRequest() {
     if (this.hasPreloadedResponse()) {
       this.simulateRequest()
-    } else if (this.shouldIssueRequest() && !this.request) {
+    } else if (!this.request && await this.shouldIssueRequest()) {
       this.request = new FetchRequest(this, FetchMethod.get, this.location)
       this.request.perform()
     }
@@ -231,14 +231,14 @@ export class Visit {
     }
   }
 
-  hasCachedSnapshot() {
-    return this.getCachedSnapshot() != null
+  async hasCachedSnapshot() {
+    return (await this.getCachedSnapshot()) != null
   }
 
   async loadCachedSnapshot() {
     const snapshot = await this.getCachedSnapshot()
     if (snapshot) {
-      const isPreview = this.shouldIssueRequest()
+      const isPreview = await this.shouldIssueRequest()
       this.render(async () => {
         this.cacheSnapshot()
         if (this.isSamePage) {
@@ -391,11 +391,11 @@ export class Visit {
     return typeof this.response == "object"
   }
 
-  shouldIssueRequest() {
+  async shouldIssueRequest() {
     if (this.isSamePage) {
       return false
-    } else if (this.action == "restore") {
-      return !this.hasCachedSnapshot()
+    } else if (this.action === "restore") {
+      return !(await this.hasCachedSnapshot())
     } else {
       return this.willRender
     }
diff --git a/src/core/native/browser_adapter.js b/src/core/native/browser_adapter.js
index 7505d7f8f..c3f1086de 100644
--- a/src/core/native/browser_adapter.js
+++ b/src/core/native/browser_adapter.js
@@ -22,11 +22,7 @@ export class BrowserAdapter {
 
   visitRequestStarted(visit) {
     this.progressBar.setValue(0)
-    if (visit.hasCachedSnapshot() || visit.action != "restore") {
-      this.showVisitProgressBarAfterDelay()
-    } else {
-      this.showProgressBar()
-    }
+    this.showVisitProgressBarAfterDelay()
   }
 
   visitRequestCompleted(visit) {
diff --git a/src/tests/functional/navigation_tests.js b/src/tests/functional/navigation_tests.js
index f18a7ea1e..c4263d0f3 100644
--- a/src/tests/functional/navigation_tests.js
+++ b/src/tests/functional/navigation_tests.js
@@ -195,7 +195,13 @@ test("test following a POST form clears cache", async ({ page }) => {
   await page.click("#form-post-submit")
   await nextBeat() // 301 redirect response
   await nextBeat() // 200 response
+
+  assert.equal(await page.textContent("h1"), "One")
+
   await page.goBack()
+  await nextBeat()
+
+  assert.equal(await page.textContent("h1"), "Navigation")
   assert.notOk(await hasSelector(page, "some-cached-element"))
 })