Respect turbo-visit-control for frame requests
#867
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
turbo-rails omits the contents of the document from How does this strategy interact with servers that omit portions of the page when handling a request with a |
|
Re Turbo Rails, the idea is to stop stripping out the head for frame responses (hotwired/turbo-rails#428). For providing a way for certain pages to render outside the frame when needed (the "redirect to login page" problem described above), there's an alternative approach to this we discussed (#864), which avoids that issue by letting you specify a list of paths on the requesting page (most likely in an application layout). Paths listed there would be permitted ahead of time to break out of a frame if returned in response to a frame request. But, after talking this over with some folks, we felt that |
Turbo normally performs a fill page reload whenever a response contains
the appropriate `turbo-visit-control` meta tag:
<meta name="turbo-visit-control" content="reload">
Such responses are considered "not visitable".
For frame requests, we have previously been ignoring any
`turbo-visit-control` set in the response, and instead treating all
valid frame responses as "visitable".
This commit changes this behaviour so that `turbo-visit-control` will be
treated consistently for both frame and non-frame requests.
As well as being more consistent, this provides a useful escape hatch
for situations where a frame request redirects to something that should
be a full page reload, but which would be prevented due to that content
missing the expected frame. The class example of this is when an expired
session causes a frame request to be redirected to a login page. By
including `turbo-visit-control` on that login page, we can ensure that
it is always rendered as a full page, and never hidden by a failed frame
request.
kevinmcconnell
force-pushed
the
frames/respect-visit-control
branch
from
93e5bb9
to
cb61ece
Compare
|
Clever judo! Let's make sure we document this hatch together with the specific use case (how to deal with expiring sessions). |
|
Will do 👍 I'll work up a doc PR to cover the missing frame behaviour, including this escape hatch. |
This was referenced Feb 7, 2023
seanpdoyle
added a commit
to seanpdoyle/turbo
that referenced
this pull request
Mar 2, 2023
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
added a commit
to seanpdoyle/turbo
that referenced
this pull request
Mar 2, 2023
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
added a commit
to seanpdoyle/turbo
that referenced
this pull request
Mar 2, 2023
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
another-rex
referenced
this pull request
in google/osv.dev
Mar 6, 2023
[](https://renovatebot.com) This PR contains the following updates: | Package | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---| | [@hotwired/turbo](https://turbo.hotwired.dev) ([source](https://togithub.com/hotwired/turbo)) | [`7.2.5` -> `7.3.0`](https://renovatebot.com/diffs/npm/@hotwired%2fturbo/7.2.5/7.3.0) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | | [sass](https://togithub.com/sass/dart-sass) | [`1.58.0` -> `1.58.3`](https://renovatebot.com/diffs/npm/sass/1.58.0/1.58.3) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>hotwired/turbo</summary> ### [`v7.3.0`](https://togithub.com/hotwired/turbo/releases/tag/v7.3.0) [Compare Source](https://togithub.com/hotwired/turbo/compare/v7.2.5...v7.3.0) #### What's Changed - Don't break out of frames when frame missing by [@​kevinmcconnell](https://togithub.com/kevinmcconnell) in [https://github.com/hotwired/turbo/pull/863](https://togithub.com/hotwired/turbo/pull/863) - Respect `turbo-visit-control` for frame requests by [@​kevinmcconnell](https://togithub.com/kevinmcconnell) in [https://github.com/hotwired/turbo/pull/867](https://togithub.com/hotwired/turbo/pull/867) - Allow changing the submitter text during form submission by [@​afcapel](https://togithub.com/afcapel) in [https://github.com/hotwired/turbo/pull/869](https://togithub.com/hotwired/turbo/pull/869) - Form submissions from frames should clear cache by [@​kevinmcconnell](https://togithub.com/kevinmcconnell) in [https://github.com/hotwired/turbo/pull/882](https://togithub.com/hotwired/turbo/pull/882) - Deprecate `[data-turbo-cache=false]` in favor of `[data-turbo-temporary]` by [@​packagethief](https://togithub.com/packagethief) in [https://github.com/hotwired/turbo/pull/871](https://togithub.com/hotwired/turbo/pull/871) - `resume()` does not require `value` by [@​dahlbyk](https://togithub.com/dahlbyk) in [https://github.com/hotwired/turbo/pull/854](https://togithub.com/hotwired/turbo/pull/854) - Rename `isIdempotent` to `isSafe` by [@​kevinmcconnell](https://togithub.com/kevinmcconnell) in [https://github.com/hotwired/turbo/pull/883](https://togithub.com/hotwired/turbo/pull/883) **Full Changelog**: hotwired/turbo@v7.2.5...v7.3.0 </details> <details> <summary>sass/dart-sass</summary> ### [`v1.58.3`](https://togithub.com/sass/dart-sass/blob/HEAD/CHANGELOG.md#​1583) [Compare Source](https://togithub.com/sass/dart-sass/compare/1.58.2...1.58.3) - No user-visible changes. ### [`v1.58.2`](https://togithub.com/sass/dart-sass/blob/HEAD/CHANGELOG.md#​1582) [Compare Source](https://togithub.com/sass/dart-sass/compare/1.58.1...1.58.2) ##### Command Line Interface - Add a timestamp to messages printed in `--watch` mode. - Print better `calc()`-based suggestions for `/`-as-division expression that contain calculation-incompatible constructs like unary minus. ### [`v1.58.1`](https://togithub.com/sass/dart-sass/blob/HEAD/CHANGELOG.md#​1581) [Compare Source](https://togithub.com/sass/dart-sass/compare/1.58.0...1.58.1) - Emit a unitless hue when serializing `hsl()` colors. The `deg` unit is incompatible with IE, and while that officially falls outside our compatibility policy, it's better to lean towards greater compatibility. </details> --- ### Configuration 📅 **Schedule**: Branch creation - "before 6am on wednesday" in timezone Australia/Sydney, Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/google/osv.dev). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4xMjUuMSIsInVwZGF0ZWRJblZlciI6IjM0LjE0Ni4yIn0=-->
|
@kevinmcconnell Can you please help me figure out how to apply this to, for example, break out of a turbo frame if a form is successful, but stay within the frame if not? Previous to #863, a simple In this case, I'm having this issue on a page that displays a login form within a turbo frame, because it goes through multiple steps, and when the final step is successful, I redirect to a new page without the frame. I also can't Please note that I'm not trying to troubleshoot my code here, but rather providing a real-world example to foster discussion. An issue was also raised asking more clarity here about Wondering if the solution is rather finding a way to break out of the frame from the controller when we desire so. This would prevent the missing flash issue mentioned here #897 and would solve the issue on the example I mentioned above. |
|
It has come to my attention that there's already a PR for my previous suggestion! I think that would also fix the example scenario I provided. |
|
how to use this behavior at certain/specific pages? or this settings only could be applied to whole application? I think I need new layout with the redirect works but its not rendering notice |
seanpdoyle
added a commit
to seanpdoyle/turbo
that referenced
this pull request
Jul 21, 2023
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
added a commit
to seanpdoyle/turbo
that referenced
this pull request
Sep 10, 2023
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
shiftyp
pushed a commit
to shiftyp/ts-turbo
that referenced
this pull request
Sep 11, 2023
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
](https://renovatebot.com){kind=link}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Turbo normally performs a fill page reload whenever a response contains the appropriate
turbo-visit-controlmeta tag:Such responses are considered "not visitable".
For frame requests, we have previously been ignoring any
turbo-visit-controlset in the response, and instead treating all valid frame responses as "visitable".This commit changes this behaviour so that
turbo-visit-controlwill be treated consistently for both frame and non-frame requests.As well as being more consistent, this provides a useful escape hatch for situations where a frame request redirects to something that should be a full page reload, but which would be prevented due to that content missing the expected frame. The class example of this is when an expired session causes a frame request to be redirected to a login page. By including
turbo-visit-controlon that login page, we can ensure that it is always rendered as a full page, and never hidden by a failed frame request.