1

I'm a javascript/dom noob coming from mainly Python and Clojure, and the semantics of referring to things in the dom are really confusing me.

Take the following extract from some code which I just wrote into a chrome extension to identify a subset of pages matching criteria defined in testdownloadpage and then hang eventlisteners on pdf download links to intercept and scrape them.:

function hanglisteners() {
    if (testdownloadpage(url)) {
        var links = document.getElementsByTagName("a");
        for (i = 0, len = links.length; i < len; i++) {
            var l = links[i];
            if (testpdf(l.href)) {
                l.addEventListener("click", function(e) {
                    e.preventDefault();
                    e.stopPropagation();
                    e.stopImmediatePropagation();
                    getlink(this.href);
                }, false);
            };
        };
    };
};

Originally, I had the call to getlink at the bottom reading getlink(l.href), and I thought (evidently naively) that what would happen with that code would be that it would loop through every matching link, and slap a listener on each that called getlink on the url for that link. But that didn't work at all. I tried replacing l.href with this.href just as a guess, and it started working.

I'm not sure why this.href worked when l.href did not. My best guess is that the javascript interpreter doesn't evaluate l.href in an addEventListener call until some point later, when l has changed to something else(??). But I'm not sure why that should be, or how to know when javascript does evaluate arguments to a function call and when it doesn't...

And now I'm worrying about the higher-up call to testpdf(l.href). That function is meant to check to make sure a link is a pdf download link before hanging the listener on it. But is that going to evaluate l.href within the loop, and hence correctly evaluate each link? Or is that also going to evaluate at some point after the loop, and should I be using this.href instead?

Can some kind soul please explain the underlying semantics to me, so that I don't have to guess whether referring to the loop variable or referring to this is correct? Thanks!

EDIT/ADDITION:

The consensus seems to be that my problem is a (clearly well-known) issue where inner functions in loops are victims of scope leaks s.t. when they're called, unless the inner function closes over all the variables it uses, they end up bound to the last element of the loop. But: why does this code then work?

  var links = document.getElementsByTagName("a");
for (i = 0, len = links.length; i < len; i++) {
  let a = links[i];
  a.addEventListener("click", function(e) {
    e.preventDefault();
    e.stopPropagation();
    e.stopImmediatePropagation();
    console.log(a.href);
  });
};
<html>
<head>
  <title>silly test</title>
</head>
<body>
  <p>
    <a href="link1">Link 1</a>
    <a href="link2">link2</a>
    <a href="link3">link 3</a>
  </p>


</body>
</html>

Based on those answers, I'd expect clicking on every link to log "link 3," but they actually log the correct/expected naive results...

