From ed72ba16bc23dcccd88e0ab76ba72cfef82cd599 Mon Sep 17 00:00:00 2001 From: Sean Doyle Date: Thu, 2 Mar 2023 09:25:33 +0000 Subject: [PATCH] Create Frame Snapshot from Fetch Response HTML After the changes made in [@hotwired/turbo#867][] and changes made in [@hotwired/turbo-rails#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 `` 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/turbo#867]: https://github.com/hotwired/turbo/pull/867 [@hotwired/turbo-rails#428]: https://github.com/hotwired/turbo-rails/pull/428 [outerHTML]: https://developer.mozilla.org/en-US/docs/Web/API/Element/outerHTML --- src/core/frames/frame_controller.js | 10 +++---- src/tests/fixtures/frame_preloading.html | 20 +++++++------- src/tests/fixtures/frames/body_script.html | 3 ++- src/tests/fixtures/frames/body_script_2.html | 3 ++- .../fixtures/frames/eval_false_script.html | 3 ++- src/tests/fixtures/frames/form-redirect.html | 2 +- .../fixtures/frames/form-redirected.html | 2 +- src/tests/fixtures/frames/form.html | 2 +- src/tests/fixtures/frames/frame.html | 3 ++- .../fixtures/frames/frame_for_eager.html | 17 +++++++++--- src/tests/fixtures/frames/hello.html | 2 +- src/tests/fixtures/frames/part.html | 17 +++++++++--- src/tests/fixtures/frames/preloading.html | 19 ++++++++++--- src/tests/fixtures/frames/recursive.html | 3 ++- src/tests/fixtures/frames/self.html | 1 + src/tests/fixtures/frames/unvisitable.html | 3 ++- src/tests/fixtures/frames/without_layout.html | 5 ---- src/tests/fixtures/rendering.html | 1 - src/tests/fixtures/tabs.html | 2 +- src/tests/fixtures/tabs/three.html | 27 +++++++++++++------ src/tests/fixtures/tabs/two.html | 27 +++++++++++++------ src/tests/functional/rendering_tests.js | 9 ------- 22 files changed, 113 insertions(+), 68 deletions(-) delete mode 100644 src/tests/fixtures/frames/without_layout.html diff --git a/src/core/frames/frame_controller.js b/src/core/frames/frame_controller.js index 6fe1dcbe0..1cab2902e 100644 --- a/src/core/frames/frame_controller.js +++ b/src/core/frames/frame_controller.js @@ -26,7 +26,7 @@ import { PageSnapshot } from "../drive/page_snapshot" import { TurboFrameMissingError } from "../errors" export class FrameController { - fetchResponseLoaded = (_fetchResponse) => {} + fetchResponseLoaded = (_fetchResponse) => Promise.resolve() #currentFetchRequest = null #resolveVisitPromise = () => {} #connected = false @@ -140,7 +140,7 @@ export class FrameController { } } } finally { - this.fetchResponseLoaded = () => {} + this.fetchResponseLoaded = () => Promise.resolve() } } @@ -315,7 +315,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) } @@ -354,10 +354,10 @@ export class FrameController { const pageSnapshot = PageSnapshot.fromElement(frame).clone() const { visitCachedSnapshot } = frame.delegate - frame.delegate.fetchResponseLoaded = (fetchResponse) => { + frame.delegate.fetchResponseLoaded = async (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 = { response, diff --git a/src/tests/fixtures/frame_preloading.html b/src/tests/fixtures/frame_preloading.html index 4b8fd7a38..9fb02266f 100644 --- a/src/tests/fixtures/frame_preloading.html +++ b/src/tests/fixtures/frame_preloading.html @@ -1,14 +1,12 @@ - - - - Page With Preloading Frame - - - - - - - + + + Frame Preloading + + + + + + diff --git a/src/tests/fixtures/frames/body_script.html b/src/tests/fixtures/frames/body_script.html index 2d77614ae..6d8ae62f5 100644 --- a/src/tests/fixtures/frames/body_script.html +++ b/src/tests/fixtures/frames/body_script.html @@ -2,8 +2,9 @@ - Frame + Frames: Body Script + diff --git a/src/tests/fixtures/frames/body_script_2.html b/src/tests/fixtures/frames/body_script_2.html index 3bd5b8f2e..038dbd4c1 100644 --- a/src/tests/fixtures/frames/body_script_2.html +++ b/src/tests/fixtures/frames/body_script_2.html @@ -2,8 +2,9 @@ - Frame + Frames: Body Script 2 + diff --git a/src/tests/fixtures/frames/eval_false_script.html b/src/tests/fixtures/frames/eval_false_script.html index 3db83983b..fefdc6643 100644 --- a/src/tests/fixtures/frames/eval_false_script.html +++ b/src/tests/fixtures/frames/eval_false_script.html @@ -2,8 +2,9 @@ - Frame + Frames: Eval False Script + diff --git a/src/tests/fixtures/frames/form-redirect.html b/src/tests/fixtures/frames/form-redirect.html index 744dae2c2..18843a70c 100644 --- a/src/tests/fixtures/frames/form-redirect.html +++ b/src/tests/fixtures/frames/form-redirect.html @@ -2,7 +2,7 @@ - Frame + Frames: Form Redirect diff --git a/src/tests/fixtures/frames/form-redirected.html b/src/tests/fixtures/frames/form-redirected.html index 553e8d63e..1d118fbee 100644 --- a/src/tests/fixtures/frames/form-redirected.html +++ b/src/tests/fixtures/frames/form-redirected.html @@ -2,7 +2,7 @@ - Frame + Frames: Form Redirected diff --git a/src/tests/fixtures/frames/form.html b/src/tests/fixtures/frames/form.html index 01e0ef341..cc0f0d9e0 100644 --- a/src/tests/fixtures/frames/form.html +++ b/src/tests/fixtures/frames/form.html @@ -2,7 +2,7 @@ - Form + Frames: Form diff --git a/src/tests/fixtures/frames/frame.html b/src/tests/fixtures/frames/frame.html index 914cec830..4f9b32c3f 100644 --- a/src/tests/fixtures/frames/frame.html +++ b/src/tests/fixtures/frames/frame.html @@ -2,8 +2,9 @@ - Frame + Frames: Frame +

Frames: #frame

diff --git a/src/tests/fixtures/frames/frame_for_eager.html b/src/tests/fixtures/frames/frame_for_eager.html index 8445b3571..5727cc2fc 100644 --- a/src/tests/fixtures/frames/frame_for_eager.html +++ b/src/tests/fixtures/frames/frame_for_eager.html @@ -1,3 +1,14 @@ - -

Eager-loaded frame: Loaded

- + + + + + Frames: Frame for Eager + + + + + +

Eager-loaded frame: Loaded

+ + + diff --git a/src/tests/fixtures/frames/hello.html b/src/tests/fixtures/frames/hello.html index 6c41d263b..c01c40ecd 100644 --- a/src/tests/fixtures/frames/hello.html +++ b/src/tests/fixtures/frames/hello.html @@ -2,7 +2,7 @@ - Frame + Frames: Hello diff --git a/src/tests/fixtures/frames/part.html b/src/tests/fixtures/frames/part.html index 45ab6010d..bb498e971 100644 --- a/src/tests/fixtures/frames/part.html +++ b/src/tests/fixtures/frames/part.html @@ -1,3 +1,14 @@ - -

Frames: #frame-part

- + + + + + Frames: Part + + + + + +

Frames: #frame-part

+ + + diff --git a/src/tests/fixtures/frames/preloading.html b/src/tests/fixtures/frames/preloading.html index 08b4860b1..b4f56836a 100644 --- a/src/tests/fixtures/frames/preloading.html +++ b/src/tests/fixtures/frames/preloading.html @@ -1,4 +1,15 @@ - - Visit preloaded - page - + + + + + Frames: Preloading + + + + + + Visit preloaded + page + + + diff --git a/src/tests/fixtures/frames/recursive.html b/src/tests/fixtures/frames/recursive.html index b95893e3e..271cb0387 100644 --- a/src/tests/fixtures/frames/recursive.html +++ b/src/tests/fixtures/frames/recursive.html @@ -2,8 +2,9 @@ - Frame + Frames: Recursive + diff --git a/src/tests/fixtures/frames/self.html b/src/tests/fixtures/frames/self.html index 64ed8ee15..12f9a8a84 100644 --- a/src/tests/fixtures/frames/self.html +++ b/src/tests/fixtures/frames/self.html @@ -4,6 +4,7 @@ Frames: Self + diff --git a/src/tests/fixtures/frames/unvisitable.html b/src/tests/fixtures/frames/unvisitable.html index 3606f1595..2e67453d4 100644 --- a/src/tests/fixtures/frames/unvisitable.html +++ b/src/tests/fixtures/frames/unvisitable.html @@ -1,9 +1,10 @@ - + Frame + diff --git a/src/tests/fixtures/frames/without_layout.html b/src/tests/fixtures/frames/without_layout.html deleted file mode 100644 index 41c6ebf62..000000000 --- a/src/tests/fixtures/frames/without_layout.html +++ /dev/null @@ -1,5 +0,0 @@ - -

Frames: Without Layout

- -
Permanent element
- diff --git a/src/tests/fixtures/rendering.html b/src/tests/fixtures/rendering.html index 710c2c4ee..c427e5104 100644 --- a/src/tests/fixtures/rendering.html +++ b/src/tests/fixtures/rendering.html @@ -43,7 +43,6 @@

Rendering

Visit control: reload

Permanent element

Permanent element in frame

-

Permanent element in frame without layout

Delayed link

Redirect link

diff --git a/src/tests/fixtures/tabs.html b/src/tests/fixtures/tabs.html index 702cea0a1..83637f2d4 100644 --- a/src/tests/fixtures/tabs.html +++ b/src/tests/fixtures/tabs.html @@ -3,7 +3,7 @@ Tabs - + diff --git a/src/tests/fixtures/tabs/three.html b/src/tests/fixtures/tabs/three.html index cc7804ef8..517d0dc2b 100644 --- a/src/tests/fixtures/tabs/three.html +++ b/src/tests/fixtures/tabs/three.html @@ -1,9 +1,20 @@ - -
- Tab 1 - Tab 2 - Tab 3 -
+ + + + + Frame + + + + + +
+ Tab 1 + Tab 2 + Tab 3 +
-
Three
- +
Three
+ + + diff --git a/src/tests/fixtures/tabs/two.html b/src/tests/fixtures/tabs/two.html index 80d46f66c..3f31b87f2 100644 --- a/src/tests/fixtures/tabs/two.html +++ b/src/tests/fixtures/tabs/two.html @@ -1,9 +1,20 @@ - -
- Tab 1 - Tab 2 - Tab 3 -
+ + + + + Frame + + + + + +
+ Tab 1 + Tab 2 + Tab 3 +
-
Two
- +
Two
+ + + diff --git a/src/tests/functional/rendering_tests.js b/src/tests/functional/rendering_tests.js index fc85d0e1b..5b4211a80 100644 --- a/src/tests/functional/rendering_tests.js +++ b/src/tests/functional/rendering_tests.js @@ -433,15 +433,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()