[fluxible-router] Safari popstate issue causing a handleHistory.js to execute the navigateAction on page load.

[skippykawakami]

Safari fires popstate on page load. Ordinarily this shouldn't cause an issue in fluxible-router, but because of how we're doing some ATS things, it ends up executing the navigateAction. Basically in our prod environment, url and currentUrl won't match inside _onHistoryChange, which causes the navigateAction to execute. handleHistory.js should guard against this.

For reference on the Safari issue:

• http://stackoverflow.com/questions/28441025/ignore-safaris-initial-page-load-popstate
• http://stackoverflow.com/questions/15896434/window-onpopstate-on-page-load
• Popstate fired on initial page load causes double trigger (Safari 8) visionmedia/page.js#213

[lingyan]

@skippykawakami Thanks for reporting. Let me see what's the best way to work around this. We could potentially do something similar as visionmedia/page.js#213. I'll try it it out in safari 8 first.

[skippykawakami]

@lingyan Here's an alternate solution that avoids having to set a timeout. I'm sure I don't know all the scary edge-cases fluxible-router has to handle, so this approach may be a bad idea, but it seems to prevent this issue when I use it in the project I'm working on.

skippykawakami@e553f87

[lingyan]

@skippykawakami That's a good approach. The only thing is to test out this use case: load page 1 -> click and navigate to page 2 -> browser refresh reload page 2. On the last refresh, check this window.history.state. It might still have fluxible: true in there.

The other approach I was gonna try is to extract the url === currentUrl into a customizable function so you can customize.

Also could you elaborate a little on the "ATS thing" you mentioned? Is your ATS remapping the url to a different path, therefore url is not matching currentUrl?

[skippykawakami]

@lingyan Yeah, the ATS mapping we have for our bucketing and dogfooding appends a query param to the url it's requesting for some reason (I believe it's a well thought-out reason, but I'm not sure what it is).

I'll check your use case and see what I find. If that really is the case, it might be better if I check for the fluxible=true in the popState event object's state property rather than the history property (actually maybe that's better overall... Surely there's nothing in e.state for a back button that no one actually pressed)

[skippykawakami]

@lingyan You were right about reloading being an issue. Checking against the event object instead of window.history seems to fix it. Here's a PR if you want it: #351

[lingyan]

@skippykawakami Thanks. Saw your PR. I will test it out a bit. In the meanwhile, you can consider letting ATS pass header field to your node server if possible, instead of using query param. That will work around this issue while we test out your PR.

[skippykawakami]

@lingyan I'll look into that.

[lingyan]

Closed via #394