Drop the nsINativeTreeView pretense

RESOLVED FIXED in Firefox 60

Status

()

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

unspecified
mozilla60
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(2 attachments, 3 obsolete attachments)

Posted file test.xul
In bug 334085 we added an nsINativeTreeView thing that was supposed to be implemented by views that we'd allow untrusted code to set on a tree.

But we never made anything QI to it, so all that stuff is a no-op.  The attached testcase throws a security exception when run as non-system, even though it never does anything interesting with views.

Credit to Adrian Wielgosik for finding this.
MozReview-Commit-ID: FBEGEFRM9Us
Attachment #8950353 - Flags: review?(gijskruitbosch+bugs)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
MozReview-Commit-ID: FBEGEFRM9Us
Attachment #8950381 - Flags: review?(gijskruitbosch+bugs)
Attachment #8950353 - Attachment is obsolete: true
Attachment #8950353 - Flags: review?(gijskruitbosch+bugs)
MozReview-Commit-ID: FBEGEFRM9Us
Attachment #8950382 - Flags: review?(gijskruitbosch+bugs)
Attachment #8950381 - Attachment is obsolete: true
Attachment #8950381 - Flags: review?(gijskruitbosch+bugs)

Comment 4

Last year
Comment on attachment 8950382 [details] [diff] [review]
Drop the nsINativeTreeView stuff, since it's all not working anyway

Review of attachment 8950382 [details] [diff] [review]:
-----------------------------------------------------------------

Clearing review because of the below, but also because when I looked at DXR I spotted nsINativeTreeSelection which looks just as dead. Perhaps we can remove this too?

(That's beside the point that I'm not technically a peer, but I assume your decision to ask me for review was deliberate... :-) )

::: layout/xul/tree/nsITreeBoxObject.idl
@@ +24,5 @@
>     * The view that backs the tree and that supplies it with its data.
>     * It is dynamically settable, either using a view attribute on the
>     * tree tag or by setting this attribute to a new value.
>     */
> +  readonly attribute nsITreeView view;

I'm a bit confused. Why is this readonly now, while the webidl isn't? The comment here ("It is dynamically settable ... by setting this attribute") doesn't help. There's definitely still a setter in the implementation. Perhaps updating the comment here would help?
Attachment #8950382 - Flags: review?(gijskruitbosch+bugs)
> but also because when I looked at DXR I spotted nsINativeTreeSelection which looks just as dead.

It's not as dead because things actually QI to it.  Nothing QIs to nsINativeTreeView.

> I'm not technically a peer, but I assume your decision to ask me for review was deliberate

Technically the only peers for "XUL" are me an Jan Varga or something like that.  I figured you'd maybe at least know something about trees.  ;)

> Why is this readonly now, while the webidl isn't? 

Ah.  I'll adjust the comments to make this clearer.
There are no consumers of nsITreeBoxObject::SetView, so this fix removes that
setter.  There are consumers of GetView, though.

MozReview-Commit-ID: FBEGEFRM9Us
Attachment #8950454 - Flags: review?(gijskruitbosch+bugs)
Attachment #8950382 - Attachment is obsolete: true

Updated

Last year
Attachment #8950454 - Flags: review?(gijskruitbosch+bugs) → review+

Comment 7

Last year
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce006d43344b
Drop the nsINativeTreeView stuff, since it's all not working anyway.  r=gijs

Comment 8

Last year
bugherder
https://hg.mozilla.org/mozilla-central/rev/ce006d43344b
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla60

Updated

Last year
Depends on: 1438470
Depends on: 1438512
You need to log in before you can comment on or make changes to this bug.