Frames: handle GET form submissions (#449)

* Frames: handle `GET` form submissions

Closes [446][].

When handling a `<form method="get" action="..."
data-turbo-frame="...">` submission targeting a `<turbo-frame>` element,
responses that don't redirect do not change the element's `[src]`
attribute.

Following the changes made in [424][], `<form>` submissions that result
in `GET` requests are handled the same as other form submissions in that
they fire `turbo:submit-start` and `turbo:submit-end` events. What
[424][] did not account for is navigations initiated by `<form
method="get">` submissions that _do not_ redirect.

In response to those requests, this commit adds two additional criteria
for updating the `<turbo-frame src="...">` attribute:

* the response's status code is [200 OK][], which is idempotent and does
  not indicate a server-side state change (for example, like a [201
  Created][] with a `Location:` header might)
* the response's `Content-Type:` is HTML (not JSON or even Turbo Stream)

Under those circumstances, the frame's `[src]` is updated.

This commit adds test coverage for this new behavior along with
additional coverage to guard against regressions.

[446]: #446
[424]: #424
[200 OK]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/200
[201 Created]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/201

* Fix flaky Progress Bar test

While testing the progress bar rendering, navigate the page to the
`/__turbo/delayed_response` to sleep for 1 second to give the progress
bar a chance to render. Similarly, wait on the `turbo:load` event to
ensure that the progress bar is hidden.

src/core/frames/frame_controller.ts

@@ -99,7 +99,7 @@ export class FrameController implements AppearanceObserverDelegate, FetchRequest
   }
 
   async loadResponse(fetchResponse: FetchResponse) {
-    if (fetchResponse.redirected) {
+    if (fetchResponse.redirected || (fetchResponse.succeeded && fetchResponse.isHTML)) {
       this.sourceURL = fetchResponse.response.url
     }
 

src/tests/fixtures/frames.html

@@ -19,6 +19,9 @@ <h1>Frames</h1>
     <turbo-frame id="frame" data-loaded-from="/src/tests/fixtures/frames.html">
       <h2>Frames: #frame</h2>
 
+      <form action="/src/tests/fixtures/frames/frame.html">
+        <button id="frame-form-get-no-redirect">Navigate #frame without a redirect</button>
+      </form>
       <button id="add-turbo-action-to-frame" type="button">Add [data-turbo-action="advance"] to frame</button>
       <a id="link-frame" href="/src/tests/fixtures/frames/frame.html">Navigate #frame from within</a>
       <a id="link-nested-frame-action-advance" href="/src/tests/fixtures/frames/frame.html" data-turbo-action="advance">Navigate #frame from within with a[data-turbo-action="advance"]</a>

src/tests/functional/form_submission_tests.ts

@@ -359,6 +359,9 @@ export class FormSubmissionTests extends TurboDriveTestCase {
 
     const otherEvents = await this.eventLogChannel.read()
     this.assert.equal(otherEvents.length, 0, "no more events")
+
+    const src = await (await this.querySelector("#frame")).getAttribute("src") || ""
+    this.assert.equal((new URL(src)).pathname, "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test frame POST form targetting frame toggles submitter's [disabled] attribute"() {
@@ -387,6 +390,9 @@ export class FormSubmissionTests extends TurboDriveTestCase {
 
     const otherEvents = await this.eventLogChannel.read()
     this.assert.equal(otherEvents.length, 0, "no more events")
+
+    const src = await (await this.querySelector("#frame")).getAttribute("src") || ""
+    this.assert.equal((new URL(src)).pathname, "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test frame GET form targetting frame toggles submitter's [disabled] attribute"() {
@@ -529,6 +535,7 @@ export class FormSubmissionTests extends TurboDriveTestCase {
     this.assert.ok(await this.hasSelector("#frame form.redirect"))
     this.assert.equal(await message.getVisibleText(), "Hello!")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/form.html")
+    this.assert.notOk(await this.propertyForSelector("#frame", "src"), "does not change frame's src")
   }
 
   async "test frame form submission with HTTP verb other than GET or POST"() {

src/tests/functional/frame_tests.ts

@@ -302,14 +302,32 @@ export class FrameTests extends TurboDriveTestCase {
     this.assert.notOk(await this.nextAttributeMutationNamed("html", "aria-busy"), "removes aria-busy from the <html>")
   }
 
+  async "test navigating a frame with a form[method=get] that does not redirect still updates the [src]"() {
+    await this.clickSelector("#frame-form-get-no-redirect")
+    await this.nextEventNamed("turbo:before-fetch-request")
+    await this.nextEventNamed("turbo:before-fetch-response")
+    await this.nextEventOnTarget("frame", "turbo:frame-render")
+    await this.nextEventOnTarget("frame", "turbo:frame-load")
+    await this.noNextEventNamed("turbo:before-fetch-request")
+
+    const src = await this.attributeForSelector("#frame", "src") ?? ""
+
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame.html"), "updates src attribute")
+    this.assert.equal(await (await this.querySelector("h1")).getVisibleText(), "Frames")
+    this.assert.equal(await (await this.querySelector("#frame h2")).getVisibleText(), "Frame: Loaded")
+    this.assert.equal(await this.pathname, "/src/tests/fixtures/frames.html")
+  }
+
   async "test navigating turbo-frame[data-turbo-action=advance] from within pushes URL state"() {
     await this.clickSelector("#add-turbo-action-to-frame")
     await this.clickSelector("#link-frame")
     await this.nextBeat
 
     const title = await this.querySelector("h1")
     const frameTitle = await this.querySelector("#frame h2")
+    const src = await this.attributeForSelector("#frame", "src") ?? ""
 
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame.html"), "updates src attribute")
     this.assert.equal(await title.getVisibleText(), "Frames")
     this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
@@ -321,7 +339,9 @@ export class FrameTests extends TurboDriveTestCase {
 
     const title = await this.querySelector("h1")
     const frameTitle = await this.querySelector("#frame h2")
+    const src = await this.attributeForSelector("#frame", "src") ?? ""
 
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame.html"), "updates src attribute")
     this.assert.equal(await title.getVisibleText(), "Frames")
     this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
@@ -333,7 +353,9 @@ export class FrameTests extends TurboDriveTestCase {
 
     const title = await this.querySelector("h1")
     const frameTitle = await this.querySelector("#frame h2")
+    const src = await this.attributeForSelector("#frame", "src") ?? ""
 
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame.html"), "updates src attribute")
     this.assert.equal(await title.getVisibleText(), "Frames")
     this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
@@ -345,7 +367,9 @@ export class FrameTests extends TurboDriveTestCase {
 
     const title = await this.querySelector("h1")
     const frameTitle = await this.querySelector("#frame h2")
+    const src = await this.attributeForSelector("#frame", "src") ?? ""
 
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame.html"), "updates src attribute")
     this.assert.equal(await title.getVisibleText(), "Frames")
     this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
@@ -357,7 +381,9 @@ export class FrameTests extends TurboDriveTestCase {
 
     const title = await this.querySelector("h1")
     const frameTitle = await this.querySelector("#frame h2")
+    const src = await this.attributeForSelector("#frame", "src") ?? ""
 
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame.html"), "updates src attribute")
     this.assert.equal(await title.getVisibleText(), "Frames")
     this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
@@ -369,7 +395,9 @@ export class FrameTests extends TurboDriveTestCase {
 
     const title = await this.querySelector("h1")
     const frameTitle = await this.querySelector("#frame h2")
+    const src = await this.attributeForSelector("#frame", "src") ?? ""
 
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame.html"), "updates src attribute")
     this.assert.equal(await title.getVisibleText(), "Frames")
     this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")
@@ -380,28 +408,31 @@ export class FrameTests extends TurboDriveTestCase {
     await this.clickSelector("#link-frame")
     await this.nextEventNamed("turbo:load")
     await this.goBack()
-    await this.nextBody
+    await this.nextEventNamed("turbo:load")
 
     const title = await this.querySelector("h1")
     const frameTitle = await this.querySelector("#frame h2")
 
     this.assert.equal(await title.getVisibleText(), "Frames")
     this.assert.equal(await frameTitle.getVisibleText(), "Frames: #frame")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames.html")
+    this.assert.equal(await this.propertyForSelector("#frame", "src"), null)
   }
 
   async "test navigating back then forward after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames next contents"() {
     await this.clickSelector("#add-turbo-action-to-frame")
     await this.clickSelector("#link-frame")
     await this.nextEventNamed("turbo:load")
     await this.goBack()
-    await this.nextBody
+    await this.nextEventNamed("turbo:load")
     await this.goForward()
-    await this.nextBody
+    await this.nextEventNamed("turbo:load")
 
     const title = await this.querySelector("h1")
     const frameTitle = await this.querySelector("#frame h2")
+    const src = await this.attributeForSelector("#frame", "src") ?? ""
 
+    this.assert.ok(src.includes("/src/tests/fixtures/frames/frame.html"), "updates src attribute")
     this.assert.equal(await title.getVisibleText(), "Frames")
     this.assert.equal(await frameTitle.getVisibleText(), "Frame: Loaded")
     this.assert.equal(await this.pathname, "/src/tests/fixtures/frames/frame.html")

src/tests/functional/navigation_tests.ts

@@ -7,12 +7,12 @@ export class NavigationTests extends TurboDriveTestCase {
 
   async "test navigating renders a progress bar"() {
     await this.remote.execute(() => window.Turbo.setProgressBarDelay(0))
-    await this.clickSelector("#same-origin-unannotated-link")
+    await this.clickSelector("#delayed-link")
 
     await this.waitUntilSelector(".turbo-progress-bar")
     this.assert.ok(await this.hasSelector(".turbo-progress-bar"), "displays progress bar")
 
-    await this.nextBody
+    await this.nextEventNamed("turbo:load")
     await this.waitUntilNoSelector(".turbo-progress-bar")
 
     this.assert.notOk(await this.hasSelector(".turbo-progress-bar"), "hides progress bar")