views:

3722

answers:

4

Hello. I am having problems with my JScript code. I am trying to loop through all of the rows in a table and add an onclick event. I can get the onclick event to add but have a couple of problems.

The first problem is that all rows end up getting set up with the wrong parameter for the onclick event.

The second problem is that it only works in IE.

Here is the code excerpt...

shanesObj.addTableEvents = function(){
table = document.getElementById("trackerTable");
for(i=1; i<table.getElementsByTagName("tr").length; i++){
 row = table.getElementsByTagName("tr")[i];
 orderID = row.getAttributeNode("id").value;
 alert("before onclick: " + orderID);
 row.onclick=function(){shanesObj.tableRowEvent(orderID);};
}}

shanesObj.tableRowEvent = function(orderID){
alert(orderID);}

The table is located at the following location...

http://www.blackcanyonsoftware.com/OrderTracker/testAJAX.html

The id's of each row in sequence are... 95, 96, 94...

For some reason, when shanesObj.tableRowEvent is called, the onclick is set up for all rows with the last value id that went through iteration on the loop (94).

I added some alerts to the page to illustrate the problem.

Thanks.

Shane

+2  A: 

Why not attach the event handler to the table and grab the rowindex of the cell that fired the click event inside the click handler.

Element
+2  A: 

When a function is called in javascript, the first thing that happens is that the interpreter sets the scope chain to the state it was in when the function was defined. In your case the scope chain looks like:

   GLOBAL
     |
CAll addTableEvents 
     |
CALL onclick

So when the variable orderID is stumbled upon by the javascript interpreter it searches the scope chain for that variable. There is no orderID defined inside your onclick function, so the next spot to look is inside addTableEvents. You would think orderID is defined here, but since you did not declare orderID with the var keyword, orderID became a global variable, so orderID is not found inside addTableEvents. The last spot to look is inside GLOBAL, and sure enough it is there, and its value is the last one which it was assigned, and in this case its the last value of orderID = row.getAttributeNode("id").value; in the for loop.

To see this more clearly look at the following

shanesObj.addTableEvents = function(){
table = document.getElementById("trackerTable");
for(i=1; i<table.getElementsByTagName("tr").length; i++){
        row = table.getElementsByTagName("tr")[i];
        orderID = row.getAttributeNode("id").value;
        alert("before onclick: " + orderID);
        row.onclick=function(){var orderID = orderID; shanesObj.tableRowEvent(orderID);};
}
orderID = -1;
}

shanesObj.tableRowEvent = function(orderID){
alert(orderID);
}

The result of a click here will always be -1.

So, to get around this problem, I suggest you drop all the extra and unneeded code and use something like the following,

for(i=1; i<table.getElementsByTagName("tr").length; i++){
        row = table.getElementsByTagName("tr")[i];
        row.onclick=function(){shanesObj.tableRowEvent(this.id);};

}

Just access the id attribute directly from the called object.

P.S. I have no idea why your code doesn't work in IE (it worked in my IE7).

ForYourOwnGood
Thank you. This worked very well, and it also helped me understand the problem.
Shane Larson
P.S. This does work in Firefox and IE now that the code has been corrected. Thanks again!
Shane Larson
Just a note to add something that caught me out: if you are re-generating your table regularly, via a regular AJAX poll for example, don't forget to re-add the listeners each time. Also, the index appears to need to start at 0 if you are looking at 'td' - not sure of this is the case for 'tr' also.
Mick
A: 
sourceboy
A: 

Code does not work on Firefox.. :( What ever the new up to date one is at this time 8/1/10

Troll