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

5.x: POST to create causes URL change even if form errors occured #60

Closed
sblackstone opened this issue Mar 29, 2016 · 38 comments · May be fixed by #495
Closed

5.x: POST to create causes URL change even if form errors occured #60

sblackstone opened this issue Mar 29, 2016 · 38 comments · May be fixed by #495

Comments

@sblackstone
Copy link

@sblackstone sblackstone commented Mar 29, 2016

Running Rails 5.0.0beta3 + TurboLinks 5.x

When I post a form for "create", and the create action re-renders the "new" action, turbolinks is updating the URL to the POST url.

If the user then reloads the page, they end up with the index action rather than back at "new"...

@sstephenson

This comment has been minimized.

Copy link
Contributor

@sstephenson sstephenson commented Mar 29, 2016

Turbolinks doesn’t handle form submission. Please see Redirecting After a Form Submission in the documentation.

@sblackstone

This comment has been minimized.

Copy link
Author

@sblackstone sblackstone commented Mar 29, 2016

The redirection part isn't the problem, its when it doesn't redirect but instead does

render action: 'new'

because some error occurred..

@sblackstone

This comment has been minimized.

Copy link
Author

@sblackstone sblackstone commented Mar 29, 2016

Just to be clear,

Suppose I'm at a page /users/new

I fill out the form but omit a required field, and then click submit.

The url changes to /users but rails will render action:new

If you were to then refresh the page, you will then end up with the index action rather than the new action.

Similarly, if you are editing a user at /users/1/edit and click save, the URL would update to /users/1 and a refresh would get you the show action instead of the edit action.

@packagethief

This comment has been minimized.

Copy link
Member

@packagethief packagethief commented Mar 30, 2016

I don't think this is a Turbolinks problem. You should see the same behavior in an app that doesn't use Turbolinks.

@sblackstone

This comment has been minimized.

Copy link
Author

@sblackstone sblackstone commented Mar 30, 2016

Your right - I think I might have misunderstood my issue..

Thanks,
Stephen

@vishaltelangre

This comment has been minimized.

Copy link

@vishaltelangre vishaltelangre commented Jun 23, 2016

Same issue. URL change is intended behavior, but refresh with GET is not.

When turbolinks is not required in application.js; after refreshing a form with errors, a confirmation dialog appears which on confirming resends the form data to the server on the form action url again.

monosnap 2016-06-23 23-14-53

Whereas, when turbolinks is required, it refreshes the page (to the index action) without displaying a confirmation dialog.

This is not an intended behavior (as it seems like overriding the default Rails behavior in such a case).

@lserman

This comment has been minimized.

Copy link

@lserman lserman commented Sep 7, 2016

@vishaltelangre seeing the same behavior and I do believe it is a bug. After the server renders errors resulting from a POST, browsers typically re-submit the POST on refresh. With turbolinks, the browser does a GET which in my case goes to a 404 (we don't have an index page for this resource). Is this intended behavior? If not, can we re-open this issue?

@wangrui438

This comment has been minimized.

Copy link

@wangrui438 wangrui438 commented Sep 28, 2016

@vishaltelangre @lserman I have the same problem. I don't know whether this is an intended behavior.

@youngbrioche

This comment has been minimized.

Copy link

@youngbrioche youngbrioche commented Jan 6, 2017

Same here.

If I remove Turbolinks 5.0.1 from my Rails 5.0.0.1 app, everything works as expected (Hitting F5 triggers a browser warning before resubmitting the form via POST).

If Turbolinks is active (while not actually being used on form submits), this default behaviour breaks. When hitting F5, the browser issues a GET to the current URL.

+1 for reopening this.

@victorlcampos

This comment has been minimized.

Copy link

@victorlcampos victorlcampos commented Feb 10, 2017

+1 for reopening this.

@notentered

This comment has been minimized.

Copy link

@notentered notentered commented Mar 14, 2017

Another +1 here

@notentered

This comment has been minimized.

Copy link

@notentered notentered commented Mar 14, 2017

Not sure if helpful in any way, but what I believe is happening is:

  1. On any request turbolinks push or replace history state. Thats cool :)
  2. Turbolinks obviously cannot receive the request method with JS (https://twitter.com/BrendanEich/status/136607819091816448). Also it is not possible to put post query to the history.
  3. By this reason (and probably not just this) all form submits always acts as turbolinks is disabled. Thats cool too.
  4. Now, the tricky part is that turbolinks push state immediately after page is loaded. So if we have post with failed validations, which just renders "new" or "edit" views, turbolinks actually do pushState with the current location. On refresh most browsers just assume this is GET request and thus we are doomed :)

