604476 - for(i in XRayWrapper(nodelist)) fails with "i is undefined" error post-compartments landing

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101014 Firefox/4.0b8pre
Build Identifier: 

After I updated Minefield to today's nightly (with compartments landed), any time my extensions use "for(i in ARRAY)" or "for(let i in ARRAY)" to go through a list of objects inside ARRAY, I get errors saying "i is undefined".

If I change the code to do something like "for(let i=0;i<ARRAY.length;i++)", it works just fine.

Reproducible: Always

Comment 1

[Boris Zbarsky [:bzbarsky]]

Apparently "ARRAY" in this case is the return value from calling getElementsByTagName on a content document from a chrome script.

Presumably XRayWrapper::Enumerate isn't calling into the classinfo enumerate hook?

Comment 2

[Peter Van der Beken [:peterv]]

It should, the call to js::GetPropertyNames with wnObject should end up in the enumerat hook.

Comment 3

[Peter Van der Beken [:peterv]]

Is there a testcase for this?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Wes, care to attach or link to an extension that shows the problem?

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Attachment: my addon

Here's my addon. 

I added some alert() statements to alert the errors caught in /chrome/rtse.jar/content/pageFix.js, lines 1199, 1231, 1262, and 1281.

That code is only run if you are logged in with an account on this site (http://roosterteeth.com/ ), on this page ( http://roosterteeth.com/members/ )

If you don't feel like setting up an account, you can use the one I just created.
Username: rtbugzilla
Password: password1


Oh, and you'll need to go into the extension's options window to the "extras" tab, and make sure the "Enable hide homepage elements feature" checkbox IS checked.

Comment 6

[Peter Van der Beken [:peterv]]

Andreas, we end up in ResolveNativeProperty from XrayWrapper<Base,
Policy>::enumerate. The NewResolve hook tries to define the property, in
XrayWrapper<Base, Policy>::defineProperty we end up defining it on the expando
(GetExpandoObject). Next thing in ResolveNativeProperty we call
JS_GetPropertyDescriptorById on the holder, which of course doesn't find the
property. Any ideas?

Comment 7

[Andreas Gal :gal]

We should have defined it on the wrapped native, not the expando.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

We need to do the full property resolution so that our internal state is set up correctly.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/41690f030f9a

Comment 10

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/41690f030f9a