Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 39 additions & 24 deletions src/ng/httpBackend.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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;

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 && msie <= 8) {
if (msie <= 8) {
script.onreadystatechange = function() {
if (/loaded|complete/.test(script.readyState)) {
doneWrapper();
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;
}
}
52 changes: 14 additions & 38 deletions test/ng/httpBackendSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -356,7 +366,7 @@ describe('$httpBackend', function() {
script.readyState = 'complete';
script.onreadystatechange();
} else {
script.onload();
browserTrigger(script, "load");
}

expect(callback).toHaveBeenCalledOnce();
Expand All @@ -372,20 +382,19 @@ 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) {
if (msie <= 8) {

it('should attach onreadystatechange handler to the script object', function() {
$backend('JSONP', 'http://example.org/path?cb=JSON_CALLBACK', null, noop);
Expand All @@ -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);
Expand Down