Closed Bug 465767 Opened 12 years ago Closed 12 years ago

document.adoptNode() stops working if node was previously adopted in another document

Categories

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

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9.1b3

People

(Reporter: ecfbugzilla, Assigned: smaug)

References

Details

(Keywords: regression, testcase, verified1.9.1)

Attachments

(2 files)

Attached file Testcase
I got this issue reported for the Element Hiding Helper extension. Apparently, in the current trunk nightlies document.adoptNode() at some point starts throwing NS_ERROR_UNEXPECTED. I managed to create a minimized testcase. Open it, click "Adopt node" (will be successful). Wait some time without moving your mouse (30 seconds), click "Adopt node" again - this time there will be an error. The testcase unloads the document the node was adopted in, it seems that something gets garbage collected as a result and makes new adoptNode() calls fail. During my testing, this was very consistently reproducible in Minefield 20081119 build on Ubuntu 8.10, on Windows this wasn't quite as reliable (maybe I just didn't wait long enough).
Assignee: nobody → Olli.Pettay
Blocks: 456271
Depends on: strongparent
Flags: blocking1.9.1?
Status: NEW → ASSIGNED
Strong parentNode/ownerDocument would fix this too, but since we don't have
that yet we can do this.
Attachment #349007 - Flags: superreview?(bzbarsky)
Attachment #349007 - Flags: review?(bzbarsky)
Comment on attachment 349007 [details] [diff] [review]
no need to be so strict

>+++ mozilla/content/base/test
>+function CC() {
>+  netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
>+  window.QueryInterface(Components.interfaces.nsIInterfaceRequestor)
>+        .getInterface(Components.interfaces.nsIDOMWindowUtils)
>+        .garbageCollect();
>+}

Having to copy/paste this all over is ridiculous.  How about putting it in SimpleTest.js as SimpleTest.gc instead?  I've had that in my tree for a while now, but the patch it's part of doesn't seem to be getting review love... ;)

Other than that, looks fine to me.
Attachment #349007 - Flags: superreview?(bzbarsky)
Attachment #349007 - Flags: superreview+
Attachment #349007 - Flags: review?(bzbarsky)
Attachment #349007 - Flags: review+
Yeah, I was thinking about that .CC thing too. I'll file a new bug about that.
Attachment #349007 - Flags: approval1.9.1?
filed Bug 465809
Good to know that you can force garbage collection, this might make some of my testcases simpler in future :)
could this land for b3?
Yes, I'm hoping so. The patch must get approval first.
Attachment #349007 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 349007 [details] [diff] [review]
no need to be so strict

a191=beltzner
Blocks: abp
Not blocking, but this has approval and looks ready to land...
Flags: blocking1.9.1? → blocking1.9.1-
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [needs-1.9.1-landing]
Fixed on trunk and 1.9.1
Whiteboard: [needs-1.9.1-landing]
Flags: in-testsuite+
Keywords: fixed1.9.1
Target Milestone: --- → mozilla1.9.1b3
verified FIXED using attached testcase on builds:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090721
Minefield/3.6a1pre ID:20090721044139

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1pre) Gecko/20090720
Shiretoko/3.5.1pre ID:20090720042942
Status: RESOLVED → VERIFIED
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.