Closed Bug 326875 Opened 20 years ago Closed 19 years ago

ASSERTION: no document: 'doc' (nsXULElement::GetControllers)

Categories

(Core :: XUL, defect)

Product:
Core
Component:
XUL
Platform:
PowerPC
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jruderman, Assigned: neil)

References

Details

(Keywords: assertion, fixed1.8.1, testcase)

Attachments

(3 files)

testcase
385 bytes, application/vnd.mozilla.xul+xml
Details
Trunk patch
5.29 KB, patch
bzbarsky
: review+
bzbarsky
: superreview+
Details | Diff | Splinter Review
Branch patch
1.66 KB, patch
bzbarsky
: approval-branch-1.8.1+
Details | Diff | Splinter Review
Attached file testcase —
Blocks: 287002
That's a bogus a bogus assertion. It should be removed, imo.
We should just return null in that case, right?
Probably. Or fix things so that controllers are not dependent on being in a document.
(In reply to comment #4) >Or fix things so that controllers are not dependent on being in a document. The controllers aren't, but for some reason XUL elements want to be able to set the command dispatcher on the controllers (although nobody seems to use them).
> The controllers aren't They are because of cycles. See the code in nsXULElement::UnbindFromTree that nulls out controllers? We really need to clean up this stuff too. :(
(In reply to comment #6) >>The controllers aren't >They are because of cycles. See the code in nsXULElement::UnbindFromTree that >nulls out controllers? So if I add a controller to a textarea it just leaks?
I don't know. If you post a testcase, I can tell you. ;)
Something like this? data:text/html,<body onload="document.body.firstChild.controllers.appendController({leak: [window,document,document.body.firstChild]})"><textarea>
That throws, since unprivileged content can't access controllers. ;) But yes, if done from chrome that would leak (tested by locally removing the security checks on calling from content).
Attached patch Trunk patch — — Splinter Review
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #213747 - Flags: superreview?
Attachment #213747 - Flags: review?(bzbarsky)
Comment on attachment 213747 [details] [diff] [review] Trunk patch So is this commandDispatcher thing just completely unused?
Yes, I checked already when we looked at the window.controllers issue. Obviously the commandDispatcher is also available on the document, where it is used by script (iirc C++ just goes stright to the focus controller). Obviously only the nsXULElement changes could be checked in to the branch.
Comment on attachment 213747 [details] [diff] [review] Trunk patch Looks good, then.
Attachment #213747 - Flags: superreview?
Attachment #213747 - Flags: superreview+
Attachment #213747 - Flags: review?(bzbarsky)
Attachment #213747 - Flags: review+
Fix checked in to the trunk. Do you want me to extract the nsXULElement portion of the patch as a new attachment for branch approval?
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
Is this relevant on the branches?
I was only considering the 1.8.1 branch, on the basis that since we want it to be possible to enumerate and read the properties of a window without exceptions we should also do the same for a new XUL element.
Ah. Yeah, sure.
Attached patch Branch patch — — Splinter Review
Attachment #214111 - Flags: approval-branch-1.8.1?(bzbarsky)
Attachment #214111 - Flags: approval-branch-1.8.1?(bzbarsky) → approval-branch-1.8.1+
Fix checked in to the branch.
Keywords: fixed1.8.1
Crashtest checked in.
Flags: in-testsuite+
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: