Skip to content

Commit c869679

Browse files
committed
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
1 parent 748a6c8 commit c869679

File tree

2 files changed

+25
-91
lines changed

2 files changed

+25
-91
lines changed

src/ng/httpBackend.js

+23-31
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,9 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
5252
};
5353

5454
var jsonpDone = jsonpReq(url.replace('JSON_CALLBACK', 'angular.callbacks.' + callbackId),
55-
function() {
56-
if (callbacks[callbackId].data) {
57-
completeRequest(callback, 200, callbacks[callbackId].data);
58-
} else {
59-
completeRequest(callback, status || -2);
60-
}
61-
callbacks[callbackId] = angular.noop;
55+
function(status, text) {
56+
completeRequest(callback, status, callbacks[callbackId].data, "", text);
57+
callbacks[callbackId] = noop;
6258
});
6359
} else {
6460

@@ -162,29 +158,25 @@ function createHttpBackend($browser, createXhr, $browserDefer, callbacks, rawDoc
162158
// we can't use jQuery/jqLite here because jQuery does crazy shit with script elements, e.g.:
163159
// - fetches local scripts via XHR and evals them
164160
// - adds and immediately removes script elements from the document
165-
var script = rawDocument.createElement('script'),
166-
doneWrapper = function() {
167-
script.onreadystatechange = script.onload = script.onerror = null;
168-
rawDocument.body.removeChild(script);
169-
if (done) done();
170-
};
171-
172-
script.type = 'text/javascript';
173-
script.src = url;
174-
175-
if (msie && msie <= 8) {
176-
script.onreadystatechange = function() {
177-
if (/loaded|complete/.test(script.readyState)) {
178-
doneWrapper();
179-
}
180-
};
181-
} else {
182-
script.onload = script.onerror = function() {
183-
doneWrapper();
184-
};
185-
}
186-
187-
rawDocument.body.appendChild(script);
188-
return doneWrapper;
161+
var script = jqLite('<script>'), callback = null;
162+
script.prop({
163+
type: 'text/javascript',
164+
async: true,
165+
src: url
166+
});
167+
168+
script.on('load error', callback = function(event) {
169+
script.off("load error", callback);
170+
rawDocument.body.removeChild(script[0]);
171+
script = null;
172+
callback = null;
173+
var status = (event && (event.type === "error" ? 404 : 200)) || -1;
174+
var text = (event && event.type) || "unknown";
175+
if (done) {
176+
done(status, text);
177+
}
178+
});
179+
rawDocument.body.appendChild(script[0]);
180+
return callback;
189181
}
190182
}

test/ng/httpBackendSpec.js

+2-60
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ describe('$httpBackend', function() {
330330
script.readyState = 'complete';
331331
script.onreadystatechange();
332332
} else {
333-
script.onload();
333+
browserTrigger(script, "load");
334334
}
335335

336336
expect(callback).toHaveBeenCalledOnce();
@@ -346,71 +346,13 @@ describe('$httpBackend', function() {
346346
callbackId = script.src.match(SCRIPT_URL)[2];
347347

348348
callbacks[callbackId]('some-data');
349-
350-
if (script.onreadystatechange) {
351-
script.readyState = 'complete';
352-
script.onreadystatechange();
353-
} else {
354-
script.onload();
355-
}
349+
browserTrigger(script, "load");
356350

357351
expect(callbacks[callbackId]).toBe(angular.noop);
358352
expect(fakeDocument.body.removeChild).toHaveBeenCalledOnceWith(script);
359353
});
360354

361355

362-
if(msie<=8) {
363-
364-
it('should attach onreadystatechange handler to the script object', function() {
365-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop);
366-
367-
expect(fakeDocument.$$scripts[0].onreadystatechange).toEqual(jasmine.any(Function));
368-
369-
var script = fakeDocument.$$scripts[0];
370-
371-
script.readyState = 'complete';
372-
script.onreadystatechange();
373-
374-
expect(script.onreadystatechange).toBe(null);
375-
});
376-
377-
} else {
378-
379-
it('should attach onload and onerror handlers to the script object', function() {
380-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop);
381-
382-
expect(fakeDocument.$$scripts[0].onload).toEqual(jasmine.any(Function));
383-
expect(fakeDocument.$$scripts[0].onerror).toEqual(jasmine.any(Function));
384-
385-
var script = fakeDocument.$$scripts[0];
386-
script.onload();
387-
388-
expect(script.onload).toBe(null);
389-
expect(script.onerror).toBe(null);
390-
});
391-
392-
}
393-
394-
it('should call callback with status -2 when script fails to load', function() {
395-
callback.andCallFake(function(status, response) {
396-
expect(status).toBe(-2);
397-
expect(response).toBeUndefined();
398-
});
399-
400-
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, callback);
401-
expect(fakeDocument.$$scripts.length).toBe(1);
402-
403-
var script = fakeDocument.$$scripts.shift();
404-
if (script.onreadystatechange) {
405-
script.readyState = 'complete';
406-
script.onreadystatechange();
407-
} else {
408-
script.onload();
409-
}
410-
expect(callback).toHaveBeenCalledOnce();
411-
});
412-
413-
414356
it('should set url to current location if not specified or empty string', function() {
415357
$backend('JSONP', undefined, null, callback);
416358
expect(fakeDocument.$$scripts[0].src).toBe($browser.url());

0 commit comments

Comments
 (0)