Closed Bug 350080 Opened 14 years ago Closed 14 years ago

nsPIWidgetMac.idl uses two non-IDL defined interfaces

Categories

(Core Graveyard :: Widget: Mac, defect)

PowerPC
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jhpedemonte, Assigned: jaas)

References

Details

(Keywords: fixed1.8.1)

Attachments

(3 files, 1 obsolete file)

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)?
AFAIK none of nsPIWidget needs to be scriptable. Just get rid of "scriptable" here:

[scriptable, uuid(5DE488F0-C9B1-427C-938F-D3D13DEFF987)]
Attached patch patchSplinter Review
Make nsPIWidgetMac non-scriptable.  Josh agrees with this patch.  Mark, what do you think?
Attachment #235285 - Flags: review?(mark)
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.
Attachment #235285 - Flags: review?(mark) → review+
Attachment #235285 - Flags: review+
Attached patch patch checked inSplinter Review
This is the patch I checked in, with Mark's suggestiong for "[ptr] native".
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Attached patch 1.8.1 branch patch (obsolete) — Splinter Review
Seeking 1.8.1 approval.  This patch is needed by patch in bug 333618.
Attachment #236824 - Flags: approval1.8.1?
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.
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].
Attachment #236824 - Attachment is obsolete: true
Attachment #236958 - Flags: approval1.8.1?
Attachment #236824 - Flags: approval1.8.1?
Comment on attachment 236958 [details] [diff] [review]
1.8.1 branch patch

a=mconnor on behalf of drivers for 1.8 branch checkin
Attachment #236958 - Flags: approval1.8.1? → approval1.8.1+
Keywords: fixed1.8.1
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.