Closed
Bug 604476
Opened 14 years ago
Closed 14 years ago
for(i in XRayWrapper(nodelist)) fails with "i is undefined" error post-compartments landing
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | beta7+ |
People
(Reporter: KWierso, Assigned: mrbkap)
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(2 files)
159.03 KB,
application/x-xpinstall
|
Details | |
2.03 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
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
Keywords: testcase-wanted
Comment 1•14 years ago
|
||
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?
Summary: for(i in ARRAY) fails with "i is undefined" error post-compartments landing → for(i in XRayWrapper(nodelist)) fails with "i is undefined" error post-compartments landing
Updated•14 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 2•14 years ago
|
||
It should, the call to js::GetPropertyNames with wnObject should end up in the enumerat hook.
Comment 3•14 years ago
|
||
Is there a testcase for this?
Comment 4•14 years ago
|
||
Wes, care to attach or link to an extension that shows the problem?
Reporter | ||
Comment 5•14 years ago
|
||
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•14 years ago
|
||
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•14 years ago
|
||
We should have defined it on the wrapped native, not the expando.
Assignee | ||
Comment 8•14 years ago
|
||
We need to do the full property resolution so that our internal state is set up correctly.
Updated•14 years ago
|
Attachment #483688 -
Flags: review?(gal) → review+
Assignee | ||
Comment 9•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/41690f030f9a
Keywords: testcase-wanted
Whiteboard: fixed-in-tracemonkey
Comment 10•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/41690f030f9a
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•14 years ago
|
blocking2.0: --- → ?
Updated•14 years ago
|
blocking2.0: ? → beta7+
You need to log in
before you can comment on or make changes to this bug.
Description
•