202391 - .contentView is unnecessary

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[[e-mail]]

Since before trees were renamed from outliners tree content views have had DOM
class info and therefore the interface was always automatically flattened. Any
users of tree.contentView can convert to tree.view without functional loss.

Comment 1

[[e-mail]]

Attachment: Proposed patch

Note: I've only tested a few of the easier call sites.

Comment 2

[Jan Varga [:janv]]

Comment on attachment 120814 [details] [diff] [review]
Proposed patch

I think we shouldn't remove .contentView at the moment, mark it as deprecated
for now.

Comment 3

[Jan Varga [:janv]]

Comment on attachment 120814 [details] [diff] [review]
Proposed patch

r=varga with the change

Comment 4

[[e-mail]]

Index: xpfe/global/resources/content/bindings/tree.xml
===================================================================
RCS file: /cvsroot/mozilla/xpfe/global/resources/content/bindings/tree.xml,v
retrieving revision 1.30
diff -u -r1.30 tree.xml
--- xpfe/global/resources/content/bindings/tree.xml	12 Mar 2003 09:00:28 -0000	1.30
+++ xpfe/global/resources/content/bindings/tree.xml	17 Apr 2003 11:41:05 -0000
@@ -43,7 +43,7 @@
                 onget="return this.treeBoxObject.view;"
                 onset="return this.treeBoxObject.view=val;"/>
       <property name="contentView"
-                onget="return
this.view.QueryInterface(Components.interfaces.nsITreeContentView);"
+                onget="return this.treeBoxObject.view;"
                 readonly="true"/>
       <property name="builderView"
                 onget="return
this.view.QueryInterface(Components.interfaces.nsIXULTreeBuilder);"

Comment 5

[Jan Varga [:janv]]

ok

Comment 6

[jag (Peter Annema)]

Comment on attachment 120814 [details] [diff] [review]
Proposed patch

What's the advantage of this change?

Use of |contentView|, and its implementation, help as documentation of which
interface these methods & attributes are on.

What about just

<property name="contentView"
	  onget="return this.treeBoxObject.view; /*.QI(nsIContentView)*/"
	  readonly="true"/>

That should give you the performance gain while keeping the self documenting
code aspect.

Comment 7

[Jan Varga [:janv]]

Well, there are not many methods on content and builder view:
contentView.getItemAtIndex()
contentView.getIndexOfItem()

builderView.getResourceAtIndex()
builderView.getIndexOfResource()

I think that's self explanatory enough, but I agree that we shouldn't remove
them, just remove the unnecessary QI.

Only one question, you think we shouldn't s/contentView/view ?

Comment 8

[[e-mail]]

Attachment: Only remove the QI

Comment 9

[jag (Peter Annema)]

Comment on attachment 121266 [details] [diff] [review]
Only remove the QI

I like this.

Comment 10

[[e-mail]]

Fix checked in.