350264 - Create a XUL element subclasses for trees, lists, frames etc instead of using box objects

Status: NEW

(Core :: XUL, defect)

Description

[Neil Deakin (away until Sept 6)]

A common error that comes up is that tree.view is null for trees that aren't visible, or not initialized yet. The problem is that the view is stored in the tree box object.

Instead, I think we should get rid of the tree box object and just create an nsXULTreeElement subclass, and store the view in a content property instead. Or, for a simpler solution, just store the view there anyway.

Comment 1

[Jan Varga [:janv]]

Neil, I've been thinking about it for some time. I have a fix for another bug that stores |nsITreeController treeController| in the tree XBL binding. I also added it to nsIDOMXULTreeElement.idl. Well in this case we would have to provide a way to create native views from JS. I don't know if it can be a security issue.

Comment 2

[[e-mail]]

(In reply to comment #1)
>I don't know if it can be a security issue.
The current tree box object has a security check on the view setter.
This is because we have the problem is that the view is used during paint.

(In reply to comment #0)
>The problem is that the view is stored in the tree box object.
Actually that's probably not the problem; although I didn't do it in bug 344085 because that simple-looking change already caused two regressions it might be possible to move all of the view initialization code to the box object.

Comment 3

[Jan Varga [:janv]]

Actually, I think it's quite confusing for XUL developers to remember properties/methods that are defined in the nsIDOMXULTreeElement interface and those that are defined in nsITreeBoxObject.
It would be great if could eliminate the wrapper object and access all properties/methods directly (e.g. tree.invalidateRow())
bryner and me asked hyatt some time ago, why there's a box object at all, I don't remember the exact answer, but I think he mentioned that there was no XBL at that time, etc.
We should probably ask him again.
If the security check has to be in C++, then I vote for a subclass.

Comment 4

[Neil Deakin (away until Sept 6)]

Attachment: work in progress patch

This patch mostly works, and replaces members of nsITreeBoxObject with properties on nsIXULTreeElement which is implemented by a new nsXULTreeElement subclass. In addition, a similar technique is used to get rid of nsIListBoxObject and the iframe/browser/editor box object interfaces.

Note that the patch includes the patch in bug 364612 as well.

Comment 5

[Neil Deakin (away until Sept 6)]

I split off the browser/iframe/editor part into bug 479895.