1437654 - Drop the nsINativeTreeView pretense

Status: RESOLVED FIXED

(Core :: XUL, enhancement)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: 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.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Deop the nsINativeTreeView stuff, since it's all not working anyway

MozReview-Commit-ID: FBEGEFRM9Us

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Deop the nsINativeTreeView stuff, since it's all not working anyway

MozReview-Commit-ID: FBEGEFRM9Us

Comment 3

[Boris Zbarsky [:bzbarsky]]

Attachment: Drop the nsINativeTreeView stuff, since it's all not working anyway

MozReview-Commit-ID: FBEGEFRM9Us

Comment 4

[:Gijs (he/him)]

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?

Comment 5

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Attachment: Drop the nsINativeTreeView stuff, since it's all not working anyway

There are no consumers of nsITreeBoxObject::SetView, so this fix removes that
setter.  There are consumers of GetView, though.

MozReview-Commit-ID: FBEGEFRM9Us

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ce006d43344b
Drop the nsINativeTreeView stuff, since it's all not working anyway.  r=gijs

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/ce006d43344b