Closed Bug 80926 Opened 24 years ago Closed 3 years ago

outliner can't be selected when loaded from file://

Categories

(Core :: Security: CAPS, defect, P3)

defect

Tracking

()

RESOLVED WORKSFORME
Future

People

(Reporter: axel, Assigned: dveditz)

Details

Attachments

(3 files)

Loading a xul containing a outliner goes fine, but one can't select. This is broken, because mozilla/xpfe/global/resources/content/bindings/outliner.xml:27 can't create the wrapper. I changed the getter of outlinerBoxObject to say onget="var retVal = this.boxObject.QueryInterface(Components.interfaces.nsIOutlinerBoxObject); dump('|'+retVal+'|'); return retVal;" and get the following when selecting a row in extensions/transformiix/resources/buster/buster.xul: |[xpconnect wrapped BoxObject @ 0x767800]|blank Warning prev sibling is not in our list!!!Document file:///tmp/mozilla/extensions/transformiix/resources/buster/buster.xul loaded successfully JavaScript error: line 0: uncaught exception: Permission denied to create wrapper for object When I add the dump before the access to boxObject, it get's printed. Axel
You can try to reproduce this by loading mozilla/extensions/transformiix/resources/buster/buster.xul. I'm going to attach a stack trace for the failing security check. aObj is an nsXULElement. If you get hit by the "Node is not defined" assertion, try replacing Node.ELEMENT_NODE in mozilla/extensions/transformiix/resources/buster/xulTxTool.js with Components.interfaces.nsIDOMNode.ELEMENT_NODE. I'm a bit at a loss where the bug is exactly.
Attached file Stack trace on Windows (deleted) —
Hmm, the way to reproduce :-): open the file, it will load data in the outliner view (this might take a while). Once you see the rows displayed, click on a row, that's where you'll see JavaScript error: line 0: uncaught exception: Permission denied to create wrapper for object
Attached patch Proposed fix (deleted) — Splinter Review
I had to add DOM ClassInfo to nsOutlinerBoxObject, to our OulinerView implemented in JS (plus add the QueryInterface implementation for that class) and remove a QueryInterface in outliner.xml. That last one is a bit strange. Why would a QI in JS in an XBL binding (loaded from chrome) be denied? Just because we use an outliner from within a file:// XUL file? Seems wrong to me.
XBL runs with the permissions of the document that loads it. There's a bug on finding some way to give XBL more permissions, but no one has yet worked out a way to do so. The problem being, of course, that Web pages need to be able to load chrome bindings (e.g., textfields for example), but they can't be allowed to take advantage of the godlike powers that the binding would have if it got permissions based off its location rather than the document it's used in.
Peter, why does xulTxTools.js need UniversalXPConnect privileges? If it's running from chrome, that shouldn't be required. Is it a testcase run from the Web?
Hyatt, I see the problem, thanks for explaining. Mitch, this (our test automation) is loaded from a file:// URL. I merely moved the UniversalXPConnect privileges up because I have a problem using Node.ELEMENT_NODE, it seems I need UniversalXPConnect privileges to use that.
Node.ELEMENT_NODE is part of the DOM, right? You shouldn't need to use XPConnect to access it, I think. Maybe that object doesn't have a ClassInfo. ClassInfo tells the security manager that the object should be fully scriptable without privileges.
That said, using UniversalXPConnect is just fine, no reason not to. It just probably shouldn't be a requirement in this case.
Yes, Node.ELEMENT_NODE is part of the DOM, in mozilla it maps to window.Components.interfaces.nsIDOMNode.ELEMENT_NODE.
Found the xbl permissions bug, adding dependency. The right fix should go there. I still hope for the right fix to exist, instead of having to whack class info for everything. (which may or may not be a good idea for some objects, is nsOutlinerBoxObject one of them?) Anyway, I never needed the trick for Node.ELEMENT_NODE on solaris, so that one still puzzles me. Axel
Depends on: 59701
No longer depends on: 59701
Adding classinfo to the class nsOutlinerBoxObject is not a bad idea (and should be done), but Axel's right, we shouldn't blindly add classinfo to every class w/o thinking about what we're doing...
If classes are part of the DOM, then why shouldn't they have a classInfo that advertises them as such? For this bug, should we go ahead and add classInfo to nsOutlinerBoxObject?
Target Milestone: --- → mozilla0.9.2
Other box objects already have classinfo, I see no reason not to add classinfo to this one. However, nsOutlinerBoxObject, inherits nsBoxObject, which already has classinfo (and nsOutlinerBoxObject::QI() does call into nsBoxObject::QI()), so why is that not enough?
Seems like that ought to work; I'm not sure what the problem is. We should look at it together.
Status: NEW → ASSIGNED
I think the inherited DOM class info doesn't work because it needs things from the nsIOutlinerBoxObject interface, which nsBoxObject doesn't implement.
The inherited classinfo stuff should work, the helper used for the boxobject does allow for QI'ing the JS objects to any interface, even if it's not in the list of interfaces supported by the object. People use QI on boxobjects in JS to get between interfaces, so a missing interface in the boxobject's interface map shouldn't matter.
Priority: -- → P3
Target Milestone: mozilla0.9.2 → mozilla0.9.3
milestone 0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4
Selection needs class info also. nsIOutlinerSelection. Is that the bug?
This patch isn't good anymore, jst changed stuff while landing prototypes. Hyatt, nsIOutlinerSelection is already good, NS_DEFINE_CLASSINFO_DATA(OutlinerSelection, nsDOMGenericSH, DEFAULT_SCRIPTABLE_FLAGS) in mozilla/dom/src/base/nsClassInfo.cpp Axel
Moving to 0.9.5.
Target Milestone: mozilla0.9.4 → mozilla0.9.5
time marches on...retargeting to 0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6
I'll attach a patch that makes mozilla/layout/xul/base/src/outliner/outliner.xul a non-asserting test-app again. Is 'working from file/remotely' XUL1.0 relevant?
Target Milestone: mozilla0.9.6 → mozilla0.9.8
performance, footprint, feature work, and re-architecture bugs will be addressed in 0.9.8
Mozilla1.0
Target Milestone: mozilla0.9.8 → mozilla1.0
Moving Netscape owned 0.9.9 and 1.0 bugs that don't have an nsbeta1, nsbeta1+, topembed, topembed+, Mozilla0.9.9+ or Mozilla1.0+ keyword. Please send any questions or feedback about this to adt@netscape.com. You can search for "Moving bugs not scheduled for a project" to quickly delete this bugmail.
Target Milestone: mozilla1.0 → mozilla1.2
Futuring.
Target Milestone: mozilla1.2alpha → Future
outliner is long-since dead. Could we re-evaluate this bug, please?
Assignee: security-bugs → dveditz
Status: ASSIGNED → NEW
QA Contact: ckritzer → caps
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: