350080 - nsPIWidgetMac.idl uses two non-IDL defined interfaces

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Mac, defect)

Description

[jhp (no longer active)]

Found this while building patches JavaXPCOM bug 333618 on trunk.

The interface nsPIWidgetMac is set as "scriptable", but has two methods that return objects of interfaces that are not defined in IDL files (and therefore, not scriptable): nsMacWindow and nsMacEventDispatchHandler.

The quick fix would be to set both of these methods as [noscript]. However, I wanted to know if this interface should be made non-scriptable all together.  Should it be available to Java or JavaScript (or Python, etc)?  Would a non-C++ embedder need to use this interface (besides the two mentioned methods)?

Comment 1

[Josh Aas]

AFAIK none of nsPIWidget needs to be scriptable. Just get rid of "scriptable" here:

[scriptable, uuid(5DE488F0-C9B1-427C-938F-D3D13DEFF987)]

Comment 2

[jhp (no longer active)]

Attachment: patch

Make nsPIWidgetMac non-scriptable.  Josh agrees with this patch.  Mark, what do you think?

Comment 3

[Mark Mentovai]

Comment on attachment 235285 [details] [diff] [review]
patch

Yeah, there's no reason for this to be scriptable.  Maybe we should also make nsMacWindow and nsMacEventDispatchHandler into [ptr] native types.

Comment 4

[jhp (no longer active)]

Attachment: patch checked in

This is the patch I checked in, with Mark's suggestiong for "[ptr] native".

Comment 5

[jhp (no longer active)]

Attachment: 1.8.1 branch patch

Seeking 1.8.1 approval.  This patch is needed by patch in bug 333618.

Comment 6

[Darin Fisher]

Comment on attachment 236824 [details] [diff] [review]
1.8.1 branch patch

>Index: nsPIWidgetMac.idl

>-[scriptable, uuid(59356b39-2031-4fd2-a856-435cda1ef700)]
>+[uuid(59356b39-2031-4fd2-a856-435cda1ef700)]
> interface nsPIWidgetMac : nsISupports

I think this change is probably not correct for the 1.8 branch.  Though
it is a private interface, we're trying not to change even private
interfaces like this for the 1.8 branch.  The rest of the patch is fine.

We'll approve the patch for the branch sans change to nsPIWidgetMac.

Comment 7

[jhp (no longer active)]

Attachment: 1.8.1 branch patch

By comment #6, did you mean that you would not accept any "scriptable" changes, or just to nsPIWidget?  This patch keeps "scriptable" on nsPIWidget, but removes it from nsPIWidgetMac_MOZILLA_1_8_BRANCH.  If you don't want me to do that, then I can keep nsPIWidgetMac_MOZILLA_1_8_BRANCH as "scriptable", but I would need to make its first two methods [noscript].

Comment 8

[Mike Connor [:mconnor]]

Comment on attachment 236958 [details] [diff] [review]
1.8.1 branch patch

a=mconnor on behalf of drivers for 1.8 branch checkin