From 496b7d757b12b1a8b26d735e7f45795f5fb64c79 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Wed, 26 Jun 2013 16:27:52 -0400 Subject: [PATCH 1/5] Simplify OpenSeadragon.makeAjaxRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Since async is always true – and browsers are starting to deprecate synchronous XHR – we were able to prune considerable amount of code * Add an error callback to match the existing success callback --- src/openseadragon.js | 59 +++++++++++++------------------------------- 1 file changed, 17 insertions(+), 42 deletions(-) diff --git a/src/openseadragon.js b/src/openseadragon.js index 87e15e03..905259a9 100644 --- a/src/openseadragon.js +++ b/src/openseadragon.js @@ -1306,64 +1306,39 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ * @function * @name OpenSeadragon.makeAjaxRequest * @param {String} url - the url to request - * @param {Function} [callback] - a function to call when complete + * @param {Function} onSuccess - a function to call on a successful response + * @param {Function} onError - a function to call on when an error occurs * @throws {Error} */ - makeAjaxRequest: function( url, callback ) { + makeAjaxRequest: function( url, onSuccess, onError ) { + var request = $.createAjaxRequest(); - var async = true, - request = $.createAjaxRequest(), - options; + request.onreadystatechange = function() { + // 4 = DONE (https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#Properties) + if ( request.readyState == 4) { + request.onreadystatechange = function(){}; - - if( $.isPlainObject( url ) ){ - options.async = options.async || async; - }else{ - options = { - url: url, - async: $.isFunction( callback ), - success: callback, - error: null - }; - } - - if ( options.async ) { - /** @ignore */ - request.onreadystatechange = function() { - // 4 = DONE (https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#Properties) - if ( request.readyState == 4) { - request.onreadystatechange = function(){}; - options.success( request ); + if (request.status == 200) { + onSuccess( request ); + } else { + onError( request ); } - }; - } + } + }; try { - request.open( "GET", options.url, options.async ); + request.open( "GET", url, true ); request.send( null ); } catch (e) { - $.console.log( - "%s while making AJAX request: %s", - e.name, - e.message - ); + $.console.log("%s while making AJAX request: %s", e.name, e.message); request.onreadystatechange = function(){}; request = null; - if ( options.error && $.isFunction( options.error ) ) { - options.error( request ); - } + onError(request, e); } - - if( !options.async && $.isFunction( options.success ) ){ - options.success( request ); - } - - return options.async ? null : request; }, - /** * Taken from jQuery 1.6.1 * @function From 5eccdfee9801bd19bec52695136fe8819b9fee8e Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Wed, 26 Jun 2013 17:07:37 -0400 Subject: [PATCH 2/5] Better match project style --- src/openseadragon.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/openseadragon.js b/src/openseadragon.js index 905259a9..5acc4468 100644 --- a/src/openseadragon.js +++ b/src/openseadragon.js @@ -1315,10 +1315,10 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ request.onreadystatechange = function() { // 4 = DONE (https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#Properties) - if ( request.readyState == 4) { + if ( request.readyState == 4 ) { request.onreadystatechange = function(){}; - if (request.status == 200) { + if ( request.status == 200 ) { onSuccess( request ); } else { onError( request ); @@ -1335,7 +1335,7 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ request.onreadystatechange = function(){}; request = null; - onError(request, e); + onError( request, e ); } }, From 1c6a57c7100081318a54f8ed6a5d359cd7a0365f Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Wed, 26 Jun 2013 17:17:50 -0400 Subject: [PATCH 3/5] makeAjaxRequest: log all errors, test callback * All AJAX errors will log to the console * The onError callback will only be called if defined --- src/openseadragon.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/openseadragon.js b/src/openseadragon.js index 5acc4468..2a6e2689 100644 --- a/src/openseadragon.js +++ b/src/openseadragon.js @@ -1321,7 +1321,11 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ if ( request.status == 200 ) { onSuccess( request ); } else { - onError( request ); + $.console.log("AJAX request returned %s: %s", request.status, url); + + if ($.isFunction(onError)) { + onError( request ); + } } } }; @@ -1335,7 +1339,9 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ request.onreadystatechange = function(){}; request = null; - onError( request, e ); + if ($.isFunction(onError)) { + onError( request, e ); + } } }, From db38b71dd9994e7d27e957cc05e59ed08cd0d758 Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Thu, 27 Jun 2013 16:02:17 -0400 Subject: [PATCH 4/5] makeAjaxRequest: throw error for missing callback --- src/openseadragon.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/openseadragon.js b/src/openseadragon.js index 2a6e2689..12c11ade 100644 --- a/src/openseadragon.js +++ b/src/openseadragon.js @@ -1313,6 +1313,10 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ makeAjaxRequest: function( url, onSuccess, onError ) { var request = $.createAjaxRequest(); + if (!$.isFunction(onSuccess)) { + throw new Error( "makeAjaxRequest requires a success callback" ); + } + request.onreadystatechange = function() { // 4 = DONE (https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest#Properties) if ( request.readyState == 4 ) { From 464428cef723a94ce548878c5673734e963856ca Mon Sep 17 00:00:00 2001 From: Chris Adams Date: Fri, 28 Jun 2013 13:59:59 -0400 Subject: [PATCH 5/5] makeAjaxRequest: don't set request to null Setting onreadystatechange can avoid the possibility of it somehow being called twice but there's no point in nulling the request right before we let the GC collect it. --- src/openseadragon.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/openseadragon.js b/src/openseadragon.js index 12c11ade..4f4e9752 100644 --- a/src/openseadragon.js +++ b/src/openseadragon.js @@ -1341,7 +1341,6 @@ window.OpenSeadragon = window.OpenSeadragon || function( options ){ $.console.log("%s while making AJAX request: %s", e.name, e.message); request.onreadystatechange = function(){}; - request = null; if ($.isFunction(onError)) { onError( request, e );