browserstate / history.js

History.js gracefully supports the HTML5 History/State APIs (pushState, replaceState, onPopState) in all browsers. Including continued support for data, titles, replaceState. Supports jQuery, MooTools and Prototype. For HTML5 browsers this means that you can modify the URL directly, without needing to use hashes anymore. For HTML4 browsers it will revert back to using the old onhashchange functionality.
http://browserstate.github.com/history.js/demo/
Other
10.75k stars 1.35k forks source link

History.pushState Triggers PopState Event #312

Open tgrant54 opened 11 years ago

tgrant54 commented 11 years ago

Every time History.pushState is called, the window.onpopstate event is triggered.

I tried binding to the statechange event using History.Adapter, and I also tried without using it.

So, if I have a ajax call to load a file after pushState, and another ajax call when the popstate is fired, this results in 2 ajax requests.

sprintstar commented 11 years ago

Getting this too. This is a fairly major fail and renders this library useless.

patrickliechty commented 11 years ago

What version are you guys using? It may be fixed in 1.8.0b2

leliel commented 11 years ago

I'm having the same issue here this appears to be persistent in 1.8.0b2

EDIT: turns out I had an event loop in my code not a problem with History.js for me.

benauthor commented 11 years ago

Seems someone decided to do this on purpose but I cannot fathom why one would want popstate on pushstate(). I would call it a bug, not a feature.

Snede commented 11 years ago

Did you guys find a solution to this problem? As Sprintstar said, it renders it completely useless. Well, half useless, it works for me, but each page gets called by Ajax twice because of it...

leliel commented 11 years ago

I've worked around this for my code. however, as @benauthor states, it's quite clearly deliberate (look at the code in the link).

we're still waiting for some feedback on why this was done, but you can always pull a local copy and see what happens with the call to popstate commented out.

I have a feeling this is a matter of smoothing out cross browser oddities. though it's certainly not the way I'd prefer it.

Snede commented 11 years ago

Yes it's definetely intentional, but I can't get my mind around why you would do that. I'm trying to migrate from a pure HTML5 implementation of window.history, to this. And in the original implementation, the popstate is ONLY called when you actually move between states.

I have tried commenting it out, but it destroys the code. If it's commented out, you - for some reason - have to press the back button multiple times to go one step back.

But how did you get around it in your code? Some global value?( Which I don't like that much)

I worked around it like this:

CompanyName.Mobile.PushHistoryState = function (state, title, url) {
    if (!CompanyName.Mobile.IsStateEqualToCurrentState(state)) {
        History.LastPushedState = state;
        History.pushState(state, title, url);
    }
};

and:

CompanyName.Mobile.OnHistoryPopChange = function (event) {
    var state = History.getState();
    state = state.data;
    if (state === undefined || complexObjectEquals(History.LastPushedState, state) || state.ignore === true)
        return;
   // rest of your popstate code.
}
leliel commented 11 years ago

What I'm doing is a visualisation of log data. this doesn't have complex state to store. the key elements of app state are encoded in the url querystring to allow users to pass views around via url sharing.

so when an event happens that should change state, the new querystring is computed then pushed.

This triggers popState, which handles data loading.

so no global variables, just arranging operations in an order compatible with the way History.js works. I don't like it. it's possible because all the state is independent of the data loaded on statechange.

unfortunately I have no idea why commenting out the popstate call causes that behaviour. I've not dug that deeply into History.js yet.

benauthor commented 11 years ago

@SA-ASH I ended up with a very stupid-simple (but mostly stupid) solution in which I have a flag isActuallyPop that is by default true but I flip to false when I do a History.pushState. Then my event handler checks the flag.

Snede commented 11 years ago

@benauthor It seems our solutions are quite similar, you use a flag, and I check for equality of the states. In essense its the same thing.

Bad implementations, require heartbreaking hacks.

mcjahudka commented 11 years ago

In my opinion the proper way of working with ajax & history.js is doing the ajax call before you push the new history state; you can then save the ajax response in the state's data and apply the response in any way your app needs in the popstate handler. That way your application only needs to perform the ajax request when it is directly invoked (e.g. a link is clicked), but when you navigate back and forward in history, the data is already present in the states' data stores.

I think that's what the page linked by @benauthor says (though I haven't read it), just wanted to add a short explanation to this here discussion :-)

benauthor commented 11 years ago

@mcjahudka my link was to the source, just pointing out that it is an explicit call and not an accidental side-effect, i.e. not exactly a 'bug' in the sense that the effects appear to align with the author's intention.

There are certainly a variety of strategies for working with ajax and the history API, and you outline one that sounds reasonable, provided you are developing with history.js in mind from the get-go.

However the chief complaint here is that a library that claims to be a HTML5 History API polyfill (and is truly the best available option for old-browser support at the moment) diverges from both the spec and the actual-existing browser implementations on a behavior that is fairly fundamental. I for one think that's an odd design decision.

mcjahudka commented 11 years ago

@benauthor aha ok, you've got a point.. Maybe this could be switched with an option upon initialization?

MaddieM4 commented 10 years ago

This is a dealbreaker for my use case, where I need to be able to push states onto the stack without side effects. When can we expect a fix for this?

Snede commented 10 years ago

@campadrenalin I had the same issue as you, but considering how many people have made a fix for this "bug". I don't think it's going to be fixed...

Instead, just work around it, it can work just as well, even though it leaves a bad taste in your mouth...

MaddieM4 commented 10 years ago

@SA-ASH I've worked around it for now, but it ought to be fixed, and now that it's not a pressing emergency I might even have time to fix it myself. We shouldn't have to work around this, and I can't imagine any platform quirk that could possibly justify breaking expected popstate behavior on all platforms, so I don't much care what the original author's reasoning is.

Anyone know where the test documentation is/if it exists at all? It would be nice to be able to run tests locally to avoid regressions, even on a limited set of platforms (at least until the community is able to review the pull request).

Snede commented 10 years ago

@campadrenalin I completely agree with you.

But my code only needs very few changes to adjust for it (a few lines). So I moved on to other things instead. You could fork the project, and fix it :)

benauthor commented 10 years ago

@campadrenalin if you do decide to work on this, this comment from three years ago looks like a clue about what to test for regression.