Reload page if failed form changes tracked content #759

kevinmcconnell · 2022-10-11T14:43:00Z

When a form submission fails with a client error, the response from the form replaces the current page content. In Turbo 7.1, whenever this replacement would affect resources marked with data-turbo-track="reload", a reload of the page is triggered instead.

In 7.2.0, we do not perform this reload. Instead, the form's response is discarded. This happens because the content replacement does not occur during a visit, and so we have no new location to reload.

This difference can affect situations like expired sessions, where reloading the page would cause a new session to be established. If we instead allow the response to be discarded, actions can fail silently.

This commit changes the behaviour to reload the current page when reload is called outside of a current visit, in order to match the behaviour of 7.1, and to not discard the response from the form submission.

When a form submission fails with a client error, the response from the form replaces the current page content. In Turbo 7.1.x, whenever this replacement would affect resources marked with `data-turbo-track="reload"`, a reload of the page is triggered instead. In 7.2.0, we do not perform this reload. Instead, the form's response is discarded. This difference can affect situations like expired sessions, where the previous behaviour of reloading the page would cause a new session to be established. If we instead allow the response to be discarded, actions can fail silently. This commit changes the behaviour to reload the current page when `reload` is called outside of a current visit, in order to match the behaviour of 7.1, and to not discard the response from the form submission.

packagethief

Thanks @kevinmcconnell.

It looks like this changed in #601. This updated the browser adapter to depend on its own location (assigned during visit proposal) instead of window.location when reloading the page. The thing is, we won't have a visit if we're rendering in response to a failed form submission. Falling back to window.location seems reasonable to restore the previous behavior.

Ideally we'd use window.location always in the context of reloading and page invalidation. The intent is that it behave exactly like the browser's native "reload". It operates on the current location, not the intended one.

kevinmcconnell · 2022-10-11T17:14:08Z

@packagethief thanks for the review! 🙏

Ideally we'd use window.location always in the context of reloading and page invalidation.

Yes, I totally agree. I think it's worth shipping this small fix first to match the previous behaviour. But we can also look into removing the location from the browser adapter, as it would simplify things a little if it could have a single concept of the current location in all cases.

packagethief · 2022-10-11T19:51:43Z

I think it's worth shipping this small fix first to match the previous behaviour

Definitely.

packagethief approved these changes Oct 11, 2022

View reviewed changes

dhh merged commit 728a561 into hotwired:main Oct 12, 2022

afcapel mentioned this pull request Sep 28, 2023

add failing test about form submission responded with exceptional 422 #1006

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reload page if failed form changes tracked content #759

Reload page if failed form changes tracked content #759

kevinmcconnell commented Oct 11, 2022

packagethief left a comment •

edited

Loading

kevinmcconnell commented Oct 11, 2022

packagethief commented Oct 11, 2022

Reload page if failed form changes tracked content #759

Reload page if failed form changes tracked content #759

Conversation

kevinmcconnell commented Oct 11, 2022

packagethief left a comment • edited Loading

Choose a reason for hiding this comment

kevinmcconnell commented Oct 11, 2022

packagethief commented Oct 11, 2022

packagethief left a comment •

edited

Loading