fix(router): ensure navigation via back button works

The router will now navigate and respect the current address value
accordingly whenever a popState event is handled.

Closes angular#2201

Author: matsko

modules/angular2/src/router/location.ts

@@ -34,7 +34,9 @@ export class Location {
     this._platformStrategy.onPopState((_) => this._onPopState(_));
   }
 
-  _onPopState(_): void { ObservableWrapper.callNext(this._subject, {'url': this.path()}); }
+  _onPopState(_): void {
+    ObservableWrapper.callNext(this._subject, {'url': this.path(), 'pop': true});
+  }
 
   path(): string { return this.normalize(this._platformStrategy.path()); }
 

modules/angular2/src/router/router.ts

@@ -100,7 +100,7 @@ export class Router {
    * If the given URL begins with a `/`, router will navigate absolutely.
    * If the given URL does not begin with `/`, the router will navigate relative to this component.
    */
-  navigate(url: string): Promise<any> {
+  navigate(url: string, _skipLocationChange: boolean = false): Promise<any> {
     return this._currentNavigation = this._currentNavigation.then((_) => {
       this.lastNavigationAttempt = url;
       this._startNavigating();
@@ -117,7 +117,7 @@ export class Router {
               return this._canDeactivate(matchedInstruction)
                   .then((result) => {
                     if (result) {
-                      return this.commit(matchedInstruction)
+                      return this.commit(matchedInstruction, _skipLocationChange)
                           .then((_) => {
                             this._emitNavigationFinish(matchedInstruction.accumulatedUrl);
                             return true;
@@ -180,7 +180,7 @@ export class Router {
   /**
    * Updates this router and all descendant routers according to the given instruction
    */
-  commit(instruction: Instruction): Promise<any> {
+  commit(instruction: Instruction, _skipLocationChange: boolean = false): Promise<any> {
     this._currentInstruction = instruction;
     if (isPresent(this._outlet)) {
       return this._outlet.commit(instruction);
@@ -290,14 +290,17 @@ export class RootRouter extends Router {
               hostComponent: Type) {
     super(registry, pipeline, null, hostComponent);
     this._location = location;
-    this._location.subscribe((change) => this.navigate(change['url']));
+    this._location.subscribe((change) => this.navigate(change['url'], isPresent(change['pop'])));
     this.registry.configFromComponent(hostComponent, true);
     this.navigate(location.path());
   }
 
-  commit(instruction: Instruction): Promise<any> {
-    return super.commit(instruction)
-        .then((_) => { this._location.go(instruction.accumulatedUrl); });
+  commit(instruction: Instruction, _skipLocationChange: boolean = false): Promise<any> {
+    var promise = super.commit(instruction);
+    if (!_skipLocationChange) {
+      promise = promise.then((_) => { this._location.go(instruction.accumulatedUrl); });
+    }
+    return promise;
   }
 }
 
@@ -308,9 +311,9 @@ class ChildRouter extends Router {
   }
 
 
-  navigate(url: string): Promise<any> {
+  navigate(url: string, _skipLocationChange: boolean = false): Promise<any> {
     // Delegate navigation to the root router
-    return this.parent.navigate(url);
+    return this.parent.navigate(url, _skipLocationChange);
   }
 }
 

modules/angular2/test/router/location_spec.ts

@@ -88,6 +88,8 @@ export function main() {
       var locationStrategy = new MockLocationStrategy();
       var location = new Location(locationStrategy);
 
+      function assertUrl(path) { expect(location.path()).toEqual(path); }
+
       location.go('/ready');
       assertUrl('/ready');
 
@@ -102,8 +104,6 @@ export function main() {
 
       location.back();
       assertUrl('/ready');
-
-      function assertUrl(path) { expect(location.path()).toEqual(path); }
     });
   });
 }

modules/angular2/test/router/router_integration_spec.ts

@@ -27,7 +27,8 @@ import {
   RouteParams,
   Router,
   appBaseHrefToken,
-  routerDirectives
+  routerDirectives,
+  HashLocationStrategy
 } from 'angular2/router';
 
 import {LocationStrategy} from 'angular2/src/router/location_strategy';
@@ -81,6 +82,57 @@ export function main() {
          }));
     });
 
+    describe('back button app', () => {
+      beforeEachBindings(() => { return [bind(appComponentTypeToken).toValue(HierarchyAppCmp)]; });
+
+      it('should change the url without pushing a new history state for back navigations',
+         inject([AsyncTestCompleter, TestComponentBuilder], (async, tcb: TestComponentBuilder) => {
+
+           tcb.createAsync(HierarchyAppCmp)
+               .then((rootTC) => {
+                 var router = rootTC.componentInstance.router;
+                 var position = 0;
+                 var flipped = false;
+                 var history =
+                     [
+                       ['/parent/child', 'root { parent { hello } }', '/super-parent/child'],
+                       ['/super-parent/child', 'root { super-parent { hello2 } }', '/parent/child'],
+                       ['/parent/child', 'root { parent { hello } }', false]
+                     ]
+
+                     router.subscribe((_) => {
+                       var location = rootTC.componentInstance.location;
+                       var element = rootTC.nativeElement;
+                       var path = location.path();
+
+                       var entry = history[position];
+
+                       expect(path).toEqual(entry[0]);
+                       expect(element).toHaveText(entry[1]);
+
+                       var nextUrl = entry[2];
+                       if (nextUrl == false) {
+                         flipped = true;
+                       }
+
+                       if (flipped && position == 0) {
+                         async.done();
+                         return;
+                       }
+
+                       position = position + (flipped ? -1 : 1);
+                       if (flipped) {
+                         location.back();
+                       } else {
+                         router.navigate(nextUrl);
+                       }
+                     });
+
+                 router.navigate(history[0][0]);
+               });
+         }));
+    });
+
     describe('hierarchical app', () => {
       beforeEachBindings(() => { return [bind(appComponentTypeToken).toValue(HierarchyAppCmp)]; });
 
@@ -153,6 +205,11 @@ export function main() {
 class HelloCmp {
 }
 
+@Component({selector: 'hello2-cmp'})
+@View({template: 'hello2'})
+class Hello2Cmp {
+}
+
 @Component({selector: 'app-cmp'})
 @View({template: "outer { <router-outlet></router-outlet> }", directives: routerDirectives})
 @RouteConfig([new Route({path: '/', component: HelloCmp})])
@@ -166,9 +223,18 @@ class AppCmp {
 class ParentCmp {
 }
 
+@Component({selector: 'super-parent-cmp'})
+@View({template: `super-parent { <router-outlet></router-outlet> }`, directives: routerDirectives})
+@RouteConfig([new Route({path: '/child', component: Hello2Cmp})])
+class SuperParentCmp {
+}
+
 @Component({selector: 'app-cmp'})
 @View({template: `root { <router-outlet></router-outlet> }`, directives: routerDirectives})
-@RouteConfig([new Route({path: '/parent/...', component: ParentCmp})])
+@RouteConfig([
+  new Route({path: '/parent/...', component: ParentCmp}),
+  new Route({path: '/super-parent/...', component: SuperParentCmp})
+])
 class HierarchyAppCmp {
   constructor(public router: Router, public location: LocationStrategy) {}
 }

modules/angular2/test/router/router_spec.ts

@@ -76,6 +76,20 @@ export function main() {
              });
        }));
 
+    it('should not push a history change on when navigate is called with skipUrlChange',
+       inject([AsyncTestCompleter], (async) => {
+         var outlet = makeDummyOutlet();
+
+         router.registerOutlet(outlet)
+             .then((_) => router.config([new Route({path: '/b', component: DummyComponent})]))
+             .then((_) => router.navigate('/b', true))
+             .then((_) => {
+               expect(outlet.spy('commit')).toHaveBeenCalled();
+               expect(location.urlChanges).toEqual([]);
+               async.done();
+             });
+       }));
+
 
     it('should navigate after being configured', inject([AsyncTestCompleter], (async) => {
          var outlet = makeDummyOutlet();