Fix OAuth popup being blocked by pop-up blocker in Firefox and IE

src/sidebar/oauth-auth.js

@@ -337,6 +337,29 @@ function auth($http, $rootScope, $window,
     var left   = $window.screen.width / 2 - width / 2;
     var top    = $window.screen.height /2 - height / 2;
 
+    // Generate settings for `window.open` in the required comma-separated
+    // key=value format.
+    var authWindowSettings = queryString.stringify({
+      left: left,
+      top: top,
+      width: width,
+      height: height,
+    }).replace(/&/g, ',');
+
+    // Open the auth window before fetching the `oauth.authorize` URL to ensure
+    // that the `window.open` call happens in the same turn of the event loop
+    // that was initiated by the user clicking the "Log in" link.
+    //
+    // Otherwise the `window.open` call is not deemed to be in response to a
+    // user gesture in Firefox & IE 11 and their popup blocking heuristics will
+    // prevent the window being opened. See
+    // https://github.com/hypothesis/client/issues/534 and
+    // https://github.com/hypothesis/client/issues/535.
+    //
+    // Chrome, Safari & Edge have different heuristics and are not affected by
+    // this problem.
+    var authWindow = $window.open('about:blank', 'Login to Hypothesis', authWindowSettings);
+
     return apiRoutes.links().then(links => {
       var authUrl = links['oauth.authorize'];
       authUrl += '?' + queryString.stringify({
@@ -346,13 +369,7 @@ function auth($http, $rootScope, $window,
         response_type: 'code',
         state: state,
       });
-      var authWindowSettings = queryString.stringify({
-        left: left,
-        top: top,
-        width: width,
-        height: height,
-      }).replace(/&/g, ',');
-      $window.open(authUrl, 'Login to Hypothesis', authWindowSettings);
+      authWindow.location = authUrl;
 
       return authResponse;
     }).then((resp) => {

src/sidebar/test/oauth-auth-test.js

@@ -12,21 +12,31 @@ class FakeWindow {
   constructor() {
     this.callbacks = [];
 
-    this.location = {
-      origin: 'client.hypothes.is',
-    };
-
     this.screen = {
       width: 1024,
       height: 768,
     };
 
-    this.open = sinon.stub();
+
+    this.location = 'https://client.hypothes.is/app.html';
+    this.open = sinon.spy(href => {
+      var win = new FakeWindow;
+      win.location = href;
+      return win;
+    });
 
     this.setTimeout = window.setTimeout.bind(window);
     this.clearTimeout = window.clearTimeout.bind(window);
   }
 
+  get location() {
+    return this.url;
+  }
+
+  set location(href) {
+    this.url = new URL(href);
+  }
+
   addEventListener(event, callback) {
     this.callbacks.push({event, callback});
   }
@@ -511,18 +521,24 @@ describe('sidebar.oauth-auth', function () {
         var authUrl = links['oauth.authorize'];
         var params = {
           client_id: fakeSettings.oauthClientId,
-          origin: 'client.hypothes.is',
+          origin: 'https://client.hypothes.is',
           response_mode: 'web_message',
           response_type: 'code',
           state: 'notrandom',
         };
         var expectedAuthUrl = `${authUrl}?${stringify(params)}`;
+
+        // Check that the auth window was opened and then set to the expected
+        // location. The final URL is not passed to `window.open` to work around
+        // a pop-up blocker issue.
         assert.calledWith(
           fakeWindow.open,
-          expectedAuthUrl,
+          'about:blank',
           'Login to Hypothesis',
           'height=400,left=312,top=184,width=400'
         );
+        var authPopup = fakeWindow.open.returnValues[0];
+        assert.equal(authPopup.location.href, expectedAuthUrl);
       });
     });