From 535e8fafd57d46bd97d1a89542a6c0d713356364 Mon Sep 17 00:00:00 2001 From: Caitlin Potter Date: Tue, 18 Mar 2014 11:11:39 -0400 Subject: [PATCH] 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 #4987 --- src/ng/httpBackend.js | 53 ++++++++++++++--------------- test/ng/httpBackendSpec.js | 68 ++------------------------------------ 2 files changed, 26 insertions(+), 95 deletions(-) diff --git a/src/ng/httpBackend.js b/src/ng/httpBackend.js index 28107966f643..10055d410627 100644 --- a/src/ng/httpBackend.js +++ b/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 { @@ -158,33 +155,31 @@ 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; - rawDocument.body.removeChild(script); - if (done) done(); - }; - - script.type = 'text/javascript'; + var script = document.createElement('script'), callback = null; + script.type = "text/javascript"; script.src = url; + script.async = true; + + callback = function(event) { + removeEventListenerFn(script, "load", callback); + removeEventListenerFn(script, "error", callback); + rawDocument.body.removeChild(script); + script = null; + if (!callbacks[callbackId].called) event = { type: "error" }; + var status = (event && (event.type === "error" ? 404 : 200)) || -1; + var text = (event && event.type) || "unknown"; + if (done) { + done(status, text); + } + }; - if (msie && msie <= 8) { - script.onreadystatechange = function() { - if (/loaded|complete/.test(script.readyState)) { - doneWrapper(); - } - }; - } else { - script.onload = script.onerror = function() { - doneWrapper(); - }; - } - + addEventListenerFn(script, "load", callback); + addEventListenerFn(script, "error", callback); rawDocument.body.appendChild(script); - return doneWrapper; + return callback; } } diff --git a/test/ng/httpBackendSpec.js b/test/ng/httpBackendSpec.js index 5c9cbf586c36..1e8dc85f6135 100644 --- a/test/ng/httpBackendSpec.js +++ b/test/ng/httpBackendSpec.js @@ -325,13 +325,7 @@ describe('$httpBackend', function() { expect(url[1]).toBe('http://example.org/path'); callbacks[url[2]]('some-data'); - - if (script.onreadystatechange) { - script.readyState = 'complete'; - script.onreadystatechange(); - } else { - script.onload(); - } + browserTrigger(script, "load"); expect(callback).toHaveBeenCalledOnce(); }); @@ -346,71 +340,13 @@ 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); expect(fakeDocument.body.removeChild).toHaveBeenCalledOnceWith(script); }); - if(msie<=8) { - - it('should attach onreadystatechange handler to the script object', function() { - $backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop); - - expect(fakeDocument.$$scripts[0].onreadystatechange).toEqual(jasmine.any(Function)); - - var script = fakeDocument.$$scripts[0]; - - script.readyState = 'complete'; - script.onreadystatechange(); - - 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() { - callback.andCallFake(function(status, response) { - expect(status).toBe(-2); - expect(response).toBeUndefined(); - }); - - $backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback); - expect(fakeDocument.$$scripts.length).toBe(1); - - var script = fakeDocument.$$scripts.shift(); - if (script.onreadystatechange) { - script.readyState = 'complete'; - script.onreadystatechange(); - } else { - script.onload(); - } - expect(callback).toHaveBeenCalledOnce(); - }); - - it('should set url to current location if not specified or empty string', function() { $backend('JSONP', undefined, null, callback); expect(fakeDocument.$$scripts[0].src).toBe($browser.url());