From 14e263e790ccaef8b1c17421f2c9ec4e171c6c67 Mon Sep 17 00:00:00 2001
From: Adrien Siami <adrien@siami.fr>
Date: Thu, 8 Feb 2024 16:08:24 +0100
Subject: [PATCH] Stop reloading turbo frames when complete attribute changes

---
 src/core/frames/frame_controller.js           | 22 +++++-----------
 src/elements/frame_element.js                 |  6 ++---
 .../frame_refresh_after_navigation.html       |  3 +++
 src/tests/fixtures/page_refresh.html          |  5 ++++
 src/tests/functional/loading_tests.js         | 11 --------
 src/tests/functional/page_refresh_tests.js    | 26 +++++++++++++++++++
 6 files changed, 42 insertions(+), 31 deletions(-)
 create mode 100644 src/tests/fixtures/frame_refresh_after_navigation.html

diff --git a/src/core/frames/frame_controller.js b/src/core/frames/frame_controller.js
index e82e097f4..07764fc86 100644
--- a/src/core/frames/frame_controller.js
+++ b/src/core/frames/frame_controller.js
@@ -90,20 +90,12 @@ export class FrameController {
 
   sourceURLReloaded() {
     const { src } = this.element
-    this.#ignoringChangesToAttribute("complete", () => {
-      this.element.removeAttribute("complete")
-    })
+    this.element.removeAttribute("complete")
     this.element.src = null
     this.element.src = src
     return this.element.loaded
   }
 
-  completeChanged() {
-    if (this.#isIgnoringChangesTo("complete")) return
-
-    this.#loadSourceURL()
-  }
-
   loadingStyleChanged() {
     if (this.loadingStyle == FrameLoadingStyle.lazy) {
       this.appearanceObserver.start()
@@ -528,13 +520,11 @@ export class FrameController {
   }
 
   set complete(value) {
-    this.#ignoringChangesToAttribute("complete", () => {
-      if (value) {
-        this.element.setAttribute("complete", "")
-      } else {
-        this.element.removeAttribute("complete")
-      }
-    })
+    if (value) {
+      this.element.setAttribute("complete", "")
+    } else {
+      this.element.removeAttribute("complete")
+    }
   }
 
   get isActive() {
diff --git a/src/elements/frame_element.js b/src/elements/frame_element.js
index 4feb36713..8dc2890f3 100644
--- a/src/elements/frame_element.js
+++ b/src/elements/frame_element.js
@@ -25,7 +25,7 @@ export class FrameElement extends HTMLElement {
   loaded = Promise.resolve()
 
   static get observedAttributes() {
-    return ["disabled", "complete", "loading", "src"]
+    return ["disabled", "loading", "src"]
   }
 
   constructor() {
@@ -48,11 +48,9 @@ export class FrameElement extends HTMLElement {
   attributeChangedCallback(name) {
     if (name == "loading") {
       this.delegate.loadingStyleChanged()
-    } else if (name == "complete") {
-      this.delegate.completeChanged()
     } else if (name == "src") {
       this.delegate.sourceURLChanged()
-    } else {
+    } else if (name == "disabled") {
       this.delegate.disabledChanged()
     }
   }
diff --git a/src/tests/fixtures/frame_refresh_after_navigation.html b/src/tests/fixtures/frame_refresh_after_navigation.html
new file mode 100644
index 000000000..4b8f79d03
--- /dev/null
+++ b/src/tests/fixtures/frame_refresh_after_navigation.html
@@ -0,0 +1,3 @@
+<turbo-frame id="refresh-after-navigation">
+  <h2 id="refresh-after-navigation-content">Frame has been navigated</h2>
+</turbo-frame>
diff --git a/src/tests/fixtures/page_refresh.html b/src/tests/fixtures/page_refresh.html
index 35cb44354..c0586677f 100644
--- a/src/tests/fixtures/page_refresh.html
+++ b/src/tests/fixtures/page_refresh.html
@@ -90,6 +90,11 @@ <h2>Frame to be morphed</h2>
       <h2>Frame to be reloaded</h2>
     </turbo-frame>
 
+    <turbo-frame id="refresh-after-navigation">
+      <h2>Frame to be navigated then reset to its initial state after reload</h2>
+      <a id="refresh-after-navigation-link" href="/src/tests/fixtures/frame_refresh_after_navigation.html">Navigate</a>
+    </turbo-frame>
+
     <div id="preserve-me" data-turbo-permanent>
       Preserve me!
 
diff --git a/src/tests/functional/loading_tests.js b/src/tests/functional/loading_tests.js
index f9c28f361..4390dfd57 100644
--- a/src/tests/functional/loading_tests.js
+++ b/src/tests/functional/loading_tests.js
@@ -119,17 +119,6 @@ test("navigating away from a page does not reload its frames", async ({ page })
   assert.equal(requestLogs.length, 1)
 })
 
-test("removing the [complete] attribute of an eager frame reloads the content", async ({ page }) => {
-  await nextEventOnTarget(page, "frame", "turbo:frame-load")
-  await page.evaluate(() => document.querySelector("#loading-eager turbo-frame")?.removeAttribute("complete"))
-  await nextEventOnTarget(page, "frame", "turbo:frame-load")
-
-  assert.ok(
-    await hasSelector(page, "#loading-eager turbo-frame[complete]"),
-    "sets the [complete] attribute after re-loading"
-  )
-})
-
 test("changing [src] attribute on a [complete] frame with loading=lazy defers navigation", async ({ page }) => {
   await page.click("#loading-lazy summary")
   await nextEventOnTarget(page, "hello", "turbo:frame-load")
diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js
index 6619ae58e..6453e48b7 100644
--- a/src/tests/functional/page_refresh_tests.js
+++ b/src/tests/functional/page_refresh_tests.js
@@ -184,6 +184,32 @@ test("frames marked with refresh='morph' are excluded from full page morphing",
   await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame")
 })
 
+test("navigated frames without refresh attribute are reset after morphing", async ({ page }) => {
+  await page.goto("/src/tests/fixtures/page_refresh.html")
+
+  await page.click("#refresh-after-navigation-link")
+
+  assert.ok(
+    await hasSelector(page, "#refresh-after-navigation-content"),
+    "navigates theframe"
+  )
+
+  await page.click("#form-submit")
+
+  await nextEventNamed(page, "turbo:render", { renderMethod: "morph" })
+  await nextBeat()
+
+  assert.ok(
+    await hasSelector(page, "#refresh-after-navigation-link"),
+    "resets the frame"
+  )
+
+  assert.notOk(
+    await hasSelector(page, "#refresh-after-navigation-content"),
+    "does not reload the frame"
+  )
+})
+
 test("it preserves the scroll position when the turbo-refresh-scroll meta tag is 'preserve'", async ({ page }) => {
   await page.goto("/src/tests/fixtures/page_refresh.html")