views:

226

answers:

2

I want to add a Listener event to each generated marker, so that when you click a marker you are redirected to the permalink url. With the code below the permalink value is the same for every marker(it get's the last value). I've read about closure problems and that seems to be what I'm having. I don't really get the examples I've looked at though.

Can somebody take a look at my code and point me in the right direction? Any help is greatly appreciated!

downloadUrl("http://localhost/map/generatexml.php", function(data) {
            var xml = parseXml(data);
            var markers = xml.documentElement.getElementsByTagName("marker");
            for (var i = 0; i < markers.length; i++) {
              var permalink = markers[i].getAttribute("permalink");
              var point = new google.maps.LatLng(
                  parseFloat(markers[i].getAttribute("lat")),
                  parseFloat(markers[i].getAttribute("lng")));
              var marker = new google.maps.Marker({map: map,position: point,icon: icon.icon,shadow: icon.shadow,title: name});
              google.maps.event.addListener(marker, 'click', function() {self.location.href = permalink;});
            }
A: 

JavaScript only has function scope, so when you're declaring var's in the middle of the for-loop you're in fact only declaring them once for the entire function. First thing you should do is bring all of the var declarations to the top to make some more sense of what's going on:

downloadUrl("http://localhost/map/generatexml.php", function(data) {
            var xml = parseXml(data);
            var markers = xml.documentElement.getElementsByTagName("marker");
            var i, permalink, point, marker;
            for (i = 0; i < markers.length; i++) {
              permalink = markers[i].getAttribute("permalink");
              point = new google.maps.LatLng(
                  parseFloat(markers[i].getAttribute("lat")),
                  parseFloat(markers[i].getAttribute("lng")));
              marker = new google.maps.Marker({map: map,position: point,icon: icon.icon,shadow: icon.shadow,title: name});
              google.maps.event.addListener(marker, 'click', function() {self.location.href = permalink;});
            }

Your click function will use the final value of permalink for every marker event since permalink gets replaced with a new value every time the for-loop loops.

Edit: So yeah, Pointy beat me to actually solving your problem. But there you have, at least, an explanation of the problem

Bob
Thanks Bob for helping me out, you're awesome :)
Daniel Westman
+5  A: 

Try this:

for (var i = 0; i < markers.length; ++i) {
  // ... like what you have already ...
  (function(permalink) {
    google.maps.event.addListener(marker, 'click', function() {self.location.href = permalink;});
  })(permalink);
}

By making a new lexical scope with a copy of the "permalink" value at each iteration, your handlers should work better.

Pointy
Why was I struggling to come up with so simple a solution? I'm jealous
Bob
Thanks Pointy, I've been trying to get this to work for hours. Your code worked and you're my hero!
Daniel Westman