809508 - Iterator started yielding more keys than usual

Status: RESOLVED INVALID

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

Description

[Jonathan Protzenko [:protz]]

I've got a Thunderbird addon (Conversations) that broke today after I upgraded nightly. I've tracked down the bug to Iterator that, when applied a node collection, started yielding [0, theDomNode] (that's the usual behavior) but also (that's the new, unexpected behavior) [theDomNodesId, theDomNode].

Unfortunately, the bug is not easy to reproduce in Scratchpad since it involves what I believe is cross-compartment voodoo. In case someone is motivated enough to track it down, here are the STR:
- install Thunderbird Conversations from AMO in Daily (Thunderbird's codename for "Nightly"),
- enable debugging from the addon's options,
- try to view any message with attachments.

The bug appears in modules/message.js:899 where we do an extra iteration of the loop with the key being "msg0att0" (in the case of a single message with a single attachment). "msg0att0" happens to be the node's id which is set in one compartment, while the call to getElementsByClassName is made outside this compartment (at least, that's what I believe).

I'm very sorry for not having the time to work up a more minimal testcase.

As to my addon, I just moved to not using Iterator in that particular place. That's fine, since Iterator is pretty unreliable already: for instance, it sometimes yields out-of-bounds indices on a dom node collection, because the collection happens to be a recycled one from subsequent identical calls to getElementsByClassName (except one element was removed in the meanwhile), see bug 611957

Thanks,

jonathan

Comment 1

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

Bug 772869?

Comment 2

[Peter Van der Beken [:peterv]]

(In reply to Jonathan Protzenko [:protz] from comment #0)
> (that's the new, unexpected behavior) [theDomNodesId,
> theDomNode].

It's new but not unexpected, it's more spec-compliant (we fixed bug 772869).

> "msg0att0" happens to be the node's id which is set in
> one compartment, while the call to getElementsByClassName is made outside
> this compartment (at least, that's what I believe).

Why would compartments change anything? If you set the node's id then it has the same id in all compartments. getElementsByClassName returns a HTMLCollection and HTMLCollection has a named getter, so it should enumerate the named properties (see http://dev.w3.org/2006/webapi/WebIDL/#property-enumeration, second item).

Comment 3

[Jonathan Protzenko [:protz]]

I thought the compartments were involved because a basic test in the scratchpad didn't seem to exhibit that behavior, but I now realize I'm running Aurora, so that's to be expected.

If it's just a move to be more spec-compliant, then I'm fine with it! I was just surprised about the change and somehow thought it was a bug.

(In reply to :Ms2ger from comment #1)
> Bug 772869?
Most certainly.