Change default action for form submissions that redirect to the curre…

…nt location (#1072)

* Change default action for form submissions that redirect to the current location

This changes the default action for form submission that redirect to the
current location from "advance" to "replace". This makes more sense since
we're re-rendering the current page, there should be no new entry in the
history.

We are also changing the morphing behaviour to only kick in when the
visit action is "replace". This is to avoid morphing the page when, for example,
we are follwing a link, unless we opt in to morphing by setting the
data-turbo-action attribute to replace.

The main use case for morphing remains the same: you submit a form that
changes some state and refirect to the same location. Turbo handles the
redirect and morphs the page to reflect the new state.

* Add parentheses to clarify conditional

src/core/drive/navigator.js

@@ -78,7 +78,7 @@ export class Navigator {
         }
 
         const { statusCode, redirected } = fetchResponse
-        const action = this.getActionForFormSubmission(formSubmission)
+        const action = this.#getActionForFormSubmission(formSubmission, fetchResponse)
         const visitOptions = {
           action,
           shouldCacheSnapshot,
@@ -153,7 +153,13 @@ export class Navigator {
     return this.history.restorationIdentifier
   }
 
-  getActionForFormSubmission({ submitter, formElement }) {
-    return getVisitAction(submitter, formElement) || "advance"
+  #getActionForFormSubmission(formSubmission, fetchResponse) {
+    const { submitter, formElement } = formSubmission
+    return getVisitAction(submitter, formElement) || this.#getDefaultAction(fetchResponse)
+  }
+
+  #getDefaultAction(fetchResponse) {
+    const sameLocationRedirect = fetchResponse.redirected && fetchResponse.location.href === this.location?.href
+    return sameLocationRedirect ? "replace" : "advance"
   }
 }

src/core/drive/page_view.js

@@ -56,7 +56,7 @@ export class PageView extends View {
   }
 
   isPageRefresh(visit) {
-    return !visit || this.lastRenderedLocation.href === visit.location.href
+    return !visit || (this.lastRenderedLocation.href === visit.location.href && visit.action === "replace")
   }
 
   get snapshot() {

src/tests/fixtures/navigation.html

@@ -79,6 +79,17 @@ <h1>Navigation</h1>
           </custom-link-element>
         </turbo-toggle>
       </p>
+      <p>
+        <form action="/__turbo/redirect" method="post" class="redirect" data-turbo-frame="_top">
+          <input type="hidden" name="path" value="/src/tests/fixtures/one.html">
+          <button id="redirect-submit">Redirect to the another location</button>
+        </form>
+      </p>
+      <p>
+        <form action="/__turbo/refresh" method="post" class="redirect">
+          <button id="refresh-submit">Redirect to the current location</button>
+        </form>
+      </p>
       <p><a id="delayed-link" href="/__turbo/delayed_response">Delayed link</a></p>
       <p><a id="delayed-failure-link" href="/__turbo/delayed_response?status=500">Delayed failure link</a></p>
       <p><a id="link-target-iframe" href="/src/tests/fixtures/one.html" target="iframe">Targets iframe[name="iframe"]</a></p>

src/tests/functional/navigation_tests.js

@@ -307,6 +307,18 @@ test("clicking the forward button", async ({ page }) => {
   assert.equal(await visitAction(page), "restore")
 })
 
+test("form submissions that redirect to a different location have a default advance action", async ({ page }) => {
+  await page.click("#redirect-submit")
+  await nextBody(page)
+  assert.equal(await visitAction(page), "advance")
+})
+
+test("form submissions that redirect to the current location have a default replace action", async ({ page }) => {
+  await page.click("#refresh-submit")
+  await nextBody(page)
+  assert.equal(await visitAction(page), "replace")
+})
+
 test("link targeting a disabled turbo-frame navigates the page", async ({ page }) => {
   await page.click("#link-to-disabled-frame")
   await nextBody(page)