JS onclick not firing correctly

I have the following function. The problem is that instead of waiting for the user to click the image as expected, the function immediately fires the imgReplace function for each element in the images array.

Have I done something wrong? Could the fact I'm using a separate Javascript routine based on Jquery be relevant here?

function开发者_如何转开发 setup () {
    var images = document.getElementById("mycarousel");
    images = images.getElementsByTagName("img");

    for (var i = 0; i< images.length; i++) {
        images[i].onclick = imgReplace (images[i]);
    }

}

Wow I just fixed this embarrassing bug in some of my own code. Everybody else has gotten it wrong:

images[i].onclick = function() {imgReplace(images[i]);};

won't work. Instead, it should be:

images[i].onclick = (function(i) { return function() { imgReplace(images[i]); }; })(i);

Paul Alexander's answer is on the right track, but you can't fix the problem by introducing another local variable like that. JavaScript blocks (like the {} block in the "for" loop) don't create new scopes, which is a significant (and non-obvious) difference from Java or C++. Only functions create scope (setting aside some new ES5 features), so that's why another function is introduced above. The "i" variable from the loop is passed in as a parameter to an anonymous function. That function returns the actual event handler function, but now the "i" it references will be the distinct parameter of the outer function's scope. Each loop iteration will therefore create a new scope devoted to that single value of "i".

Your assigning the result of the call to imageReplace to the onclick handler. Instead wrap the call to imageReplace in it's own function

images[i].click = function(){ imgReplace( images[i] ) }

However, doing so will always replace the last image. You need to create a new variable to enclose the index

for (var i = 0; i< images.length; i++) {
    var imageIndex = i;
    images[i].onclick = function(){ imgReplace (images[imageIndex]); }
}

What you want to do here is:

images[i].onclick = function() {imgReplace(images[i]);}

try that.

Cheers