olivernn / davis.js

RESTful degradable JavaScript routing using pushState
http://davisjs.com
532 stars 58 forks source link

Chrome 21 back button #48

Closed iainxt closed 12 years ago

iainxt commented 12 years ago

I'm just looking into a bug where if I go from a davis.js enabled page, to a normal page it correctly does a full page load, then hit back, the url changes and popstate is fired, but nothing happens. The 'normal' page does not have davis.js included, so I find it odd that it's trying to use it. Any ideas?

iainxt commented 12 years ago

It appears that pusstate is being called before the redirect to the 'normal' page, perhaps that is why. I am using version 0.8.1, do you know if upgrading will fix the issue?

iainxt commented 12 years ago

I have adjusted the code so that it only calls history.pushState if there is a registered route, and in Chrome its working correctly now. Time to test the other browsers ....

ghost commented 12 years ago

I think I've seen this if you invoke the 404 routing handler, interested to see what your changes were. Keep us updated!

olivernn commented 12 years ago

Its probably worth using the latest version of Davis which is 0.9.2. Let me know if you are still having issues with that version, thanks.

iainxt commented 12 years ago

I upgraded and same issue, and same fix.

iainxt commented 12 years ago

Just to make it clear, what is happening is:

  1. User clicks on a link that is within the website (same origin) but has no route and does not use html5
  2. davis.js adds history entry via pushState
  3. davis.js finds no route and executes delegateToServer
  4. Browser loads page correctly
  5. User hits back button
  6. Browser changes URL, fires popstate event, but does not actually navigate to the page, the page content remains unchanged.

The fix is to only add the history record if a route is found.

olivernn commented 12 years ago

Ok I understand now. Only adding a history record if a route is found is actually quite a large change to the library I think. I'm currently thinking about how the library is structured and will take this into account when redesigning the internals.

For now a work around, or perhaps a better way to use Davis, is to be explicit about the links and forms that you want to bind to.

When defining your app you can give a selector for the links you want to bind to like this:

Davis(function () {
    this.settings.linkSelector = 'a.davis'
    this.settings.formSelector = 'form.davis'
})

You can use whatever selector makes sense for your app.

olivernn commented 12 years ago

Closing this for now, since there is a workaround. I'll definitely take this into account when refactoring the internals of the lib

editstudio commented 11 years ago

@iainxt:

I am having what I think is a similiar issue. Could you please explain what you did when you "adjusted the code so that it only calls history.pushState"?

I specified the link and form selector values, however, no change.

Thanks!

iainxt commented 11 years ago

This is testing my memory, but I think I adjusted the following method

Added addOnDemand instead of calling directly

function assign(request) {
    var addOnDemand = function () {
        history.pushState(wrapStateData(request.toJSON()), request.title, request.location());
    };
    request.addOnDemand = addOnDemand;
    Davis.utils.forEach(pushStateHandlers, function (handler) {
        handler(request);
    });
};

... later on around lines 1578

var handleRequest = function (request) {
        if (beforeFiltersPass(request)) {
            self.trigger('lookupRoute', request)
            var route = self.lookupRoute(request.method, request.path);
            if (route) {
                if (request.addOnDemand) request.addOnDemand();
                self.trigger('runRoute', request, route);

                try {
                    route.run(request)
                    self.trigger('routeComplete', request, route)
                } catch (error) {
                    self.trigger('routeError', request, route, error)
                }

                Davis.utils.every(
        self.lookupAfterFilter(request.method, request.path),
        runFilterWith(request)
      );
editstudio commented 11 years ago

@iainxt Great, thanks a lot! Will give a try.