From 2492d00e75aee90012ea64f66b54ee33f92ae00c Mon Sep 17 00:00:00 2001
From: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Date: Wed, 6 Jun 2018 11:27:19 +0100
Subject: [PATCH] Stylistic tweaks

---
 .../src/Services/UriHelper.ts                 | 11 ++++--
 .../Tests/RoutingTest.cs                      | 37 +++++++++----------
 2 files changed, 25 insertions(+), 23 deletions(-)

diff --git a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Services/UriHelper.ts b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Services/UriHelper.ts
index 350d78c5d4..4200864dab 100644
--- a/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Services/UriHelper.ts
+++ b/src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Services/UriHelper.ts
@@ -23,10 +23,9 @@ registerFunction(`${registeredFunctionPrefix}.enableNavigationInterception`, ()
     if (anchorTarget) {
       const href = anchorTarget.getAttribute('href');
       const absoluteHref = toAbsoluteUri(href);
-      //if the user wants to user some specific browser/OS feature, we dont handle it and let the browser/OS
-      const anyChangeBehaviorKeyHold = event.ctrlKey || event.shiftKey || event.altKey || event.metaKey;
-      if (isWithinBaseUriSpace(absoluteHref) && !anyChangeBehaviorKeyHold) 
-      {
+
+      // Don't stop ctrl/meta-click (etc) from opening links in new tabs/windows
+      if (isWithinBaseUriSpace(absoluteHref) && !eventHasSpecialKey(event)) {
         event.preventDefault();
         performInternalNavigation(absoluteHref);
       }
@@ -92,3 +91,7 @@ function isWithinBaseUriSpace(href: string) {
 function toBaseUriWithTrailingSlash(baseUri: string) {
   return baseUri.substr(0, baseUri.lastIndexOf('/') + 1);
 }
+
+function eventHasSpecialKey(event: MouseEvent) {
+  return event.ctrlKey || event.shiftKey || event.altKey || event.metaKey;
+}
diff --git a/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/RoutingTest.cs b/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/RoutingTest.cs
index d2981abef9..ebe8b0fac8 100644
--- a/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/RoutingTest.cs
+++ b/test/Microsoft.AspNetCore.Blazor.E2ETest/Tests/RoutingTest.cs
@@ -88,43 +88,42 @@ namespace Microsoft.AspNetCore.Blazor.E2ETest.Tests
         [Fact]
         public void CanFollowLinkToOtherPageWithCtrlClick()
         {
+            // On macOS we need to hold the command key not the control for opening a popup
             var key = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? Keys.Command : Keys.Control;
+
             try
             {
-                SetUrlViaPushState($"{ServerPathBase}/RouterTest/");
+                SetUrlViaPushState($"{ServerPathBase}/");
 
                 var app = MountTestComponent<TestRouter>();
                 var button = app.FindElement(By.LinkText("Other"));
-                //on mac os build we need to hold the meta button not the control for openning a popup
               
-                new Actions(Browser)
-                    .KeyDown(Keys.Control)
-                    .Click(button)
-                    .Build()
-                    .Perform();
+                new Actions(Browser).KeyDown(key).Click(button).Build().Perform();
 
                 Assert.Equal(2, Browser.WindowHandles.Count);
-
-                //closing newly opened windows if a new one was opened
-
-                Browser.SwitchTo().Window(Browser.WindowHandles.Last());
-                Browser.Close();
-                Browser.SwitchTo().Window(Browser.WindowHandles.First());
             }
             finally
             {
-                // leaving the ctrl key up 
-                new Actions(Browser)
-                    .KeyUp(key)
-                    .Build()
-                    .Perform();
+                // Leaving the ctrl key up 
+                new Actions(Browser).KeyUp(key).Build().Perform();
+                
+                // Closing newly opened windows if a new one was opened
+                while (Browser.WindowHandles.Count > 1)
+                {
+                    Browser.SwitchTo().Window(Browser.WindowHandles.Last());
+                    Browser.Close();
+                }
+
+                // Needed otherwise Selenium tries to direct subsequent commands
+                // to the tab that has already been closed
+                Browser.SwitchTo().Window(Browser.WindowHandles.First());
             }
         }
 
         [Fact]
         public void CanFollowLinkToOtherPageDoesNotOpenNewWindow()
         {
-            SetUrlViaPushState($"{ServerPathBase}/RouterTest/");
+            SetUrlViaPushState($"{ServerPathBase}/");
 
             var app = MountTestComponent<TestRouter>();