Element sort routine works in Firefox, but crashes in Chrome

I've written the following routine to sort the option elements within a select:

function SortSelect(select)
{
    var tmpAry = new Array();
    for (var i in select.options)
        tmpAry[i] = select.options[i];
    tmpAry.sort(function(opta, optb)
    {
        if (opta.id > optb.id) return  1;
        if (opta.id < optb.id) return -1;
        return 0;
    });
    while (select.options.length > 0)
        select.options[0] = null;
    for (var i in tmpAry)
        select.appendChild(tmpAry[i]);
}

It works just fine with Firefox as part of a Greasemonkey script. However, in Chrome, both with and without TamperMonkey, I get this:

Uncaught Error: NOT_FOUND_ERR: D开发者_StackOverflowOM Exception 8
SortSelect:125

As is typical for Javascript debuggers, the error line number is totally wrong, so it's difficult to pin down exactly why this is breaking and where. I'm open to suggestions as to why the code is bugging out or ways to effectively debug it (I'm new to Chrome). Thanks.

You should iterate over the options collection using an index, not a for..in loop. The following:

for (var i in select.options) {
  tmpAry[i] = select.options[i];

should be:

var options = select.options;
for (var i=0, iLen=options.length; i<iLen; i++) {
  tmpAry[i] = options[i];
}

You are likely getting properties from the options collection that aren't option elements, such as length.

You also should not assign "null" to an option. If you want to remove all the options, just set the length of options to zero:

var options.length = 0;

Finally, you should iterate over tmpAray using an index since for..in will not return the options in the same order in every browser and may return non-numeric enumerable properties if there are any, use an index. Also, you can just assign the option back to the select's options collection, there is no need for appendChild:

select.options.length = 0;
for (var i=0, iLen=tmpAry.length; i<iLen; i++) {
    select.options[i] = tmpAry[i];
}

If you are not removing any options, you should be able to just assign them in the new order, but some browsers can't handle that so removing them first is best.

Edit

Note that while the options property of a select element is readonly, the properties of an options collection are not. You can assign values (which should be references to option elements) directly to them.

What are you trying to do with this piece of code?

while (select.options.length > 0)
    select.options[0] = null;

If the answer is you're trying to clear all the select options, that seems dangerous. I could see how this could easily be an infinite loop.

It looks to me like this would be a lot safer:

for (var i = select.options.length - 1; i > 0; i--) {
    select.remove(i);
}

Then, there's an select.options.add() method for adding them back.

FYI, it is also considered risky practice to use this construct on arrays or pseudo arrays:

for (var i in select.options)
for (var i in tmpAry)

as that can pick up properties that have been added to the object in addition to just array elements.

More typing, but safer to use:

for (var i = 0, len = tmpAry.length; i < len; i++) {
   // code here
}

These lines can cause an infinite loop or an access violation:

while (select.options.length > 0)
    select.options[0] = null;

Also, you do not need to delete the nodes and reinsert them; appendChild() works fine in all browsers to move nodes around.

So, this code will work and should be more efficient than removing and recreating nodes (which can also trash any event listeners). :

See it in action at jsFiddle.

function SortSelect (select)
{
    var tmpAry  = [];
    for (var J = select.options.length - 1;  J >= 0;  --J)
        tmpAry.push (select.options[J] );

    tmpAry.sort ( function (opta, optb) {
        if (opta.id > optb.id) return  1;
        if (opta.id < optb.id) return -1;
        return 0;
    } );

    while (tmpAry.length) {
        select.appendChild ( document.getElementById (tmpAry[0].id) );
        tmpAry.shift ();
    }
}