Well, I'm not really sure that this is the exact situation here. But if it is... can't we skip the first pushState in any way? Or probably better... Rails knows the request.method for every request. Can't turbolinks use this and skip first pushState() for pages rendered after method = "POST"?

@notentered

This comment has been minimized.

Copy link

@notentered notentered commented Mar 14, 2017

And I'll answer myself :)
... probably we can't.

I just commented the first @history.replace in controller.coffee -> startHistory() which seemingly solved the problem. The form was resubmitted when we refresh the page. "Yey!" :)

  startHistory: ->
    @location = Turbolinks.Location.wrap(window.location)
    @restorationIdentifier = Turbolinks.uuid()
    @history.start()
#    @history.replace(@location, @restorationIdentifier)

However two not really desired things happened.

  1. Apparently I messed with turbolinks cache. Probably fixable but:
  2. If I move forward through some link and than hit back again I obtained the same behavior. So, not solution at all... just moving the problem a few clicks away :)

Anyway... I'm out of ideas... will look for some other workaround. Maybe the easiest one is to have fallback route when I'm missing the index. In my case (devise), I'll just redirect

get 'users', to: redirect('/users/sign_in')

Works for me for now. However any better suggestions are highly appreciated :)

@michimawan

This comment has been minimized.

Copy link

@michimawan michimawan commented Mar 31, 2017

+1 for reopening this

@hendyanto

This comment has been minimized.

Copy link

@hendyanto hendyanto commented Mar 31, 2017

+1 for reopening this

Just stumbled open this issue today, I tried to disable turbolinks on that page, still to no avail.

@humphreybc

This comment has been minimized.

Copy link

@humphreybc humphreybc commented Apr 4, 2017

I'm having the same issue.

@elsurudo

This comment has been minimized.

Copy link

@elsurudo elsurudo commented Apr 12, 2017

+1

Experiencing the same issue on Chrome and Firefox. Works fine on Safari. However, on Safari I get #257 Can anyone corroborate?

@Hates

This comment has been minimized.

Copy link

@Hates Hates commented Apr 19, 2017

Same problem here. Any ideas or fixes? Works fine when I remove turbolinks.

@focused

This comment has been minimized.

Copy link

@focused focused commented Apr 20, 2017

It's interesting that it works as expected in Safari only, but does not work in Chrome and FF..

@inopinatus

This comment has been minimized.

Copy link

@inopinatus inopinatus commented Apr 20, 2017

