Improved popup error handling.

sahat · Feb 17, 2016 · 0a8b967 · 0a8b967
1 parent e91d703
commit 0a8b967
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 15 deletions.
diff --git a/satellizer.js b/satellizer.js
@@ -750,36 +750,43 @@ if (typeof module !== 'undefined' && typeof exports !== 'undefined' && module.ex
             (redirectUriParser.port ? ':' + redirectUriParser.port: '');
 
           var polling = $interval(function() {
+            if (!Popup.popupWindow || Popup.popupWindow.closed || Popup.popupWindow.closed === undefined) {
+              deferred.reject('Popup window was closed.');
+              $interval.cancel(polling);
+            }
+
             try {
               var popupWindowHost = Popup.popupWindow.location.protocol + '//' + Popup.popupWindow.location.hostname +
-                (Popup.popupWindow.location.port ? ':' + Popup.popupWindow.location.port: '');
+                    (Popup.popupWindow.location.port ? ':' + Popup.popupWindow.location.port: '');
 
-              if (popupWindowHost === redirectUriHost && (Popup.popupWindow.location.search || Popup.popupWindow.location.hash)) {
-                var queryParams = Popup.popupWindow.location.search.substring(1).replace(/\/$/, '');
-                var hashParams = Popup.popupWindow.location.hash.substring(1).replace(/[\/$]/, '');
-                var hash = utils.parseQueryString(hashParams);
-                var qs = utils.parseQueryString(queryParams);
+              if (popupWindowHost === redirectUriHost) {  // Redirect occurred.
+                if (Popup.popupWindow.location.search || Popup.popupWindow.location.hash) {
+                  var queryParams = Popup.popupWindow.location.search.substring(1).replace(/\/$/, '');
+                  var hashParams = Popup.popupWindow.location.hash.substring(1).replace(/[\/$]/, '');
+                  var hash = utils.parseQueryString(hashParams);
+                  var qs = utils.parseQueryString(queryParams);
 
-                angular.extend(qs, hash);
+                  angular.extend(qs, hash);
 
-                if (qs.error) {
-                  deferred.reject(qs);
+                  if (qs.error) {
+                    deferred.reject(qs);
+                  } else {
+                    deferred.resolve(qs);
+                  }
                 } else {
-                  deferred.resolve(qs);
+                  deferred.reject('Redirect occurred but satellizer found no query or hash parameters.'
+                                  + 'They were either not set by the redirect, or somebody else '
+                                  + 'removed them before satellizer could read them '
+                                  + '(e.g. angular routing mechanism).');
                 }
 
                 $interval.cancel(polling);
-
                 Popup.popupWindow.close();
               }
             } catch (error) {
               // Ignore DOMException: Blocked a frame with origin from accessing a cross-origin frame.
               // A hack to get around same-origin security policy errors in IE.
             }
-
-            if (!Popup.popupWindow || Popup.popupWindow.closed || Popup.popupWindow.closed === undefined) {
-              $interval.cancel(polling);
-            }
           }, 20);
 
           return deferred.promise;

diff --git a/test/popup.spec.js b/test/popup.spec.js
@@ -51,6 +51,29 @@ describe('SatellizerPopup', function() {
     expect(angular.isObject(open)).toBe(true);
   });
 
+  it('should handle the case when popup redirect has occured but no parameters are set', function() {
+    this.popup.popupWindow = {
+      location: {
+        host: document.location.host
+      }
+    };
+    var open = this.popup.pollPopup();
+    var error;
+    open.catch(function (err) {
+      error = err;
+    });
+    this.$interval.flush(300);
+    expect(error).toBeDefined();
+  });
 
+  it('should handle the case when popup is closed', function() {
+    var open = this.popup.pollPopup();
+    var error;
+    open.catch(function (err) {
+      error = err;
+    });
+    this.$interval.flush(300);
+    expect(error).toBeDefined();
+  });
 });