defunkt / jquery-pjax

pushState + ajax = pjax
https://pjax.herokuapp.com
MIT License
16.74k stars 1.97k forks source link

faulty check for initialPop #516

Open bbken opened 9 years ago

bbken commented 9 years ago

When run in Safari 7.0.5, OS X 10.9.4, initialPop is set to false by the following code block.

// Non-webkit browsers don't fire an initial popstate event
if ('state' in window.history) {
  initialPop = false
}

However, https://developer.mozilla.org/en-US/docs/Web/Events/popstate states that an initial popstate event does indeed fire in Safari: "Browsers tend to handle the popstate event differently on page load. Chrome (prior to v34) and Safari always emit a popstate event on page load, but Firefox doesn't." I have confirmed this behavior in my own testing.

Particularly, Safari will sometimes fire the popstate event as the initially loaded JS is executing (the exact timing of the popstate in Safari seems non-deterministic, and based on factors that I have not yet come to understand). Since pjax is unaware that my version of Safari on my version of OS X, performs an initial pop, a pjax request that fires as soon as the page loads will incorrectly be cancelled.

mislav commented 9 years ago

Thanks for reporting. Our initial popstate detection might not be perfect and out automated test for this is even forse (intermittently fails).

Also, if an inital popsate happens before jquery-pjax.js has even loaded, there's no chance that we can even detect it.

Why are you firing a pjax request on page load?

opensrcken commented 9 years ago

In the particular case where this issue was detect, we are firing a pjax request on page load to load the heaviest parts of the page asynchronously. This allows users to see the lighter-weight "frame" of the page quickly, enabling a better user experience.

zephiransas commented 9 years ago

I also have a same case.

So I comment-out like this.

// Non-webkit browsers don't fire an initial popstate event
//if ('state' in window.history) {
//  initialPop = false
//}

This fix looks good on Safari and Chrome. But I can't decide is this correct... FYI.

wbinnssmith commented 9 years ago

I'm also having this exact issue, and this patch resolves it. Any chance it will be merged? Would be much appreciated!