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

VERIFIED FIXED in mozilla1.9.1b3

Status

()

defect
VERIFIED FIXED
11 years ago
2 months ago

People

(Reporter: gaubugzilla, Assigned: smaug)

Tracking

(Blocks 1 bug, {regression, testcase, verified1.9.1})

Trunk
mozilla1.9.1b3
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reporter

Description

11 years ago
Posted 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?
Reporter

Comment 5

11 years ago
Good to know that you can force garbage collection, this might make some of my testcases simpler in future :)

Comment 6

11 years ago
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
Reporter

Updated

11 years ago
Blocks: abp
Not blocking, but this has approval and looks ready to land...
Flags: blocking1.9.1? → blocking1.9.1-
Status: ASSIGNED → RESOLVED
Last Resolved: 11 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
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.