views:

219

answers:

4

I have a list of check boxes. For the check boxes that are selected, I change it's name before submitting. In FF the function works. In IE I get:

A script on this page is causing Internet Explorer to run slowly. If it continues to run, your computer may become unresponsive.

Do you want to abort the script? YES/NO

Not sure why this loop is causing problems in IE and not FF?

 function sub()
{

    var x=document.getElementsByName("user");
    for (i = 0; i < x.length; i++) //for all check boxes
    {
     if (x[i].checked == true)
     {
      x[i].name="id"; //change name of data so we know it is for an id
      //By renaming the first element of the list, we have reduced the length of the list by one 
      //and deleted the first element. This is why we need to keep i at it's current position after a name change.
      i=i-1; 
     }

    }//end for

    document.checks.submit();

}
A: 

Edit: This is wrong, Nick is right. Although I'd look extra hard to make sure IE actually implements the NodeList correctly, since it still sounds like what you're running into is an infinite loop in IE.

Thanks for the heads up, Nick, I learned something new today. Javascript never ceases to surprise me :)


Just renaming an element in the array would not remove it from the array -- getElementsByName is not run constantly, just the initial time that it is called. So IE is stuck in an infinite loop where it keeps checking the same checked element over and over. I'm honestly not sure why Firefox doesn't also get stuck in the same infinite loop.

As far as I understand, the i=i-1 line is entirely unnecessary.

Luke Rinard
Not correct: getElementsByName returns a NodeList, not an array. NodeLists are "live", in that they reflect changes to the document. See http://www.w3.org/TR/2000/WD-DOM-Level-1-20000929/level-one-core.html#ID-536297177
NickFitz
NodeLists can be pretty confusing :-)
NickFitz
Internet Explorer is buggy in this regard. It isn't fixed in IE8 either.
Chetan Sastry
+1  A: 

Internet Explorer is buggy. It doesn't return a live nodelist but just a snapshot.

Using a framework such as jQuery is your best bet in achieving full compatibility.

Chetan Sastry
+2  A: 

I would avoid writing a script like that - it is like having an for/i++ loop calling a function that changes the index as a side effect - unpredictable. You run an iterator through NodeList while modifying the list from inside the loop. You cannot be sure it works until you happen to know exactly the way NodeList is implemented.

It's unpleasant, but I would first copy the list into "real" array, and then do the renaming.

buti-oxa
+1  A: 

This should work with a both a live and a non-live list.

Personally, though, I'd try to find some way to output the original page so that the server can figure out which elements to use, instead of relying on javascript to do it.

 function sub()
{

    var x=document.getElementsByName("user");
    var nodesToChangeIndex=0;
    var nodesToChange=new Array();
    for (i = 0; i < x.length; i++) //for all check boxes
    {
        if (x[i].checked == true)
        {
                nodesToChange[nodesToChangeIndex++] = x[i];
        }

    }//end for

    for(i=0; i < nodesToChangeIndex; i++)
    {
        nodesToChange[i].name="id";
    }
    document.checks.submit();

}
Dave