+1 maybe just my bad karma for using a tool that reimplements fundamental browser behaviour in JS :(

@elsurudo

This comment has been minimized.

Copy link

@elsurudo elsurudo commented Apr 21, 2017

Heh, yeah. But it seems to be working fine for a lot of people (me included – in previous projects). I suspect there was a regression somewhere along the line...

@elsurudo

This comment has been minimized.

Copy link

@elsurudo elsurudo commented Apr 21, 2017

This should be reopened!

@victorlcampos

This comment has been minimized.

Copy link

@victorlcampos victorlcampos commented May 5, 2017

Guys, some info about this issue?

@sblackstone

This comment has been minimized.

Copy link
Author

@sblackstone sblackstone commented May 5, 2017

@elsurudo

This comment has been minimized.

Copy link

@elsurudo elsurudo commented May 5, 2017

Some of us are still experiencing it though (or something similar).

@focused

This comment has been minimized.

Copy link

@focused focused commented May 6, 2017

Safari and Chrome gives different results. I think it is unexpected behavior.

@elsurudo

This comment has been minimized.

Copy link

@elsurudo elsurudo commented May 8, 2017

@focused Yes, I have already opened a ticket. Please check #257 and corroborate

@GCorbel

This comment has been minimized.

Copy link

@GCorbel GCorbel commented Aug 15, 2017

I have the same issue. The @notentered 's solution seems to fix a part of the problem.

@benbonnet

This comment has been minimized.

Copy link

@benbonnet benbonnet commented May 23, 2018

@sblackstone not an issue that turbolinks have to figure out, but the occuring behaviour remains an issue. And still seem to happen

Does anyone have found a decent workaround ?

@oxfist

This comment has been minimized.

Copy link

@oxfist oxfist commented Jul 17, 2018

I just ran into this problem and after some Internet searching I only found this issue. It'd be really cool if we could get an answer on where the unintended behavior is so we can find a workaround.

@astyagun

This comment has been minimized.

Copy link

@astyagun astyagun commented Jul 24, 2018

Possible workaround idea: if a form comes back with errors, URL should be reset to the original form URL

Possible implementation idea: a Stimulus component (or something, that triggers JS code) can be added by SimpleForm automatically to forms, that have errors. This component (JS code) would update the page URL. The same SimpleForm element could add a hidden field containing a form original URL to forms.

Don't know if page refresh would preserve current form values, but at least users would end up on the same form page after a page refresh

Another idea: disable Turbolinks for form submissions

Another idea: use only remote forms and handle URL change manually

@cireficc

This comment has been minimized.

Copy link

@cireficc cireficc commented Jul 26, 2018

Bump.

I don't think this will cause an issue for my users in production, but I haven't even gotten to the beta test yet. I think that for complex forms with many fields (my use case, sadly), users accidentally or intentionally hitting refresh after a failed form submission is going to stress them out and make them angry based on the behavior Turbolinks is causing here...

I love Turbolinks, but this is IMO a glaring issue that needs to be fixed because it breaks normal user expectations of browser behavior.

@victorlcampos

This comment has been minimized.

Copy link

@victorlcampos victorlcampos commented Jul 26, 2018

Yeah, this Issue realy sux =(

@victorbueno

This comment has been minimized.

Copy link

@victorbueno victorbueno commented Aug 7, 2018

Still happening =/

@domchristie

This comment has been minimized.

Copy link
Collaborator

@domchristie domchristie commented Sep 21, 2018

Turbolinks doesn't handle form submissions itself. It makes some recommendations on handling redirects after an XHR form submission, but leaves the submission implementation up to you.

In terms of refreshing a JS form submission, unfortunately I don't think the desired behaviour is possible due to the way that browsers work, and the currently available technologies. To help explain why, it might be useful to use a couple of examples.

Let's say we have a social network with an index containing list of posts that can be "Liked". Tapping the "Like" button POSTs an XHR to /likes. If the request were to error and we refresh the page, it reloads index, which is what we'd expect. It doesn't re-request the POST to /likes, or present an alert, warning of a form resubmission. That would seem odd! This suggests that the browser's reload mechanism does not consider XHRs, when deciding how to reload a given page.

Now consider a blogging app where we visit /posts/new and POST a full page form to /posts via XHR. If it errors and we refresh the page we might expect the browser to alert us to a form resubmission, but as our previous example showed, reloading doesn't consider XHRs. It just re-requests the current URL.

The requests are very similar in these cases, it is just the UI which alters expectations. This highlights some of the difficulties when formulating a general solution to this problem.

pushState adds a bit more complexity to the situation. It would make sense to update the URL via pushState in the second case (to /posts) to match how a browser typically operates when responding to full form submissions. However, pushState does not contain any details regarding how a request was made, or of any form parameters sent. It is literally just pushing some state. pushState and XHRs are independent APIs, so it's not as if the browser can recognise an XHR POST followed by a change in the history, and know that it should mimic the behaviour of a traditional form request.

Once you take control of making requests via XHR and updating the address bar via pushState, the browser is like, "I'm done dealing with the history, I'll leave it to you!"


I can't find a simple explanation which defines browser reloading behaviour but here's what I think happens. On reload, the browser asks:

  1. How was this window originally loaded?
  2. What is my current location?

Typically the first answer will be, "via GET", and for the second answer it just looks at its current location in the address bar. This should hopefully explain the behaviour, after a form submission via XHR.


Dealing with forms in Turbolinks applications is a commonly faced problem, and it's something that may be handled in the future, but as the above hopefully demonstrates, it's a complex problem. I recommend checking out some of the solutions in #85 and adapting them to fit your case.

@afcapel

This comment has been minimized.

Copy link

@afcapel afcapel commented Nov 21, 2018

@domchristie the problem I'm seeing is that turbolinks seems to be changing the browser behaviour even in non XHR form submissions. Here is a minimal test case to reproduce the issue:

require "sinatra"

def layout(body)
  <<-LAYOUT
  <!DOCTYPE html>
<html>
  <head>
    <script src="https://cdnjs.cloudflare.com/ajax/libs/turbolinks/5.2.0/turbolinks.js"></script>
  </head>
  <body>
  #{body}
  </body>
</html>
  LAYOUT
end

get "/test" do
  body = <<-GET_RESPONSE
  <h1>I'm a GET REQUEST</h1>

  <form action="/test" method="post">
    <input type="submit" value="Submit me"/>
  </form>
  GET_RESPONSE
  layout(body)
end

post "/test" do
  body = <<-POST_RESPONSE
    <h1>I'm a POST REQUEST</h1>

    <form action="/test" method="post">
      <input type="submit" value="Submit me"/>
    </form>
  POST_RESPONSE
  layout(body)
end

This submits a form without XHR doing a regular POST request. If after submitting the form you reload the page, Chrome and Firefox use a GET http request. Safari does the right thing and shows an alert warning that the form will be resubmitted.

Now, if you remove the turbolinks script and do the same test, Firefox and Safari shows a the warning message. Chrome doesn't show the warning message but still reloads the page with a POST request.

@domchristie

This comment has been minimized.

Copy link
Collaborator

@domchristie domchristie commented Nov 22, 2018

@afcapel thanks for the test case and for highlighting the differences in non-XHR form requests when Turbolinks is installed. Based on this, and some of the comments above, I think we can deduce that this is a browser bug in Chrome/Firefox…

Whilst it seems unrelated, altering the history (via pushState or replaceState) causes problems with the confirm dialogs when refreshing after a POST 🤷‍♂️. Turbolinks doesn't deal with form submissions, so given the absurdity of the bug, I hope you can forgive the confusion around this issue!

We can confirm the bug (pun intended) with the following example, which demonstrates the same behaviour. To clarify, any site which uses the history API before a form POST will be susceptible to this bug.

require "sinatra"

def layout(body)
  <<-LAYOUT
  <!DOCTYPE html>
<html>
  <head>
  <script>window.history.replaceState(null, null, window.location.href)</script>
  </head>
  <body>
  #{body}
  </body>
</html>
  LAYOUT
end

get "/test" do
  body = <<-GET_RESPONSE
  <h1>I'm a GET REQUEST</h1>

  <form action="/test" method="post">
    <input type="submit" value="Submit me"/>
  </form>
  GET_RESPONSE
  layout(body)
end

post "/test" do
  body = <<-POST_RESPONSE
    <h1>I'm a POST REQUEST</h1>

    <form action="/test" method="post">
      <input type="submit" value="Submit me"/>
    </form>
  POST_RESPONSE
  layout(body)
end

As @notentered mentions above, Turbolinks replaces the state immediately after the page is initially loaded to bootstrap the history with some initial state. It might be possible to remove this, but again, you'll eventually change the history at some point via navigation, and therefore hit this bug.


To back this up, this behaviour matches my own experience dealing with failing alert, confirm, and prompt dialogs in Safari iOS following a popstate event (#336). What's more, when experimenting with @afcapel's example with the dev tools open, I discovered that upon refreshing after a POST, Chrome issues another POST but does not display a confirm dialog.

Basically, browsers do weird things (with dialogs) when the history API is used 😫

If possible, I'd follow @notentered's recommendation and provide a fallback route which responds to GET requests to the same path, and redirect as necessary. Obviously this isn't ideal, but I'm not sure if there is a better solution at the moment.

@domchristie

This comment has been minimized.

Copy link
Collaborator

@domchristie domchristie commented Oct 2, 2019

Based on this, and some of the comments above, I think we can deduce that this is a browser bug in Chrome/Firefox…

I take that back! Fix + more details: #495

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.