Create Frame Snapshot from Fetch Response HTML

After the changes made in [@hotwired/turbohotwired#867][] and changes made in [@hotwired/turbo-railshotwired#428][] (the canonical server-side implementation), Turbo expects full HTML documents in response to requests with `Turbo-Frame:` headers. Prior to this commit, the `FrameController` compensated for missing pieces of an HTML document by taking an HTML "snapshot" of the current page through the `<html>` element's [outerHTML][]. This commit changes the `fetchResponseLoaded` callback to read the `responseHTML` directly from the `FetchResponse`, since that will be a fully formed HTML document in Turbo v7.3.0 and later. To support that change, this commit also updates various `src/test/fixtures` files to render fully-formed HTML documents. [@hotwired/turbohotwired#867]: hotwired#867 [@hotwired/turbo-railshotwired#428]: hotwired/turbo-rails#428 [outerHTML]: https://developer.mozilla.org/en-US/docs/Web/API/Element/outerHTML
seanpdoyle · Jul 21, 2023 · 3882c6a · 3882c6a
1 parent da647a6
commit 3882c6a
Show file tree

Hide file tree

Showing 22 changed files with 113 additions and 68 deletions.
diff --git a/src/core/frames/frame_controller.ts b/src/core/frames/frame_controller.ts
@@ -55,7 +55,7 @@ export class FrameController
   readonly linkInterceptor: LinkInterceptor
   readonly formSubmitObserver: FormSubmitObserver
   formSubmission?: FormSubmission
-  fetchResponseLoaded = (_fetchResponse: FetchResponse) => {}
+  fetchResponseLoaded = (_fetchResponse: FetchResponse) => Promise.resolve()
   private currentFetchRequest: FetchRequest | null = null
   private resolveVisitPromise = () => {}
   private connected = false
@@ -170,7 +170,7 @@ export class FrameController
         }
       }
     } finally {
-      this.fetchResponseLoaded = () => {}
+      this.fetchResponseLoaded = () => Promise.resolve()
     }
   }
 
@@ -344,7 +344,7 @@ export class FrameController
       this.complete = true
       session.frameRendered(fetchResponse, this.element)
       session.frameLoaded(this.element)
-      this.fetchResponseLoaded(fetchResponse)
+      await this.fetchResponseLoaded(fetchResponse)
     } else if (this.willHandleFrameMissingFromResponse(fetchResponse)) {
       this.handleFrameMissingFromResponse(fetchResponse)
     }
@@ -383,10 +383,10 @@ export class FrameController
       const pageSnapshot = PageSnapshot.fromElement(frame).clone()
       const { visitCachedSnapshot } = frame.delegate
 
