[OpenLayers-Dev] Possible bug with Protocol.HTTP

[Stephen Woodbridge]

Hi Devs,

I have been working with OL.Protocol.HTTP and I think I have identified 
a bug in the code after spending considerable time reading and tracing 
the code via firebug. Basically if you create a Protocol.HTTP with a 
user callback, it never gets called. Below is a summary of my analysis 
and I would appreciate it if one of you can confirm or refute these 
observations.

In OpenLayers.Protocol.HTTP.commit() near the end the code starts 
issuing requests with:

         var queue = types[OpenLayers.State.INSERT];
         if(queue.length > 0) {
             resp.push(this.create(
                 queue, OpenLayers.Util.applyDefaults(
                     {callback: insertCallback, scope: this}, options.create
                 )
             ));
         }
         queue = types[OpenLayers.State.UPDATE];
         for(var i=queue.length-1; i>=0; --i) {
             resp.push(this.update(
                 queue[i], OpenLayers.Util.applyDefaults(
                     {callback: callback, scope: this}, options.update
                 ))
             );
         }
         queue = types[OpenLayers.State.DELETE];
         for(var i=queue.length-1; i>=0; --i) {
             resp.push(this["delete"](
                 queue[i], OpenLayers.Util.applyDefaults(
                     {callback: callback, scope: this}, options["delete"]
                 ))
             );
         }

I think the arguments to OpenLayers.Util.applyDefaults(to, from) are 
reversed in the above code. I think the intent here was to create a hash 
like:

     options[requestType] = {callback: callback, scope: this}

that would get passed as the options array to to the create, update, 
delete method respectively because this is what is expected later by the 
OpenLayers.Protocol.HTTP.callUserCallback. Also not the callback() and 
insertCallback() are local function to commit() and should not be 
confused with this.callback.

And in fact the above code might be more correctly expressed as for example:

         queue = types[OpenLayers.State.DELETE];
         for(var i=queue.length-1; i>=0; --i) {
             var opt = OpenLayers.Util.applyDefaults(options, "delete: 
{callback: callback, scope: this});
             resp.push(this["delete"](queue[i], opt)
             );
         }

It would seem, this would more accurately reflect the usage below in 
callUserCallback():

     callUserCallback: function(resp, options) {
         var opt = options[resp.requestType];
         if(opt && opt.callback) {
             opt.callback.call(opt.scope, resp);
         }
     },

In current code (OL 2.8 and OL 2.9) opt is currently always undefined so 
the callback never gets called.

A simple test of this is to create an OpenLayers.Protocol.HTTP with a

     callback: function() {alert( "Hello World!"); }

and issue a create, update, or delete request via the protocol and the 
callback is not currently called.

I admit to a fledgling understanding of OpenLayers but I have been 
trying to get my head around why the callback is not getting called. My 
next step is to try and patch the "start issuing requests" block of code 
and see if I can get it to work with a patch which I'll report back on, 
but it would be nice if a dev can look at this analysis and provide some 
feedback on it.

All the best,
   -Steve