onClick does not work properly on p tag

Question

Hi,

I want to bind a click event to every <p> but it does not seem to work properly.

When I run the script I instantly get three alerts. I only want to get them when clicking any of the three <p>'s.

Can anyone tell me what I'm doing wrong?

Edit: Sry this is what it looks like. The HTML is just as it should: <p class="special">text text text</p> etc

(function () {
    var myFunction = function (theP) {
      alert(theP.id)   
     }


    window.onload = function () {    
     var pTags = document.getElementsByTagName('p'),
      pLength = pTags.length,
       i;

     for (i = 0; i < pLength; i += 1) {
      if(pTags[i].className == 'special'){

       pTags[i].onClick = myFunction(pTags[i]);

      }
     };
    }
})();

Ps. I cannot use jQuery atm

Answer 1

Try replacing

pTags[i].onClick = myFunction(pTags[i]);

with

pTags[i].onClick = function() { myFunction(pTags[i]); }

You see, when you assign to the onClick of an object, you're copying the result of the expression to it. What your supposed to copy is a function to call when the p is clicked.

Instead, you're running the command myFunction(pTags[i]), which executes the alert()s, and takes the result of the function. Now, since the function doesn't return anything, the value of the expression myFunction(pTags[i]) is undefined.

And you take that value, and assign it to onClick. So basically what you've done is:

For each "special" paragraph:

1. Execute alert
2. Assign undefined to the paragraph's onClick.

Answer 2

myFunction would need to return a function for your code to work

Below someone mentions an option that tries to create an inline function, but because closure binding happens with variables, not values, this will not work (The i in the code will be modified after including it in the function).

There are two obvious ways to create the function and have it work. One is to create a per-handler function... The other is using this to get the element.

(function () {
    var myFunction = function (theP) {
            return function() {
                alert(theP.id);
            }                 
        }


    window.onload = function () {                       
        var pTags = document.getElementsByTagName('p'),
                pLength = pTags.length,
                i;

        for (i = 0; i < pLength; i += 1) {
                if(pTags[i].className == 'special'){

                        pTags[i].onClick = myFunction(pTags[i]);

                }
        };
    }
})();

And with this

(function () {
    var myFunction = function () {
                alert(this.id)                  
        }


    window.onload = function () {                       
        var pTags = document.getElementsByTagName('p'),
                pLength = pTags.length,
                i;

        for (i = 0; i < pLength; i += 1) {
                if(pTags[i].className == 'special'){

                        pTags[i].onClick = myFunction;

                }
        };
    }
})();

The second format is the preferred format, as it uses much less memory to implement by the browser.

Answer 3

.onclick is case sensitive, isn't it?

Answer 4

You are calling your function. When you use the parenthesis you call your function.

onclick requires a function reference:

You could modify your code this way:

myFunction(theP){
 return function(){alert(theP.id)};
}

This would insure that your call returns a function reference instead of a value.

Answer 5

Thanks for all answers. Much appreciated!
I rewrote my own script and solved it like this: How does that look?

(function () {
    var myFunction1 = function (theP) {
            theP.onclick = myFunction2;
        },

        myFunction2 = function(){
            alert('hello world')
        }


    window.onload = function () {    
     var pTags = document.getElementsByTagName('p'),
      pLength = pTags.length,
      i;

     for (i = 0; i < pLength; i += 1) {
      if(pTags[i].className == 'special'){
       myFunction1(pTags[i]);
      }
     }
    }
})();