fix(router): canDeactivate correctly cancels browser history navigation

Preventing a navigation with canDeactivate doesn't cancel the browser history navigation. This fix moves the browser back to the index where navigation was cancelled.

Depending on PR aurelia/history-browser#42.
Closes aurelia#528.

src/app-router.js

@@ -138,12 +138,20 @@ export class AppRouter extends Router {
         this.isNavigatingForward = true;
       } else if (this.currentNavigationTracker > navtracker) {
         this.isNavigatingBack = true;
-      } if (!navtracker) {
+      }
+      if (!navtracker) {
         navtracker = Date.now();
         this.history.setState('NavigationTracker', navtracker);
       }
       this.currentNavigationTracker = navtracker;
 
+      let historyIndex = this.history.getHistoryIndex();
+      this.lastHistoryMovement = historyIndex - this.currentHistoryIndex;
+      if (isNaN(this.lastHistoryMovement)) {
+        this.lastHistoryMovement = 0;
+      }
+      this.currentHistoryIndex = historyIndex;
+
       instruction.previousInstruction = this.currentInstruction;
 
       if (!instructionCount) {
@@ -256,7 +264,11 @@ function resolveInstruction(instruction, result, isInnerInstruction, router) {
 function restorePreviousLocation(router) {
   let previousLocation = router.history.previousLocation;
   if (previousLocation) {
-    router.navigate(router.history.previousLocation, { trigger: false, replace: true });
+    Promise.resolve().then(() => {
+      if (router.lastHistoryMovement && !isNaN(router.lastHistoryMovement)) {
+        router.history.history.go(-router.lastHistoryMovement);
+      }
+    });
   } else if (router.fallbackRoute) {
     router.navigate(router.fallbackRoute, { trigger: true, replace: true });
   } else {

src/router.js

@@ -94,6 +94,16 @@ export class Router {
   */
   currentNavigationTracker: number;
 
+  /**
+  * The current index in the browser history.
+  */
+  currentHistoryIndex: number;
+
+  /**
+  * The amount of index steps in the last history navigation
+  */
+  lastHistoryMovement: number;
+
   /**
   * The navigation models for routes that specified [[RouteConfig.nav]].
   */

test/app-router.spec.js

@@ -5,13 +5,48 @@ import {RouteLoader} from '../src/route-loading';
 import {Pipeline} from '../src/pipeline';
 
 class MockHistory extends History {
+  history;
   activate() {}
   deactivate() {}
-  navigate() {}
+  navigate(location) {
+    this.history.navigate(location)
+  }
   navigateBack() {}
-  setState(key, value) {}
+  setState(key, value) {
+    this.history.setState(key, value);
+  }
+  getState(key) {
+    return this.history.getState(key);
+  }
+}
+
+class MockBrowserHistory {
+  currentLocationIndex = 0;
+  states = [];
+  get length() {
+    return this.states.length;
+  }
+  setState(key, value) {
+    if (!this.states[this.currentLocationIndex]) {
+      this.states[this.currentLocationIndex] = {};
+    }
+    this.states[this.currentLocationIndex][key] = value;
+  }
   getState(key) {
-    return null;
+    if (!this.states[this.currentLocationIndex]) {
+      this.states[this.currentLocationIndex] = {};
+    }
+    return this.states[this.currentLocationIndex][key];
+  }
+  location(location) {
+    this.states.splice(this.currentLocationIndex + 1);
+    this.currentLocationIndex = this.states.length;
+    this.states.push({ HistoryIndex: this.currentLocationIndex });
+    return location;
+  }
+  go(movement) {
+    this.currentLocationIndex += movement;
+    console.log('GO', this.currentLocationIndex, this.states);
   }
 }
 
@@ -42,6 +77,7 @@ describe('app-router', () => {
 
   beforeEach(() => {
     history = new MockHistory();
+    history.history = new MockBrowserHistory();
     container = new Container();
     container.registerSingleton(RouteLoader, MockLoader);
     ea = { publish() {} };
@@ -208,12 +244,17 @@ describe('app-router', () => {
   describe('loadUrl', () => {
     it('restores previous location when route not found', (done) => {
       spyOn(history, 'navigate');
+      spyOn(history.history, 'go');
 
-      router.history.previousLocation = 'prev';
-      router.loadUrl('next')
+      router.history.previousLocation = router.history.history.location('prev');
+
+      router.lastHistoryMovement = 1;
+      router.loadUrl(router.history.history.location('next'))
         .then(result => {
           expect(result).toBeFalsy();
-          expect(history.navigate).toHaveBeenCalledWith('#/prev', { trigger: false, replace: true });
+          // Navigation is now restored through the browser history
+          // expect(history.navigate).toHaveBeenCalledWith('#/prev', { trigger: false, replace: true });
+          expect(history.history.go).toHaveBeenCalledWith(-1);
         })
         .catch(result => expect(true).toBeFalsy('should have succeeded'))
         .then(done);
@@ -235,19 +276,24 @@ describe('app-router', () => {
 
     it('restores previous location on error', (done) => {
       spyOn(history, 'navigate');
+      spyOn(history.history, 'go');
+
+      router.history.previousLocation = router.history.history.location('prev');
 
-      router.history.previousLocation = 'prev';
       router.activate();
       router.configure(config => {
         config.map([
           { name: 'test', route: '', moduleId: './test' }
         ]);
       });
 
-      router.loadUrl('next')
+      router.lastHistoryMovement = 1;
+      router.loadUrl(router.history.history.location('next'))
         .then(result => {
           expect(result).toBeFalsy();
-          expect(history.navigate).toHaveBeenCalledWith('#/prev', { trigger: false, replace: true });
+          // Navigation is now restored through the browser history
+          // expect(history.navigate).toHaveBeenCalledWith('#/prev', { trigger: false, replace: true });
+          expect(router.history.history.go).toHaveBeenCalledWith(-1);
         })
         .catch(result => expect(true).toBeFalsy('should have succeeded'))
         .then(done);