From fa2ce8100b02e00d2e48c653f91f587f5e6ec540 Mon Sep 17 00:00:00 2001
From: Dimitrios Loukadakis <dimitris.loukadakis@gmail.com>
Date: Wed, 25 May 2016 01:23:46 +0300
Subject: [PATCH] fix(router): openning links in new tab

Clicks on router-link should not prevent browser default action when
any mouse button other than left mouse button or ctrl/meta key is pressed.

router-link href should use location strategy external url.

Closes #5908
Closes #6806
Closes #7749
Closes #8806

Closes #8821
---
 .../router/src/directives/router_link.ts      | 24 ++++++++++++-------
 .../@angular/router/test/router_link_spec.ts  | 15 ++++++------
 2 files changed, 23 insertions(+), 16 deletions(-)

diff --git a/modules/@angular/router/src/directives/router_link.ts b/modules/@angular/router/src/directives/router_link.ts
index b7a75df9cf..889d02a041 100644
--- a/modules/@angular/router/src/directives/router_link.ts
+++ b/modules/@angular/router/src/directives/router_link.ts
@@ -3,6 +3,7 @@ import {Router} from '../router';
 import {RouteSegment} from '../segments';
 import {isString, isArray, isPresent} from '../facade/lang';
 import {ObservableWrapper} from '../facade/async';
+import {LocationStrategy} from '@angular/common';
 
 /**
  * The RouterLink directive lets you link to specific parts of your app.
@@ -44,7 +45,8 @@ export class RouterLink implements OnDestroy {
   @HostBinding() href: string;
   @HostBinding('class.router-link-active') isActive: boolean = false;
 
-  constructor(private _routeSegment: RouteSegment, private _router: Router) {
+  constructor(private _routeSegment: RouteSegment, private _router: Router,
+              private _locationStrategy: LocationStrategy) {
     // because auxiliary links take existing primary and auxiliary routes into account,
     // we need to update the link whenever params or other routes change.
     this._subscription =
@@ -64,20 +66,24 @@ export class RouterLink implements OnDestroy {
   }
 
 
-  @HostListener("click")
-  onClick(): boolean {
-    // If no target, or if target is _self, prevent default browser behavior
-    if (!isString(this.target) || this.target == '_self') {
-      this._router.navigate(this._commands, this._routeSegment);
-      return false;
+  @HostListener("click", ["$event.button", "$event.ctrlKey", "$event.metaKey"])
+  onClick(button: number, ctrlKey: boolean, metaKey: boolean): boolean {
+    if (button != 0 || ctrlKey || metaKey) {
+      return true;
     }
-    return true;
+
+    if (isString(this.target) && this.target != '_self') {
+      return true;
+    }
+
+    this._router.navigate(this._commands, this._routeSegment);
+    return false;
   }
 
   private _updateTargetUrlAndHref(): void {
     let tree = this._router.createUrlTree(this._commands, this._routeSegment);
     if (isPresent(tree)) {
-      this.href = this._router.serializeUrl(tree);
+      this.href = this._locationStrategy.prepareExternalUrl(this._router.serializeUrl(tree));
       this.isActive = this._router.urlTree.contains(tree);
     } else {
       this.isActive = false;
diff --git a/modules/@angular/router/test/router_link_spec.ts b/modules/@angular/router/test/router_link_spec.ts
index c031ff3391..929a73a862 100644
--- a/modules/@angular/router/test/router_link_spec.ts
+++ b/modules/@angular/router/test/router_link_spec.ts
@@ -26,8 +26,8 @@ import {
   OnActivate,
   CanDeactivate
 } from '@angular/router';
-import {Location} from '@angular/common';
-import {SpyLocation} from '@angular/common/testing';
+import {Location, LocationStrategy} from '@angular/common';
+import {SpyLocation, MockLocationStrategy} from '@angular/common/testing';
 
 export function main() {
   describe('RouterLink', () => {
@@ -35,6 +35,7 @@ export function main() {
       provide(RouterUrlSerializer, {useClass: DefaultRouterUrlSerializer}),
       RouterOutletMap,
       provide(Location, {useClass: SpyLocation}),
+      provide(LocationStrategy, {useClass: MockLocationStrategy}),
       provide(Router,
               {
                 useFactory: (resolver, urlParser, outletMap, location) => new Router(
@@ -44,14 +45,14 @@ export function main() {
     ]);
 
     describe("routerLink=", () => {
-      it("should accept an array of commands", inject([Router], (router) => {
-           let link = new RouterLink(null, router);
+      it("should accept an array of commands", inject([Router, LocationStrategy], (router, locationStrategy) => {
+           let link = new RouterLink(null, router, locationStrategy);
            link.routerLink = ['/one', 11];
            expect(link.href).toEqual("/one/11");
          }));
 
-      it("should accept a single command", inject([Router], (router) => {
-           let link = new RouterLink(null, router);
+      it("should accept a single command", inject([Router, LocationStrategy], (router, locationStrategy) => {
+           let link = new RouterLink(null, router, locationStrategy);
            link.routerLink = '/one/11';
            expect(link.href).toEqual("/one/11");
          }));
@@ -61,4 +62,4 @@ export function main() {
 
 @Component({template: ''})
 class RootCmp {
-}
\ No newline at end of file
+}