Strange behavior of marker events in OpenLayer

Question

I have markers layer on my map.

Every time I add a new marker I register it to a mouse-click event:

var lonlat = new OpenLayers.LonLat(lon,lat);
var marker = new OpenLayers.Marker(lonlat,icon);
marker.id = callId;

marker.events.register("mousedown", marker, function() {AddPopup(marker.id);});

callMarkers.addMarker(marker);

Sometimes I want to disable/enable the event. so I use these functions:

function EnableAllMarkers()
{ 
    for (var i in callMarkers.markers)
    {
        callMarkers.markers[i].events.remove("mousedown");               

        callMarkers.markers[i].events.register("mousedown", callMarkers.markers[i],   

        function() { AddPopup(callMarkers.markers[i].id); });
    }  
}


function DisableAllMarkers()
{ 
    for (var i in callMarkers.markers)
    {
        callMarkers.markers[i].events.remove("mousedown");
    }  
}

When I use this code I get strange behavior - sometimes a popup opens for the wrong marker.

I click on marker X and popup Y opens.

Can someone help me, please?

Note:
The reason EnableAllmMarkers first removes the event is because we don't know if DisableAllmMarkers was ever called since a new marker was added. if it was called indeed, remove function will do nothing.

Answer 1

This is a classic JavaScript trap: you're instantiating functions as event handlers in a loop, and the functions refer to a local variable. The problem is that all of them refer to the same local variable: the same, single, unique, only-one-place-in-memory variable. The variable in this case is "i".

At the end of that for loop, "i" will have the value of the last key in the object (and, by the way, if callMarkers.markers is really an array, then this shouldn't be a for ... in loop anyway, but that's a separate issue). When those events finally fire, therefore, all the handlers will do their thing with "i" equal to that one same key.

To fix:

  for (var i in callMarkers.markers)
    {
        callMarkers.markers[i].events.remove("mousedown");               

        callMarkers.markers[i].events.register(
          "mousedown", 
          callMarkers.markers[i],
          (function(ii) {
            return function() {
              AddPopup(callMarkers.markers[ii].id);
            }
          )(i)
         );
    } 

That introduces an intermediary anonymous function. That function is immediately called, and passed the current value of "i". By doing that — passing "i" as an argument to the anonymous function — the value is "captured" in the argument "ii". Each loop iteration will cause another invocation of the anonymous function, and the function it returns (the actual handler) will have access to its own private "ii" variable.

There are some other ways to achieve the same thing, but they're all just variations on this theme.