Closed Bug 202391 Opened 17 years ago Closed 17 years ago

.contentView is unnecessary

Categories

(Core :: XUL, defect, trivial)

x86
Windows 95
defect
Not set
trivial

Tracking

()

RESOLVED FIXED

People

(Reporter: neil, Assigned: janv)

References

()

Details

Attachments

(2 files)

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.
Blocks: 197463
Attached patch Proposed patchSplinter Review
Note: I've only tested a few of the easier call sites.
Attachment #120814 - Flags: superreview?(jaggernaut)
Attachment #120814 - Flags: review?(varga)
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 on attachment 120814 [details] [diff] [review]
Proposed patch

r=varga with the change
Attachment #120814 - Flags: review?(varga) → review+
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);"
ok
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.
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 on attachment 121266 [details] [diff] [review]
Only remove the QI

I like this.
Attachment #121266 - Flags: superreview+
Attachment #121266 - Flags: review+
Fix checked in.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Attachment #120814 - Flags: superreview?(jaggernaut)
Component: XP Toolkit/Widgets: Trees → XUL
QA Contact: shrir → xptoolkit.widgets
Depends on: 1508143
You need to log in before you can comment on or make changes to this bug.