Getting the correct href of the clicked link with javascript?

Question

I have a simple html menu:

<div id="menulinks">
  <ul>
    <li><a href="#link1">Link 1</a></li>
    <li><a href="#link2">Link 2</a></li>
    <li><a href="#link3">Link 3</a></li>
    <li><a href="#link4">Link 4</a></li>
  </ul>
</div>

And I need to write a javascript - can't use jQuery - that gives me a href of the clicked anchor link. That's what I did:

var a = document.getElementById("menulinks").getElementsByTagName("a");
for ( var o = 0; o < a.length; o++ ) {
  var clickedLink = a[o];
  clickedLink.addEventListener("click", function() {
    var b = clickedLink.getAttribute("href");
    alert(b);
  });
}

It works, but I always get '#link4' as an answer in the alertbox, no matter which link I click. Can you tell me, what's wrong? Thanks in advance!

Answer 1

Your problem is that clickedLink is getting overwritten with the subsequent value each time the for loop iterates and by the time the link is clicked, clickedLink has already been assigned the last href value.

I'm guessing your confusion results from assuming Javascript has block scoping when it actually uses lexical scoping. (You might read up on variable hoisting too...)

One solution is to bind each value assigned to clickedLink to an Immediately Invoked Function...

for ( var o = 0; o < a.length; o++ ) {
  var clickedLink = a[o];
  (function(clickedLink) { 
    clickedLink.addEventListener("click", function() {
      var b = clickedLink.getAttribute("href");
      alert(b);
    });
  })(clickedLink);
}

Answer 2

You can access the element related to the event with this from within the handler. Otherwise you are referencing the last element that was assigned to clickedLink

Also, you can use querySelectorAll to quickly select the links. But if the links change the list that querySelectorAll returns will not change, so you would have to select the links again. querySelectorAll returns a NodeList, not an HTMLCollection, and therefore cannot be accessed using array methods like forEach, but in this situation it will function perfectly.

var a = document.querySelectorAll("#menulinks a");
for (var o = 0; o < a.length; o++) {
    a[o].addEventListener('click', function () {
        alert(this.href);
    }, false);
}

<div id="menulinks">
    <ul>
        <li><a href="#link1">Link 1</a>
        </li>
        <li><a href="#link2">Link 2</a>
        </li>
        <li><a href="#link3">Link 3</a>
        </li>
        <li><a href="#link4">Link 4</a>
        </li>
    </ul>
</div>

Answer 3

Here's another way around the scoping issue; wrap the loop body in an IIFE. It's basically @sfletche's answer, but "up" one level, and doesn't require the parameter:

var a = document.getElementById("menulinks").getElementsByTagName("a");
for (var o = 0; o < a.length; o++) {
  (function() {
    var clickedLink = a[o];
    clickedLink.addEventListener("click", function() {
      var b = clickedLink.getAttribute("href");
      alert(b);
    });
  })();
}

<div id="menulinks">
  <ul>
    <li><a href="#link1">Link 1</a></li>
    <li><a href="#link2">Link 2</a></li>
    <li><a href="#link3">Link 3</a></li>
    <li><a href="#link4">Link 4</a></li>
  </ul>
</div>

I wouldn't do it this way, but it's an alternative that might be easier to follow if you're just getting into closures and/or IIFEs.

Answer 4

The link clickedLink refers to is getting overwritten with each iteration and by the time the link is clicked it refers to the last link.

You can fix it very simply by changing

var b = clickedLink.getAttribute("href");

to

var b = this.getAttribute("href");

then it will work:

var a = document.getElementById("menulinks").getElementsByTagName("a");
for ( var o = 0; o < a.length; o++ ) {
  var clickedLink = a[o];
  clickedLink.addEventListener("click", function() {
    var b = this.getAttribute("href");
    alert(b);
  });
}

<div id="menulinks">
  <ul>
    <li><a href="#link1">Link 1</a></li>
    <li><a href="#link2">Link 2</a></li>
    <li><a href="#link3">Link 3</a></li>
    <li><a href="#link4">Link 4</a></li>
  </ul>
</div>

Or if you want to use the new ES6 features (which work in some modern browsers but not all) then you can do:

<div id="menulinks">
  <ul>
    <li><a href="#link1">Link 1</a></li>
    <li><a href="#link2">Link 2</a></li>
    <li><a href="#link3">Link 3</a></li>
    <li><a href="#link4">Link 4</a></li>
  </ul>
</div>
<script type="application/javascript;version=1.7">
var a = document.getElementById("menulinks").getElementsByTagName("a");
for ( var o = 0; o < a.length; o++ ) {
  let clickedLink = a[o];
  clickedLink.addEventListener("click", function() {
    alert( clickedLink.getAttribute("href") );
  });
}
</script>

Answer 5

You've just discovered variable hoisting. Here's what the interpreter thinks your code looks like.

var a = document.getElementById("menulinks").getElementsByTagName("a");
var clikedLink, o;
for ( o = 0; o < a.length; o++ ) {
  clickedLink = a[o];
  clickedLink.addEventListener("click", function() {
    var b = clickedLink.getAttribute("href");
    alert(b);
  });
} 

Since clickedLink is closed over in the event listener, it will continue to get overwritten on every interation of the loop, hence why every link gives the href of the last a tag, because it is the last a tag.

Here's how you could do it correctly

var a = document.getElementById("menulinks").getElementsByTagName("a");
for ( var o = 0; o < a.length; o++ ) {
  a[o].addEventListener("click", function() {
    var b = this.getAttribute("href");
    alert(b);
  }.bind(a[o]));
}

Or if you can't use Function.prototype.bind:

var a = document.getElementById("menulinks").getElementsByTagName("a");
for ( var o = 0; o < a.length; o++ ) {
  (function(l) {
    l.addEventListener("click", function() {
    var b = l.getAttribute("href");
    alert(b);
  })(a[o]);
}