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. The feature has
already landed in the 1.3 branch as 6680b7b, however this alternative version is intended
to implement the feature in an IE8-compatible fashion.

Closes #7031
This commit is contained in:
Caitlin Potter
2014-03-18 11:11:39 -04:00
parent 6bea059109
commit a7ccb7531c
2 changed files with 54 additions and 63 deletions

View File

@@ -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,51 @@ 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 = rawDocument.createElement('script'), callback = null;
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 = function(event) {
removeEventListenerFn(script, "load", callback);
removeEventListenerFn(script, "error", callback);
rawDocument.body.removeChild(script);
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;
}
if (done) {
done(status, text);
}
};
addEventListenerFn(script, "load", callback);
addEventListenerFn(script, "error", callback);
if (msie <= 8) {
script.onreadystatechange = function() {
if (isString(script.readyState) && /loaded|complete/.test(script.readyState)) {
script.onreadystatechange = null;
callback({
type: 'load'
});
}
};
} else {
script.onload = script.onerror = function() {
doneWrapper();
};
}
rawDocument.body.appendChild(script);
return doneWrapper;
return callback;
}
}

View File

@@ -39,7 +39,17 @@ describe('$httpBackend', function() {
fakeDocument = {
$$scripts: [],
createElement: jasmine.createSpy('createElement').andCallFake(function() {
return {};
// msie8 depends on modifying readyState for testing. This property is readonly,
// so it requires a fake object. For other browsers, we do need to make use of
// event listener registration/deregistration, so these stubs are needed.
if (msie <= 8) return {
attachEvent: noop,
detachEvent: noop,
addEventListener: noop,
removeEventListener: noop
};
// Return a proper script element...
return document.createElement(arguments[0]);
}),
body: {
appendChild: jasmine.createSpy('body.appendChild').andCallFake(function(script) {
@@ -356,7 +366,7 @@ describe('$httpBackend', function() {
script.readyState = 'complete';
script.onreadystatechange();
} else {
script.onload();
browserTrigger(script, "load");
}
expect(callback).toHaveBeenCalledOnce();
@@ -372,12 +382,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);
@@ -385,7 +394,7 @@ describe('$httpBackend', function() {
});
if(msie<=8) {
if (msie <= 8) {
it('should attach onreadystatechange handler to the script object', function() {
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop);
@@ -400,42 +409,9 @@ 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() {
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);