Closed Bug 566846 Opened 10 years ago Closed 10 years ago

Iterator no longer plays nice with XPCNativeWrappers, possibly breaks existing code.

Categories

(Core :: XPConnect, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: protz, Assigned: mrbkap)

References

()

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

This is the followup to bug 553579.

I'm manipulating iframes that have type="content" and I'm also using the JS Iterator object. This is specific to 1.9.3 and <iframe type="content">s (removing type="content" "fixes" the issue).

Basically, I'm accessing iframe.contentDocument.body.childNodes (let's call it childNodes from now on).

In 1.9.2:
dump(childNodes) --> [object XPCNativeWrapper [object NodeList]][5]
dump(childNodes[0]) --> [object XPCNativeWrapper [object Text]]
for each (let x in Iterator(childNodes)) dump(x+"\n") -->
  0,[object Text]
  1,[object HTMLParagraphElement]
  2,[object Text]
  and so on

In 1.9.3:
dump(childNodes) --> [object XPCNativeWrapper [object NodeList]][5]
dump(childNodes[0]) --> [object XPCNativeWrapper [object Text]]
for each (let x in Iterator(childNodes)) dump(x+"\n") -->
  undefined
  undefined
  undefined
  etc...

I don't know if this is intentional, but I'd expect this Iterator to yield indices as well as XPCNativeWrappers for the childNodes[i]. This seems to me like a regression (actually, it does break one of my extensions on 1.9.3).

I've uploaded a test extension for you to try, because this needs to be run inside chrome code. It's very small (less than 20 lines of useful JS, plus some XUL boilerplate).

Steps to reproduce:
- install the .xpi from the bug URL
- enable dump in the console
- from the Error console, run open("chrome://testcase5/content/testcase.xul",
"", "chrome,width=500,height=500");
- click "add iframe"
- click "load some content"

This demonstrates the issue (watch the console output). This is quite a minimalistic test case (less than 20 lines of useful JS). Feel free to ask if you need more information. I've assigned this to XPConnect as this is an issue with XPCNativeWrappers, but I'm quite unsure this is relevant, so this probably will end up reassigned.
Attached patch Fix (obsolete) — Splinter Review
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #446412 - Flags: review?(gal)
I think a better fix would be to give your wonky iterator an iteratorObject hook that returns self. We want to get rid of __iterator__.
Attached patch Fix (obsolete) — Splinter Review
Attachment #446412 - Attachment is obsolete: true
Attachment #446417 - Flags: review?(gal)
Attachment #446412 - Flags: review?(gal)
Attached patch FixSplinter Review
Attachment #446417 - Attachment is obsolete: true
Attachment #446419 - Flags: review?(gal)
Attachment #446417 - Flags: review?(gal)
Comment on attachment 446419 [details] [diff] [review]
Fix

Do we have any other non-native iterator objects in the tree? And does this need dev doc?
Attachment #446419 - Flags: review?(gal) → review+
http://hg.mozilla.org/mozilla-central/rev/f8aca3972045
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Please consider the following piece of code.

dump("---\n");
for each (let [, x] in Iterator(document.getElementsByClassName("extra-button"))) {
  dump(x+"\n");
}
dump("---\n");

The resulting output is:

---
[object XULElement]
[object XULElement]
[object XULElement]
[object XULElement]
[object XULElement]
[object XULElement]
6
function item() {
    [native code]
}
function namedItem() {
    [native code]
}
---

I think Iterator is still broken, so I'm reopening. Or maybe I'm missing something, but I thought the purpose of Iterator was precisely to skip custom properties such as length on arrays and others... ?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
followup on bug 571849, closing
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.