Parse for links that have target="new" and add an onclick event

Question

I have a web site that gets links generated dynamically. I would like to see if I can add an onclick event handler to track external links. I am looking to see links that has target="new" ( which means external to our site) and add the event handler

html code

 <a target="new" href="http://twitter.com/cnn">CNN</a>

The code I tried to test is not working. Let me know what is wrong in my code or should I some home append the onclick event to the external links?

Js code

var links = document.getElementsByTagName("a");
for (var i=0; <links.length; i++) { 
    if(links[i].target == 'new'){   
        links[i].onclick = function() {
            alert("Added onClick: " + links[i].href);        
        }
    }
}

possible duplicate of [Access outside variable in loop from Javascript closure](http://stackoverflow.com/questions/1331769/access-outside-variable-in-loop-from-javascript-closure) not an exact dup but the problem is the same. This question pops up dozens of times daily. — Matt Ball, Aug 03 '12 at 23:26

Musa · Answer 1 · 2012-08-03T23:33:02.433

1

You can use this in your onclick function and avoid the closure issue

var links = document.getElementsByTagName("a");
for (var i=0; i<links.length; i++) { 
    if(links[i].target == 'new'){   
        links[i].onclick = function(){  
            alert("Added onClick: " + this.href);           
        }
    }
}

edited Aug 03 '12 at 23:33

answered Aug 03 '12 at 23:27

Musa

96,336
17
118
137

Youre missing 'i – Reimeus Aug 03 '12 at 23:32
You might as well add the same function each time in that case, instead of creating a new clone each iteration. – Neil Aug 03 '12 at 23:40
Good point Neil. Although it's probably easier to to keep the code as close to the asker's original code so he/she will be able to understand the change easier. – emurano Aug 03 '12 at 23:53

emurano · Accepted Answer · 2013-01-25T11:26:33.077

Another answer here is what you should go with (using this) but it's worth addressing the issue of closures in for loops.

If you want to use a variable that changes for each iteration in a for loop in a closure that's created in that for loop, define and call an anonymous function that returns a function to be bound (binded?) to the onclick event.

var links = document.getElementsByTagName("a");
for (var i=0; i<links.length; i++) { 
    if(links[i].target == 'new'){   
        links[i].onclick =
            function (obj) {
                return function(event) {
                    alert("Added onClick: " + obj.href);        
                }                   
            }(links[i]);
    }
}

Since the parameter (obj) to the anonymous function is passed by value it won't change in subsequent iterations for the for loop. The returned function will have its own copy of the object.

A lot of stuff in Javascript starts making sense when you think of functions as objects that can be passed around.

line 2 should be: for (var i=0; i < links.length; i++) { – Charles Wesley Jan 24 '13 at 23:56 — Charles Wesley, Jan 24 '13 at 23:56

score 0 · Answer 3 · answered Aug 03 '12 at 23:34

0

are you adverse to jQuery?

$("a[target='new']").click(function(){alert($(this).attr("href"));});

answered Aug 03 '12 at 23:34

AceCorban

2,023
1
15
15

I love jQuery but it's worth having pure Javascript answers for people that don't use it. – emurano Aug 03 '12 at 23:51
No doubt. I merely mentioned it because, working on a collection of dom elements using a selector is what jQuery does best. So a pure javascript solution will have to iterate through the elements manually, as has been suggested by others. $0.02 – AceCorban Aug 03 '12 at 23:53

Parse for links that have target="new" and add an onclick event

3 Answers3