Skip to content
This repository has been archived by the owner on Sep 25, 2021. It is now read-only.

Rails: unexpected behaviour in Chrome and Firefox when refreshing invalid form page #229

Open
AipackGit opened this issue Jan 19, 2017 · 12 comments · May be fixed by #495
Open

Rails: unexpected behaviour in Chrome and Firefox when refreshing invalid form page #229

AipackGit opened this issue Jan 19, 2017 · 12 comments · May be fixed by #495
Labels

Comments

@AipackGit
Copy link

Original posted on SO:

"When a user refreshes the page after submitting an invalid form, the browser should attempt to resubmit the form. In Chrome and Firefox (but not Safari) the browser performs a GET request on the create/update url instead.

The browser should resubmit the form (usually asking "Page reare you sure?" or similar), and render the same form-with-errors page. In Chrome and Firefox (but not Safari), on page refresh the browser sends a GET request to the POST url, effectively calling index.

f the url is not RESTful, or if you just happen to have no index or show views, reloading causes the app to throw a "No route" error."

This will lead a bad experience for user who try to fix their input on the form by pressing refresh button.

Such behavior only happen on Turbolink 5 , when the application using TL 2.5.3 it behave as expected.

@vitobotta
Copy link

I'm having the same problem. Have read https://github.com/turbolinks/turbolinks#redirecting-after-a-form-submission but not sure what to do.

@victorlcampos
Copy link

Same problem here

@jeremy
Copy link
Member

jeremy commented Feb 10, 2017

Looks like a dupe of #60, which should be reopened.

@notentered
Copy link

Just commented on #60 :)
... I was digging a bit in that today and maybe (just maybe) I have some directions in my head. Anyway, I still don't get turbolinks enough to be able to fix this. Any help will be appreciated :)

@focused
Copy link

focused commented Apr 20, 2017

similar to #251 with test application to reproduce

@1Mark
Copy link

1Mark commented Dec 29, 2017

Is there any progress with this?

@domchristie
Copy link
Collaborator

Closing as duplicate of #60. I have added some more detail explaining the complexity in #60 (comment)

@chase439
Copy link

chase439 commented Sep 19, 2019

@domchristie what you commented is for XHR (ajax). The issue still remains for non XHR. And I don't think this is a browser issue.

For non XHR, when you submit a standard rails form to the "create" action and it fails validation, it changes the URL to that action (e.g. /articles (with method: POST)) as expected. Refreshing the page should call POST on /articles to resubmit the form instead of calling GET on /articles which goes to the index.

@domchristie
Copy link
Collaborator

@chase439 thanks for the clarification.

This is caused by the call to history.replaceState when Turbolinks initialises, which I think primes the History API location with an initial restoration ID:

startHistory() {
this.location = Location.currentLocation
this.restorationIdentifier = uuid()
this.history.start()
this.history.replace(this.location, this.restorationIdentifier)

To demonstrate that this is the cause, remove Turbolinks, run the following on every page load, then try and resubmit a form as before.

history.replaceState({}, "", location.toString())

Given the differences between Safari and Chrome/Firefox, I wonder what the "correct" behaviour should be? It seems like calling replaceState causes Chrome and Firefox to replace the state and forget its original request type 😕

I'm not sure if there is a solution to this, but reopening 😞

@domchristie domchristie reopened this Sep 19, 2019
@chase439
Copy link

chase439 commented Sep 27, 2019

//Replace current location from the history via history API
window.history.replaceState({}, 'foo', '/foo');

This doesn't retain the method type (e.g. POST). We need a different function that retains the method type or rethink the approach completely.

Do we need to "prime the History API location with an initial restoration ID"? Should we let the browser primes the History API for us instead?

@inopinatus
Copy link

This doesn't retain the method type (e.g. POST). We need a different function that retains the method type.

There isn't one. The history interface defines the update steps for history entries to specifically require that, on replacement, "it represents a GET request, if it currently represents a non-GET request (e.g. it was the result of a POST submission)".

No doubt a safety and security measure.

The only standards-based way to retain a POST in the session history is to not disturb an existing entry.

@domchristie
Copy link
Collaborator

@inopinatus thanks for this. So whilst Safari behaves as we want, it is actually incorrect according to the spec :/

Do we need to "prime the History API location with an initial restoration ID"? Should we let the browser primes the History API for us instead?

When pushState is used, the browser is relieved of its responsibility of handling back/forward transitions. Even though the initial page was not loaded via Turbolinks, subsequent pages will be, and therefore the browser will expect something else (e.g. Turbolinks) to handle the transition between the second and first pages. So unfortunately it's not as simple as removing the initial replaceState and letting the browser handle it :(

Priming the History API location with replaceState means that there is a consistent way to handle popstate events: get the restoration identifier from the popstate event state then pass it to the delegate:

onPopState = (event: PopStateEvent) => {
if (this.shouldHandlePopState()) {
const { turbolinks } = event.state
if (turbolinks) {
const location = Location.currentLocation
const { restorationIdentifier } = turbolinks
this.delegate.historyPoppedToLocationWithRestorationIdentifier(location, restorationIdentifier)
}
}
}
But to fix this issue, we could store the initial location and restoration identifier as a property on the History instance and utilise the null event.state when navigating back to an initial page. The revised popstate handler might look something like:

onPopState = (event: PopStateEvent) => {
  if (this.shouldHandlePopState()) {
    const location = Location.currentLocation
    
    const restorationIdentifier = event.state ?
      this.restorationIdentifierFromState(event.state) :
      this.initialRestorationIdentifier()
    
    if (restorationIdentifier) {
      this.delegate.historyPoppedToLocationWithRestorationIdentifier(location, restorationIdentifier)
    }
  }
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants