dynamically adding onclick to <a href>

[gwest]

I'm finishing up a beginning JavaScript class and am having trouble with an assignment. What I have is a set of links:

<div id="divWebsites">
   <a href="http://www.yahoo.com" title="Yahoo!">Yahoo!</a> | 
   <a href="http://www.cnn.com" title="CNN">CNN</a> | 
   <a href="http://abcnews.go.com/" title="ABC">ABC News</a> | 
   <a href="http://online.wsj.com/home-page" title="WSJ">Wall Street Journal</a> | 
   <a href="http://www.bbc.co.uk/" title="BBC">BBC</a>
</div>

In my .js file I am getting each url and title and trying to dynamically add an onclick event to open each url in a new window:

window.onload=setupLinks;

function setupLinks()
{
	var webSites = document.getElementById("divWebsites");
	if (webSites)
	{
		//assign window.open event to divWebsites links
		var links = webSites.getElementsByTagName("a");
		var i, href, title;
		for (i=0; i<links.length; i++)
		{
			href = links[i].getAttribute("href");
			title = links[i].getAttribute("title");
			links[i].onclick = function(){window.open(href,title); return false;};
		}
	}	
}

Everything works fine but the url assigned to every link is the BBC url. I know I am getting each url/title set but I don't understand why only the last url gets assigned to EVERY link.

Thank you to anyone that can help. I'm sure there is a simple solution for anyone more experienced than I am.

[ApacheTech]

Have I missed something really basic here, or are you using JS to simply mimic the normal click event of the hyperlinks?

If it all works, the script will do nothing at all.

I just don't understand why any of the javascript is needed.

[jscheuer1]

The problem is that:

links[i].onclick = function(){window.open(href,title); return false;};

Refers back to:

		var links = webSites.getElementsByTagName("a");
		var i, href, title;

So, whatever it's value, is what it will be. During the execution of the loop it's value is each of the link's hrefs in turn. But once the loop is finished, it's value is:

http://www.bbc.co.uk/

So whenever any of the onclick functions executes, that's the value for the variable href.

Here's what I would do. Well not exactly, I would change things a lot. But if I were only to to change what needs to be changed to get the existing code working I would:

window.onload=setupLinks;

function setupLinks()
{
	var webSites = document.getElementById("divWebsites");
	if (webSites)
	{
		//assign window.open event to divWebsites links
		var links = webSites.getElementsByTagName("a"), i;
		for (i=0; i<links.length; i++)
		{
			(function(i){
				var href = links[i].getAttribute("href");
				var title = links[i].getAttribute("title");
				links[i].onclick = function(){window.open(href,title); return false;};
			})(i);
		}
	}
}

In that way a self executing anonymous function is run for each link, assigning a unique onclick function with unique href and title variables.

But self executing anonymous functions are a little advanced and sometimes it's hard to grasp what's happening. This version which accomplishes the same thing might be clearer:

window.onload=setupLinks;

function setupLinks()
{
	var webSites = document.getElementById("divWebsites");
	if (webSites)
	{
	function assignCilcks(link){
		var href = link.getAttribute("href");
		var title = link.getAttribute("title");
		link.onclick = function(){window.open(href,title); return false;};
	}
		//assign window.open event to divWebsites links
		var links = webSites.getElementsByTagName("a"), i;
		for (i=0; i<links.length; i++)
		{
			assignCilcks(links[i]);
		}
	}
}

There's a simpler way though if you take advantage of the meaning of the keyword this in the scope of an event assigned in this manner:

window.onload=setupLinks;

function setupLinks()
{
	var webSites = document.getElementById("divWebsites");
	if (webSites)
	{
		//assign window.open event to divWebsites links
		var links = webSites.getElementsByTagName("a"), i;
		for (i=0; i<links.length; i++)
		{
			links[i].onclick = function(){window.open(this.href,this.title); return false;};
		}
	}
}

Still not how I would do it, but we're getting closer. But I'll leave it at that because I think I've answered the question.

[gwest]

Yes, ApacheTech - that is exactly what I am trying to do. The assignment I am working on specifies that, through a function call, a link is opened in a new window. I understand that there are better ways to do it (including no JS at all!) but that is the assignment. My intention was to try to separate the javascript from the html completely (just trying to learn something) and I know I could do the following and it would be fine for the assignment:

function openWindow(url){
window.open(url);
}

<a href="http://www.somewebsite.com" title="website" onclick="openWindow('http://www.somewebsite.com'); return false;">Click for website.</a>

Thanks for your code help and explanation, jscheuer1!

I still don't understand why when this line of code is reached

links[i].onclick = function(){window.open(href,title); return false;};

that the correct href and title are not assigned to that particular link for the onclick event. I understand when you say

So, whatever it's value, is what it will be. During the execution of the loop it's value is each of the link's hrefs in turn. But once the loop is finished, it's value is:

http://www.bbc.co.uk/

So whenever any of the onclick functions executes, that's the value for the variable href.

but I don't understand why when the links aren't clicked until after the page has loaded and supposedly all of the onlick events have been assigned with the appropriate urls and titles.

I had tried using this. like in your last solution but wasn't able to get it to work. I'm wondering if it was because I was still assigning the href and title values before assigning the onclick event.

Your last solution when tried in Firefox works fine but in IE, when the first link is clicked (and only the first link - all others work fine) the status bar shows a quick "Error on page" before opening the new page in the same window. Any idea why that would be?

[ApacheTech]

window.onload=setupLinks;

function setupLinks()
{
    var webSites = document.getElementById("divWebsites");
    if (webSites)
    {
        //assign window.open event to divWebsites links
        var links = webSites.getElementsByTagName("a");
        var i, href, title;
        for (i=0; i<links.length; i++)
        {
            href = links[i].getAttribute("href");
            title = links[i].getAttribute("title");
            links[i].onclick = function(){window.open(href,title); return false;};
        }
    }    
} 

When you run this code, the variable is declared:

var i, href, title; 

And then you add data to that variable. Each iteration of the for loop changes the value of href and title and correctly adds the onclick...

        for (i=0; i<links.length; i++)
        {
            href = links[i].getAttribute("href");
            title = links[i].getAttribute("title");
            links[i].onclick = function(){window.open(href,title); return false;};
        } 

However, once the script has finished, the value of href is still the last one placed in it and the onclick method still point to that variable. It's all about the scope of the variable. With your method, href and title are local to the function as a whole; the variables are created, used multiple times and only disposed of when the function ends.

        for (i=0; i<links.length; i++)
        {
            var href = links[i].getAttribute("href");
            var title = links[i].getAttribute("title");
            links[i].onclick = function(){window.open(href,title); return false;};
        } 

This method however reduces the scope of the href and title variables to local within the for loop. With each iteration a variable is created, used then disposed and a new part of memory is used for the next iteration.

[ApacheTech]

The IE problem may possibly have something to do with IE's handling of dynamically adding the onload attribute to the page. It can be temperamental

You could try:

document.body.onload = setupLinks; 

A full cross browser compliant method would be:

if (document.addEventListener) {
    document.body.addEventListener ("load",setupLinks,false);
} else if (el.attachEvent) {
    document.body.attachEvent ("load",setupLinks);
} else {
    document.body.onload = setupLinks;
} 

[gwest]

Thanks for the help, ApacheTech. I greatly appreciate it!