404683 - for-in out of bounds with Select.options

Status: RESOLVED INVALID

(Core :: DOM: Core & HTML, defect)

Description

[Enrique Pineda]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.9) Gecko/20071025 Firefox/2.0.0.9

Occasionally the Javascript for-in goes beyond the upper bounds (i.e. Select.length) when iterating over the options collection.  For example,

for (x in myArray)
     alert(myArray[x]);

However, the following works correctly

for (x = 0; x < myArray.length; ++x)
     alert(myArray[x]);

Not sure if this is an issue with Select object, or with the "in" keyword.

Reproducible: Sometimes

Steps to Reproduce:
see above
Actual Results:  
When it goes out of bounds, you get invalid references to the array's element at the current iteration

Expected Results:  
for-in should stop at the last element of Sort.options

Comment 1

[Enrique Pineda]

In the example above, I should have iterated over a Select.options collection.  Sorry for the generic example.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

We iterate over the following keys:

log: 0
log: 1
log: 2
log: length
log: item
log: selectedIndex
log: add
log: namedItem
log: remove

I believe at least for the methods, this is wrong. Not sure about selectedIndex and length; does HTML need to change here?

Comment 3

[Cameron McCormack (:heycam)]

Web IDL does currently require attributes to be enumerable and methods not.  Whether these methods on HTMLOptionsCollection need to be enumerable for compatibility, I don't know.  (Opera, Safari, Chrome all do enumerate them.)

Comment 4

[Boris Zbarsky [:bzbarsky]]

I would be really surprised if not enumerating IDL methods didn't break compat...  Certainly they need to be enumerable on prototype objects; people rely on that right now to copy methods from one prototype to another.  And at that point they would be enumerable on the object itself, since it just gets them via the prototype.  Worth checking with other UA vendors too, though.

Note that in any case |length| would be enumerable, so the bug as filed is invalid given the current state of the specs.

Now Select could define a custom enumerator, of course...  Maybe it should.  Hard to say.

Note that |for ... in| on an Array will also enumerate the "length" property, and maybe others.  So I don't think a custom enumerator on Select should skip that, at least.

Comment 5

[Cameron McCormack (:heycam)]

(In reply to comment #4)
> I would be really surprised if not enumerating IDL methods didn't break
> compat...  Certainly they need to be enumerable on prototype objects; people
> rely on that right now to copy methods from one prototype to another.  And at
> that point they would be enumerable on the object itself, since it just gets
> them via the prototype.  Worth checking with other UA vendors too, though.

Yeah, even on "normal" objects like HTMLElement they should be enumerable, it seems: http://people.mozilla.org/~cmccormack/tests/enum-htmlelement.html

I probably had them as non-enumerable for consistency with the methods on built-in objects.

> Note that |for ... in| on an Array will also enumerate the "length" property,
> and maybe others.  So I don't think a custom enumerator on Select should skip
> that, at least.

I think "length" on Arrays are explicitly [[Enumerable]]:false according to the ES5 spec.

Comment 6

[Cameron McCormack (:heycam)]

Filed http://www.w3.org/Bugs/Public/show_bug.cgi?id=11485 for myself.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Note also that JS arrays can have holes, which are enumerated if going 0 to length but are not enumerated via for..in.

So really, the fact that for..in and going 0 to length ever coincide is an accident.  ;)