Paul Gowder
  • 2,409
  • 1
  • 21
  • 36
  • Looks like a duplicate of http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example – elclanrs Jul 24 '16 at 16:29
  • When a function is used as an event handler, its `this` is set to the element the event fired from (some browsers do not follow this convention for listeners added dynamically with methods other than addEventListener). – Yosvel Quintero Jul 24 '16 at 16:32
  • hmm @elclanrs are you saying that the problem doesn't have anything to do with the loop, but something having to do with scope leak to the inner function? – Paul Gowder Jul 24 '16 at 16:42
  • ... pondering... just to make sure I understand answers at the link above, an alternative solution would be to wrap `addEventListener` in some kind of 2-arity function that then closes over the current value of `l.href` at each iteration of the loop? – Paul Gowder Jul 24 '16 at 16:49
  • You can find the best answer at [MDN Closures](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Closures). In a nutshell, `var l` changes in the loop and is very different (if exists) at the moment the function is called. Side note: I think about special tag `closures-in-loops`. Is it good idea? – Alex Kudryashev Jul 24 '16 at 16:52
  • So here's a follow-up puzzle. This https://gist.github.com/paultopia/31ca18d2230ac8c236bd130e0b6c31cf works as expected, i.e., clicking each link logs the correct `href`, rather than each logging "link 3." But shouldn't it display the same problem behavior, i.e., `a` is already at the end of the loop when any of the functions are called? – Paul Gowder Jul 24 '16 at 16:57
  • 2
    Read about differences between `var` and `let` [MDN let](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let) for full answer in behavior. – Alex Kudryashev Jul 24 '16 at 17:55
  • Oooooooh. I hadn't even noticed the let, copied from earlier answer below. Thank you? – Paul Gowder Jul 24 '16 at 18:38
  • (That was supposed to be a "thank you!" Also, I didn't know JS had a let! It feels so nice and lispy now!) – Paul Gowder Jul 24 '16 at 19:37

2 Answers2

1

The problem arises because the loop variable has already changed by the time the listener actually fires. You should be able to see this with the example below.

function setup(){
  var root = document.getElementById('root');
  var info = document.createElement('div');
  
  for (var i = 0; i < 5; i++){
    // create a button
    var btn = document.createElement('button');
    
    // set up the parameters
    btn.innerHTML = i
    btn.addEventListener('click', function(event){
      info.innerHTML = ('i = ' + i + ', but the button name is ' +  event.target.innerHTML);
    })
    
    // add the button to the dom
    root.appendChild(btn)
  }
  var info = document.createElement('div');
  info.innerHTML = 'The Value of i is ' + i
  root.appendChild(info)
}


// run our setup function
setup();
<div id="root">
  
</div>

so what you need to do is save off a copy of l for later use. addEventListener is automatically storing the element in this for exactly this reason. Another very good way to do this, however if you don't want to mess with this for some reason is to use a generator function (a function that creates a function).

for instance:

function clickListener(i, info){
    return function(event){
        info.innerHTML = ('i = ' + i + ', but the button name is ' +  event.target.innerHTML);
    }
}

This way we can put the variables i and info into scope for the listener function which is called later without the values changing.

putting this code into our example above, we get the following snippet

function clickListener(i, info){
    return function(event){
        info.innerHTML = ('i = ' + i + ', but the button name is ' +  event.target.innerHTML);
    }
}

function setup(){
  var root = document.getElementById('root');
  var info = document.createElement('div');
  for (var i = 0; i < 5; i++){
    // create a button
    var btn = document.createElement('button');
    
    // set up the parameters
    btn.innerHTML = i
    // use the generator to get a click handler
    btn.addEventListener('click', clickListener(i, info))
    
    // add the button to the dom
    root.appendChild(btn)
  }
  info.innerHTML = 'The Value of i is ' + i
  root.appendChild(info)
}


// run our setup function
setup();
<div id="root">
  
</div>

EDIT: Answering revised question

In your second iteration of code, you use the let keyword introduced with ES6. This keyword is specifically designed to give javascript variables more traditional block scope. In our case it makes the variable scoped to the specific iteration of the loop. A similar example can be seen on MDN. If you can support ES6 in your application/build system, using let is a great way to solve the problem.

Mobius
  • 2,871
  • 1
  • 19
  • 29
0

When you're in the function of event listeners, this is bound to the DOM element for which the event listener is for. Imagine the following code:

var links = document.getElementsByTagName("a");
for (i = 0, len = links.length; i < len; i++) {
  let a = links[i];
  a.addEventListener("click", function(e) {
    console.log(this === a); // => true
  })
}

The two would be identical, so you can use either whenever

bren
  • 4,176
  • 3
  • 28
  • 43
  • hmm... I just tested this to verify for myself it works, but now extra confused as to why my code in the question would work with `this` but fail with the loop variable... (I suppose I could have just had some other bug! might have to start digging through commit history to make sure...) – Paul Gowder Jul 24 '16 at 16:36
  • @PaulGowder, see the question I linked in the comments above, that's the issue: loop + closure. – elclanrs Jul 24 '16 at 16:41
  • @elclanrs I haven't even though of closure. @PaulGowder you should use `this` then – bren Jul 24 '16 at 16:43