Passing argument by value to onClick function

Question

I can't figure out how to pass an argument by value in JavaScript to onClick-function. I have been trying with closure functions but it doesn't seem to work!! aah

...

for(var i=0; i<=15; i++){
    blockNr++;
    meal = getMealByCategory(category, i);
    place.innerHTML += "<div class='container-fluid' name='blockName' id='blockName" +
    blockNr + "' onClick='addPrice(meal, 20)'></div>"; //here, always the last meal is sent

    var itemPlace = document.getElementById("blockName"+blockNr);
    itemPlace.innerHTML = meal; //this works fine for all meals
}

Use dom manipulation instead of html injection. `document.createElement("div")`... — brso05, Dec 09 '14 at 15:01
Your onclick is binding to the meal variable at the end of the loop, not the value of meal for each iteration, which is why the last meal is always shown. There are plenty of examples of how to assign event handlers in loops to handle this. — Patrick, Dec 09 '14 at 15:02
Have you considered using jQuery? You could neaten this up quite a bit, and add a click event: $("#blockName").on("click", addPrice(meal, 20)); — Hywel Rees, Dec 09 '14 at 15:04

six fingered man · Accepted Answer · 2014-12-09T15:18:04.473

What you're doing is creating a global variable and overwriting it every time you add to the .innerHTML. Code in a string can't reference local variables, so what you want isn't possible.

One option would be to concatenate the value into the string.

place.innerHTML += "<div class='container-fluid' name='blockName' id='blockName" +
blockNr + "' onClick='addPrice(\"" + meal + "\", 20)'></div>";

This should work, but is pretty unpleasant. Instead, create your elements with DOM creation methods, and assign a function to the onclick property. That function will be able to reference the local meal.

And be sure to create a local scope, and declare meal with var.

var meal; 

for(var i=0; i<=15; i++) {
    meal = getMealByCategory(category, i);
    createDiv(meal);
}

function createDiv(meal) {
    blockNr++;
    var div = document.createElement("div");
    div.className = "container-fluid"
    div.name = "blockName"
    div.id = "blockName" + blockNr;
    div.onclick = function() {
        addPrice(meal, 20);
    }
    place.appendChild(div);
}

Not sure what `meal` is, but this will likely lead to XSS problems — Bergi, Dec 09 '14 at 15:06
@Bergi: Likely? That's overstating it. But either way, I indicated that DOM creation would be preferred. — six fingered man, Dec 09 '14 at 15:09

score 1 · Answer 2 · answered Dec 09 '14 at 15:03

1

That is because by the time you run the on-click event the loop has fully executed and the value for meal is static as the last calculated value. If this needs to change for each element, you should pass the parameter value to the function, not pass the variable name. So something like:

"' onClick='addPrice('" + meal + "', 20)'></div>";

answered Dec 09 '14 at 15:03

Mike Brant

70,514
10
99
103

Not sure what `meal` is, but this will likely lead to XSS problems – Bergi Dec 09 '14 at 15:07
@Bergi `meal` was the variable that is declared a couple of lines before this. – Mike Brant Dec 09 '14 at 16:03
Yes, but what is its value? Seems to be some HTML. – Bergi Dec 09 '14 at 16:08
@Bergi Who knows. Whatever `getMealByCategory()` returns. I don't know how you would think this is HTML, as there is no evidence of that. – Mike Brant Dec 09 '14 at 16:43
I thought `itemPlace.innerHTML = meal; //this works fine for all meals` would be evidence enough :-) But yes, whenever we don't know what it is we need to escape it properly. – Bergi Dec 09 '14 at 16:45
@Bergi: ..what kind of xss problems? function getMealByCategory(category, mealNr){ var meals = getMealsByCategory(category); /* another js function, calls to php function with jquery ajax, works fine */ var meal = meals[mealNr]; return meal; } – user3857472 Dec 10 '14 at 07:49
@user3857472: If the `meal` contains an `'` (which ends the string literal) or an `"` (which ends the html attribute), you'll get invalid markup. And depending on who creates the meals, that will be a security hole. – Bergi Dec 10 '14 at 15:15

Passing argument by value to onClick function

2 Answers2