Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GET forms targeting turbo farmes do not update the FrameElement#src on 7.1.0-rc.1 #446

Closed
agrobbin opened this issue Nov 15, 2021 · 6 comments · Fixed by #449
Closed

GET forms targeting turbo farmes do not update the FrameElement#src on 7.1.0-rc.1 #446

agrobbin opened this issue Nov 15, 2021 · 6 comments · Fixed by #449

Comments

@agrobbin
Copy link
Contributor

Big fan of Turbo! I was in particular excited for 7.1.0-rc.1, specifically #398, so I tried it out with our app, and it appears there might be a small regression.

We have a <form method="get" data-turbo-frame="foo" action="/search">, and when that form is submitted, it's no longer updating the src attribute of our <turbo-frame id="foo">. The same code was working as expected on 7.0.1.

@dhh
Copy link
Member

dhh commented Nov 15, 2021

cc @seanpdoyle

@seanpdoyle
Copy link
Contributor

Thank you for reporting this, @agrobbin.

You mention the new support for [data-turbo-action] on Frames, but the pseudo-code HTML doesn't include that attribute.

Does the regression occur on all <form method="get" data-turbo-frame="..."> submissions, or only ones that declare [data-turbo-action] on the <turbo-frame> or <form> element?

@agrobbin
Copy link
Contributor Author

My pleasure @seanpdoyle! It happens regardless of the new [data-turbo-action] attribute.

@seanpdoyle
Copy link
Contributor

Thank you for clarifying. My hunch is that it's related to the changes in #424. I'll investigate.

@agrobbin
Copy link
Contributor Author

No problem! Let me know if I can be helpful in any other way to help nail down this issue!

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Nov 16, 2021
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]: hotwired#446
[424]: hotwired#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
seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Nov 16, 2021
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]: hotwired#446
[424]: hotwired#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
@seanpdoyle
Copy link
Contributor

I've opened #449 to address this issue.

seanpdoyle added a commit to seanpdoyle/turbo that referenced this issue Nov 16, 2021
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]: hotwired#446
[424]: hotwired#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
@dhh dhh closed this as completed in #449 Nov 19, 2021
dhh pushed a commit that referenced this issue Nov 19, 2021
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

3 participants