-      frame.delegate.fetchResponseLoaded = (fetchResponse: FetchResponse) => {
+      frame.delegate.fetchResponseLoaded = async (fetchResponse: FetchResponse) => {
         if (frame.src) {
           const { statusCode, redirected } = fetchResponse
-          const responseHTML = frame.ownerDocument.documentElement.outerHTML
+          const responseHTML = await fetchResponse.responseHTML
           const response = { statusCode, redirected, responseHTML }
           const options: Partial<VisitOptions> = {
             response,

diff --git a/src/tests/fixtures/frame_preloading.html b/src/tests/fixtures/frame_preloading.html
@@ -1,14 +1,12 @@
 <!DOCTYPE html>
 <html>
-
-<head>
-  <meta charset="utf-8">
-  <title>Page With Preloading Frame</title>
-  <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
-</head>
-
-<body>
-  <turbo-frame id="menu" src="/src/tests/fixtures/frames/preloading.html"></turbo-frame>
-</body>
-
+  <head>
+    <meta charset="utf-8">
+    <title>Frame Preloading</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+  </head>
+  <body>
+    <turbo-frame id="menu" src="/src/tests/fixtures/frames/preloading.html"></turbo-frame>
+  </body>
 </html>
diff --git a/src/tests/fixtures/frames/body_script.html b/src/tests/fixtures/frames/body_script.html
@@ -2,8 +2,9 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Frame</title>
+    <title>Frames: Body Script</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
   </head>
   <body>
     <turbo-frame id="body-script" data-loaded-from="/src/tests/fixtures/frames/body_script.html">

diff --git a/src/tests/fixtures/frames/body_script_2.html b/src/tests/fixtures/frames/body_script_2.html
@@ -2,8 +2,9 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Frame</title>
+    <title>Frames: Body Script 2</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
   </head>
   <body>
     <turbo-frame id="body-script" data-loaded-from="/src/tests/fixtures/frames/body_script_2.html">

diff --git a/src/tests/fixtures/frames/eval_false_script.html b/src/tests/fixtures/frames/eval_false_script.html
@@ -2,8 +2,9 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Frame</title>
+    <title>Frames: Eval False Script</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
   </head>
   <body>
     <turbo-frame id="eval-false-script" data-loaded-from="/src/tests/fixtures/frames/eval_false_script.html">

diff --git a/src/tests/fixtures/frames/form-redirect.html b/src/tests/fixtures/frames/form-redirect.html
@@ -2,7 +2,7 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Frame</title>
+    <title>Frames: Form Redirect</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
     <script src="/src/tests/fixtures/test.js"></script>
   </head>

diff --git a/src/tests/fixtures/frames/form-redirected.html b/src/tests/fixtures/frames/form-redirected.html
@@ -2,7 +2,7 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Frame</title>
+    <title>Frames: Form Redirected</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
     <script src="/src/tests/fixtures/test.js"></script>
   </head>

diff --git a/src/tests/fixtures/frames/form.html b/src/tests/fixtures/frames/form.html
@@ -2,7 +2,7 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Form</title>
+    <title>Frames: Form</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
     <script src="/src/tests/fixtures/test.js"></script>
   </head>

diff --git a/src/tests/fixtures/frames/frame.html b/src/tests/fixtures/frames/frame.html
@@ -2,8 +2,9 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Frame</title>
+    <title>Frames: Frame</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
   </head>
   <body>
     <h1>Frames: #frame</h1>

diff --git a/src/tests/fixtures/frames/frame_for_eager.html b/src/tests/fixtures/frames/frame_for_eager.html
@@ -1,3 +1,14 @@
-<turbo-frame id="eager-loaded-frame" >
-  <h2>Eager-loaded frame: Loaded</h2>
-</turbo-frame>
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Frames: Frame for Eager</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+  </head>
+  <body>
+    <turbo-frame id="eager-loaded-frame" >
+      <h2>Eager-loaded frame: Loaded</h2>
+    </turbo-frame>
+  </body>
+</html>
diff --git a/src/tests/fixtures/frames/hello.html b/src/tests/fixtures/frames/hello.html
@@ -2,7 +2,7 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Frame</title>
+    <title>Frames: Hello</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
     <script src="/src/tests/fixtures/test.js"></script>
   </head>

diff --git a/src/tests/fixtures/frames/part.html b/src/tests/fixtures/frames/part.html
@@ -1,3 +1,14 @@
-<turbo-frame id="part">
-  <h2>Frames: #frame-part</h2>
-</turbo-frame>
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Frames: Part</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+  </head>
+  <body>
+    <turbo-frame id="part">
+      <h2>Frames: #frame-part</h2>
+    </turbo-frame>
+  </body>
+</html>
diff --git a/src/tests/fixtures/frames/preloading.html b/src/tests/fixtures/frames/preloading.html
@@ -1,4 +1,15 @@
-<turbo-frame id="menu">
-  <a href="/src/tests/fixtures/preloaded.html" id="frame_preload_anchor" data-turbo-preload="true">Visit preloaded
-    page</a>
-</turbo-frame>
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Frames: Preloading</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+  </head>
+  <body>
+    <turbo-frame id="menu">
+      <a href="/src/tests/fixtures/preloaded.html" id="frame_preload_anchor" data-turbo-preload="true">Visit preloaded
+        page</a>
+    </turbo-frame>
+  </body>
+</html>
diff --git a/src/tests/fixtures/frames/recursive.html b/src/tests/fixtures/frames/recursive.html
@@ -2,8 +2,9 @@
 <html>
   <head>
     <meta charset="utf-8">
-    <title>Frame</title>
+    <title>Frames: Recursive</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
   </head>
   <body>
     <turbo-frame id="recursive">

diff --git a/src/tests/fixtures/frames/self.html b/src/tests/fixtures/frames/self.html
@@ -4,6 +4,7 @@
     <meta charset="utf-8">
     <title>Frames: Self</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
   </head>
   <body>
     <turbo-frame id="frame" src="/src/tests/fixtures/frames/self.html">

diff --git a/src/tests/fixtures/frames/unvisitable.html b/src/tests/fixtures/frames/unvisitable.html
@@ -1,9 +1,10 @@
 <!DOCTYPE html>
 <html>
   <head>
-    <meta charset="utf-8" />
+    <meta charset="utf-8">
     <title>Frame</title>
     <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
     <meta name="turbo-visit-control" content="reload" />
   </head>
   <body>

diff --git a/src/tests/fixtures/frames/without_layout.html b/src/tests/fixtures/frames/without_layout.html
diff --git a/src/tests/fixtures/rendering.html b/src/tests/fixtures/rendering.html
@@ -43,7 +43,6 @@ <h1>Rendering</h1>
       <p><a id="visit-control-reload-link" href="/src/tests/fixtures/visit_control_reload.html">Visit control: reload</a></p>
       <p><a id="permanent-element-link" href="/src/tests/fixtures/permanent_element.html">Permanent element</a></p>
       <p><a id="permanent-in-frame-element-link" href="/src/tests/fixtures/permanent_element.html" data-turbo-frame="frame">Permanent element in frame</a></p>
-      <p><a id="permanent-in-frame-without-layout-element-link" href="/src/tests/fixtures/frames/without_layout.html" data-turbo-frame="frame">Permanent element in frame without layout</a></p>
       <p><a id="delayed-link" href="/__turbo/delayed_response">Delayed link</a></p>
       <p><a id="redirect-link" href="/__turbo/redirect">Redirect link</a></p>
       <form>

diff --git a/src/tests/fixtures/tabs.html b/src/tests/fixtures/tabs.html
@@ -3,7 +3,7 @@
   <head>
     <meta charset="utf-8">
     <title>Tabs</title>
-    <script src="/dist/turbo.es2017-umd.js"></script>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
     <script src="/src/tests/fixtures/test.js"></script>
   </head>
   <body>

diff --git a/src/tests/fixtures/tabs/three.html b/src/tests/fixtures/tabs/three.html
@@ -1,9 +1,20 @@
-<turbo-frame id="tab-frame" data-turbo-action="advance">
-  <div>
-    <a id="tabs-1" href="/src/tests/fixtures/tabs.html">Tab 1</a>
-    <a id="tabs-2" href="/src/tests/fixtures/tabs/two.html">Tab 2</a>
-    <a id="tabs-3" href="/src/tests/fixtures/tabs/three.html">Tab 3</a>
-  </div>
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Frame</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+  </head>
+  <body>
+    <turbo-frame id="tab-frame" data-turbo-action="advance">
+      <div>
+        <a id="tab-1" href="/src/tests/fixtures/tabs.html">Tab 1</a>
+        <a id="tab-2" href="/src/tests/fixtures/tabs/two.html">Tab 2</a>
+        <a id="tab-3" href="/src/tests/fixtures/tabs/three.html">Tab 3</a>
+      </div>
 
-  <div id="tab-content">Three</div>
-</turbo-frame>
+      <div id="tab-content">Three</div>
+    </turbo-frame>
+  </body>
+</html>
diff --git a/src/tests/fixtures/tabs/two.html b/src/tests/fixtures/tabs/two.html
@@ -1,9 +1,20 @@
-<turbo-frame id="tab-frame" data-turbo-action="advance">
-  <div>
-    <a id="tab-1" href="/src/tests/fixtures/tabs.html">Tab 1</a>
-    <a id="tab-2" href="/src/tests/fixtures/tabs/two.html">Tab 2</a>
-    <a id="tab-3" href="/src/tests/fixtures/tabs/three.html">Tab 3</a>
-  </div>
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Frame</title>
+    <script src="/dist/turbo.es2017-umd.js" data-turbo-track="reload"></script>
+    <script src="/src/tests/fixtures/test.js"></script>
+  </head>
+  <body>
+    <turbo-frame id="tab-frame" data-turbo-action="advance">
+      <div>
+        <a id="tab-1" href="/src/tests/fixtures/tabs.html">Tab 1</a>
+        <a id="tab-2" href="/src/tests/fixtures/tabs/two.html">Tab 2</a>
+        <a id="tab-3" href="/src/tests/fixtures/tabs/three.html">Tab 3</a>
+      </div>
 
-  <div id="tab-content">Two</div>
-</turbo-frame>
+      <div id="tab-content">Two</div>
+    </turbo-frame>
+  </body>
+</html>
diff --git a/src/tests/functional/rendering_tests.ts b/src/tests/functional/rendering_tests.ts
@@ -421,15 +421,6 @@ test("test preserves permanent elements within turbo-frames", async ({ page }) =
   assert.equal(await page.textContent("#permanent-in-frame"), "Rendering")
 })
 
-test("test preserves permanent elements within turbo-frames rendered without layouts", async ({ page }) => {
-  assert.equal(await page.textContent("#permanent-in-frame"), "Rendering")
-
-  await page.click("#permanent-in-frame-without-layout-element-link")
-  await nextBeat()
-
-  assert.equal(await page.textContent("#permanent-in-frame"), "Rendering")
-})
-
 test("test restores focus during turbo-frame rendering when transposing the activeElement", async ({ page }) => {
   await page.press("#permanent-input-in-frame", "Enter")
   await nextBeat()