From 7ac1faf72c92ff4f6f3d3a0c14601fc7d61822be Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@hey.com>
Date: Mon, 23 Oct 2023 10:56:39 +0200
Subject: [PATCH] Don't morph frames flagged with "refresh=morph" as part of
 the full page refresh

We will refresh those frames with morphing as part of the page refresh, so it does
not make sense to morph them also as part of the full page refresh. If we do, we'll
trigger a manual reload because the complete attribute will get removed. This aligns
the upstreamed version with the private gem we've been using internally.

This also fixes a couple of issues:

- We don't want to manually reload all the remote turbo-frames, only those that are
flagged with "refresh=morph". Regular remote frames will get reloaded automatically
when removing their complete attribute during regular page refreshes.

- Using idiomorph's "innerHTML" was resulting in a turbo-frame nested inside the
target turbo-frame. I think its semantics is not morphing inner contents from both
currentElement and newElement, but morphing newElement as the inner contents of currentElement.

See https://github.com/hotwired/turbo/pull/1019#issuecomment-1754539355
---
 src/core/drive/morph_renderer.js           |  8 +++++---
 src/tests/functional/page_refresh_tests.js | 18 ++++++++++++++++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/src/core/drive/morph_renderer.js b/src/core/drive/morph_renderer.js
index 8f8181464..7d99d76e4 100644
--- a/src/core/drive/morph_renderer.js
+++ b/src/core/drive/morph_renderer.js
@@ -26,6 +26,8 @@ export class MorphRenderer extends Renderer {
   }
 
   #morphElements(currentElement, newElement, morphStyle = "outerHTML") {
+    this.isMorphingTurboFrame = this.#isFrameReloadedWithMorph(currentElement)
+
     Idiomorph.morph(currentElement, newElement, {
       morphStyle: morphStyle,
       callbacks: {
@@ -40,8 +42,8 @@ export class MorphRenderer extends Renderer {
     this.#remoteFrames().forEach((frame) => {
       if (this.#isFrameReloadedWithMorph(frame)) {
         this.#renderFrameWithMorph(frame)
+        frame.reload()
       }
-      frame.reload()
     })
   }
 
@@ -56,7 +58,7 @@ export class MorphRenderer extends Renderer {
       target: currentElement,
       detail: { currentElement, newElement }
     })
-    this.#morphElements(currentElement, newElement, "innerHTML")
+    this.#morphElements(currentElement, newElement)
   }
 
   #shouldRemoveElement = (node) => {
@@ -65,7 +67,7 @@ export class MorphRenderer extends Renderer {
 
   #shouldMorphElement = (node) => {
     if (node instanceof HTMLElement) {
-      return !node.hasAttribute("data-turbo-permanent")
+      return !node.hasAttribute("data-turbo-permanent") && (this.isMorphingTurboFrame || !this.#isFrameReloadedWithMorph(node))
     } else {
       return true
     }
diff --git a/src/tests/functional/page_refresh_tests.js b/src/tests/functional/page_refresh_tests.js
index 73c0f4c5f..1422e1be5 100644
--- a/src/tests/functional/page_refresh_tests.js
+++ b/src/tests/functional/page_refresh_tests.js
@@ -41,6 +41,24 @@ test("uses morphing to update remote frames marked with refresh='morph'", async
   // Only the frame marked with refresh="morph" uses morphing
   expect(await nextEventOnTarget(page, "refresh-morph", "turbo:before-frame-morph")).toBeTruthy()
   expect(await noNextEventOnTarget(page, "refresh-reload", "turbo:before-frame-morph")).toBeTruthy()
+
+  await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame")
+
+  // Regular turbo-frames also gets reloaded since their complete attribute is removed
+  await expect(page.locator("#refresh-reload")).toHaveText("Loaded reloadable frame")
+})
+
+test("frames marked with refresh='morph' are excluded from full page morphing", async ({ page }) => {
+  await page.goto("/src/tests/fixtures/page_refresh.html")
+
+  await page.evaluate(() => document.getElementById("refresh-morph").setAttribute("data-modified", "true"))
+
+  await page.click("#form-submit")
+  await nextEventNamed(page, "turbo:render", { renderMethod: "morph" })
+  await nextBeat()
+
+  await expect(page.locator("#refresh-morph")).toHaveAttribute("data-modified", "true")
+  await expect(page.locator("#refresh-morph")).toHaveText("Loaded morphed frame")
 })
 
 test("it preserves the scroll position when the turbo-refresh-scroll meta tag is 'preserve'", async ({ page }) => {