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

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,6 +302,16 @@ 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.nextBeat
+
+    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")
+    this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.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")
@@ -313,6 +323,7 @@ export class FrameTests extends TurboDriveTestCase {
     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")
+    this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test navigating turbo-frame from within with a[data-turbo-action=advance] pushes URL state"() {
@@ -325,6 +336,7 @@ export class FrameTests extends TurboDriveTestCase {
     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")
+    this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test navigating frame with a[data-turbo-action=advance] pushes URL state"() {
@@ -337,6 +349,7 @@ export class FrameTests extends TurboDriveTestCase {
     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")
+    this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test navigating frame with form[method=get][data-turbo-action=advance] pushes URL state"() {
@@ -349,6 +362,7 @@ export class FrameTests extends TurboDriveTestCase {
     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")
+    this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test navigating frame with form[method=post][data-turbo-action=advance] pushes URL state"() {
@@ -361,6 +375,7 @@ export class FrameTests extends TurboDriveTestCase {
     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")
+    this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test navigating frame with button[data-turbo-action=advance] pushes URL state"() {
@@ -373,38 +388,41 @@ export class FrameTests extends TurboDriveTestCase {
     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")
+    this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test navigating back after pushing URL state from a turbo-frame[data-turbo-action=advance] restores the frames previous 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")
 
     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")
 
     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")
+    this.assert.equal(await this.getFrameSrc("frame"), "/src/tests/fixtures/frames/frame.html")
   }
 
   async "test turbo:before-fetch-request fires on the frame element"() {
@@ -417,6 +435,12 @@ export class FrameTests extends TurboDriveTestCase {
     this.assert.ok(await this.nextEventOnTarget("frame", "turbo:before-fetch-response"))
   }
 
+  async getFrameSrc(id: string): Promise<string> {
+    const src = await this.propertyForSelector("#" + id, "src")
+
+    return new URL(src).pathname
+  }
+
   get frameScriptEvaluationCount(): Promise<number | undefined> {
     return this.evaluate(() => window.frameScriptEvaluationCount)
   }