Just wondering if this has any attention. It is causing me a bit of grief.

@TonyLugg Thanks for your detailed report.

Running gist here: https://gist.run/?id=f4a7fa39c9d82f5de0af3b0bc8e01850

Hello!

Any update on this bug?

I'm going to want to start looking at this for 1.6.0, potentially a March release if all goes well.

The core issue is that router.navigateBack() calls history.back(), which is a purposefully opaque API. When the cancel is processed, it is not forwarded to the history and so the history believes that the navigation back was successful.

I'm not familiar with the implementation details but wouldn't it be logical to call history.back only after candeactivate returns true? It seems weird to me that we navigate back before checking if we are allowed to.

It's not a straightforward switch. history.back() is used to change the url, which kicks off the navigation pipeline including canDeactivate. This strategy makes sense as well because if we just read the previous state and navigated to it, we'd be adding a new entry into the history, not navigating backwards in it.

I've done some experiments calling history.forward when a navigateBack() fails, but it seems that calling the two in rapid succession causes them to fail to function properly. See this gist: https://gist.run/?id=6e306fa6d5c9675e8ea55fe5f495be2f

I've found a number of related bugs. Specifically, if you start at #/first and navigate to #/second via <a href="#/second">, and if first returns false from canDeactivate(), a new history item would be appended for the current route.

Some early tests show this solving this issue and most of the related issues I've uncovered:

  function restorePreviousLocation(router) {
    var previousLocation = router.history.previousLocation;
    if (previousLocation) {
      Promise.resolve().then(() => history.go(router.isNavigatingBack ? 1 : -1));
    } else if (router.fallbackRoute) {
      router.navigate(router.fallbackRoute, { trigger: true, replace: true });
    } else {
      logger.error('Router navigation failed, and no previous location or fallbackRoute could be restored.');
    }
  }

I need to do additional testing to with pushState and with child routers.

This strategy is not a good one because it enters an infinite loop when the user navigates back (or forwards) more than one entry.

I think the only and right way to handle this is using sessionStorage. I'm going to give that a try.

It has been a year since I reported this issue. Any update on expected fix date?

I did spend a fair amount of time trying to fix it, but unfortunately due to limitations on the browser detailed above I was not able to make sufficient progress. I'd be happy to accept a PR from a community member who thinks they may have a solution.

Thanks for the update. We will avoid using CanDeactivate.

@TonyLugg @davismj I think I could put together a PR if it's of interest.

@jwx @davismj It is of great interest. Thank you.

I'd really like to see it too. I'd be happy to work with you on it. It's tougher than it seems. If you open up a preliminary PR we can collaborate on it as well.

I'll take a look later this weekend. Just to be clear (since I haven't really used the "exit" cycle steps): canDeactivate only triggers when you're navigating (either through link or history) to a different page within the same Aurelia application, right? Since navigation is done via browser location, if I go from my-aurelia-app.com/#/some-page to not-an-aurelia-app.com/elsewhere the exit cycle steps can't run due to the nature of the browser switching to the new site immediately. Right?

Note that this doesn't in any way prevent a user from leaving the Aurelia application; it only deals with navigation within the application.

Also, I'd really appreciate some help in testing this with pushstate, other browsers than Chrome, some more use cases etc. @davismj @TonyLugg

I'm happy to help with testing. However, I am a .NET/WPF with TFS guy turned Aurelia, so the whole git workflow is a bit foreign to me. If you can help me setup the test environment I will make the effort.

Thanks @TonyLugg! I've made two extra branches to simplify testing so just installing

npm install --save git://github.com/jwx/history-browser.git#history-index-test
npm install --save git://github.com/jwx/router.git#candeactivate-history-bugfix-test

in a project should do.

OK, I will give this a try. Are they ready now?

Yeah, should be.

@jwx I have tested your fix, both backward and forward, with cancellation and without, with user confirmation and without, including multiple history items deep. It works perfectly! Great work!

@davismj When can we get it released in a router update?

Hello @davismj !
I'm checking my opened issues at work and noticed I still have this issue opened. I'm surprised the fix from @jwx has not been merged, did you encounter some problems with it?

@gama410 This is on me. I was asked to create some thorough tests for this and have so far been coming up short. I hope to have it fixed before the end of the week.

Ok great! Thanks for your work!

Hi @davismj @jwx - Any update on this ?

Would also love to see this implemented, any update on when this could be merged? @davismj @jwx

It works in the basic case but edge cases are not handled, e.g. when a route is not found. See here for the status of the PR: #621

Is there any viable workaround for this issue? I need to ask user about leaving the large form with unsaved data.

So the bug is that there is no way to cancel "back" when you click it. You might try history.forward() if canDeactivate would return false.

You mean something like this?

canDeactivate() {
  const stopIt = ...;
  if (stopIt && router.isNavigatingBack)
    router.history.forward();
  return !stopIt;
}

Thanks, I can give it a try. Otherwise, I guess I need to ensure I return true from canDeactivate() when router.isNavigatingBack. The unsaved changes confirmation would not appear in that rare scenario when user uses the browser's back button. On the back navigation initiated from code, I can read router.history and call router.navigate(_last history item_) instead of router.navigateBack() to avoid the issue in a bit messy way, or display the unsaved changes confirmation before executing the navigation (if the code structure allows for that)...

Hmm... I found that router.isNavigatingBack doesn't return true if tried to use as described (probably a bug, router.isExplicitNavigationBack seems to work correctly. I tried your suggestion, but used router.history.go(1) instead of router.history.forward() (such doesn't exist in TypeScript definition), and it did not work. What seems like a working workaround is returning a redirect from canDeactivate() as suggested by @piwnik in #302:

return new Redirect((this.router.history as any).previousLocation, { trigger: false, replace: false });

Doing so will not update window title where it should be, but otherwise seems to be working.