fix($httpBackend): don't error when JSONP callback called with no parameter

This change brings Angular's JSONP behaviour closer in line with jQuery's. It will no longer treat
a callback called with no data as an error, and will no longer support IE8 via the onreadystatechanged
event.

BREAKING CHANGE:

Previously, the JSONP backend code would support IE8 by relying on the readystatechanged events. This
is no longer the case, as these events do not provide adequate useful information for deeming whether
or not a response is an error.

Previously, a JSONP response which did not pass data into the callback would be given a status of -2,
and treated as an error. Now, this situation will instead be given a status of 200, despite the lack
of data. This is useful for interaction with certain APIs.

Previously, the onload and onerror callbacks were added to the JSONP script tag. These have been
replaced with jQuery events, in order to gain access to the event object. This means that it is now
difficult to test if the callbacks are registered or not. This is possible with jQuery, using the
$.data("events") method, however it is currently impossible with jqLite. This is not expected to
break applications.

Closes angular#4987
Closes angular#6735

Author: caitp

src/ng/httpBackend.js

@@ -49,16 +49,13 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
       var callbackId = '_' + (callbacks.counter++).toString(36);
       callbacks[callbackId] = function(data) {
         callbacks[callbackId].data = data;
+        callbacks[callbackId].called = true;
       };
 
       var jsonpDone = jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
-          function() {
-        if (callbacks[callbackId].data) {
-          completeRequest(callback, 200, callbacks[callbackId].data);
-        } else {
-          completeRequest(callback, status || -2);
-        }
-        callbacks[callbackId] = angular.noop;
+          callbackId, function(status, text) {
+        completeRequest(callback, status, callbacks[callbackId].data, "", text);
+        callbacks[callbackId] = noop;
       });
     } else {
 
@@ -160,33 +157,63 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
     }
   };
 
-  function jsonpReq(url, done) {
+  function jsonpReq(url, callbackId, done) {
     // we can't use jQuery/jqLite here because jQuery does crazy shit with script elements, e.g.:
     // - fetches local scripts via XHR and evals them
     // - adds and immediately removes script elements from the document
     var script = rawDocument.createElement('script'),
-        doneWrapper = function() {
-          script.onreadystatechange = script.onload = script.onerror = null;
+        addedEventListeners,
+        callback = function(event) {
+          script.onreadystatechange = null;
+          if (addedEventListeners) {
+            removeEventListenerFn(script, "load", callback);
+            removeEventListenerFn(script, "error", callback);
+          }
           rawDocument.body.removeChild(script);
-          if (done) done();
+          script = null;
+          var status = -1;
+          var text = "unknown";
+
+          if (event) {
+            if (event.type === "load" && !callbacks[callbackId].called) {
+              event = { type: "error" };
+            }
+            text = event.type;
+            status = event.type === "error" ? 404 : 200;
+          } else if (msie <= 8) {
+            // For IE, we never have an event object here, so the only thing we can do is check if the
+            // function was called, or not.
+            if (!callbacks[callbackId].called) {
+              status = 404;
+              text = "error";
+            } else {
+              status = 200;
+              text = "success";
+            }
+          }
+
+          if (done) {
+            done(status, text);
+          }
         };
 
-    script.type = 'text/javascript';
+    script.type = "text/javascript";
     script.src = url;
+    script.async = true;
 
     if (msie && msie <= 8) {
       script.onreadystatechange = function() {
         if (/loaded|complete/.test(script.readyState)) {
-          doneWrapper();
+          callback();
         }
       };
     } else {
-      script.onload = script.onerror = function() {
-        doneWrapper();
-      };
+      addedEventListeners = true;
+      addEventListenerFn(script, "load", callback);
+      addEventListenerFn(script, "error", callback);
     }
 
     rawDocument.body.appendChild(script);
-    return doneWrapper;
+    return callback;
   }
 }

test/ng/httpBackendSpec.js

@@ -39,7 +39,8 @@ describe('$httpBackend', function() {
     fakeDocument = {
       $$scripts: [],
       createElement: jasmine.createSpy('createElement').andCallFake(function() {
-        return {};
+        // Return a proper script element...
+        return document.createElement(arguments[0]);
       }),
       body: {
         appendChild: jasmine.createSpy('body.appendChild').andCallFake(function(script) {
@@ -356,7 +357,7 @@ describe('$httpBackend', function() {
         script.readyState = 'complete';
         script.onreadystatechange();
       } else {
-        script.onload();
+        browserTrigger(script, "load");
       }
 
       expect(callback).toHaveBeenCalledOnce();
@@ -372,12 +373,11 @@ describe('$httpBackend', function() {
           callbackId = script.src.match(SCRIPT_URL)[2];
 
       callbacks[callbackId]('some-data');
-
       if (script.onreadystatechange) {
         script.readyState = 'complete';
         script.onreadystatechange();
       } else {
-        script.onload();
+        browserTrigger(script, "load");
       }
 
       expect(callbacks[callbackId]).toBe(angular.noop);
@@ -386,7 +386,6 @@ describe('$httpBackend', function() {
 
 
     if(msie<=8) {
-
       it('should attach onreadystatechange handler to the script object', function() {
         $backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop);
 
@@ -399,27 +398,11 @@ describe('$httpBackend', function() {
 
         expect(script.onreadystatechange).toBe(null);
       });
-
-    } else {
-
-      it('should attach onload and onerror handlers to the script object', function() {
-        $backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop);
-
-        expect(fakeDocument.$$scripts[0].onload).toEqual(jasmine.any(Function));
-        expect(fakeDocument.$$scripts[0].onerror).toEqual(jasmine.any(Function));
-
-        var script = fakeDocument.$$scripts[0];
-        script.onload();
-
-        expect(script.onload).toBe(null);
-        expect(script.onerror).toBe(null);
-      });
-
     }
 
-    it('should call callback with status -2 when script fails to load', function() {
+    it('should call callback with status 404 when script fails to load', function() {
       callback.andCallFake(function(status, response) {
-        expect(status).toBe(-2);
+        expect(status).toBe(404);
         expect(response).toBeUndefined();
       });
 
@@ -431,7 +414,7 @@ describe('$httpBackend', function() {
         script.readyState = 'complete';
         script.onreadystatechange();
       } else {
-        script.onload();
+        browserTrigger(script, "load");
       }
       expect(callback).toHaveBeenCalledOnce();
     });