677529 - Remove manual jspubtd.h inclusions from IDL files

Status: RESOLVED FIXED

(Core :: General, defect)

Comment 1

[Matheus Kerschbaum]

Attachment: wip

This should work, except it doesn't. Am I missing something here or does Bug 676649 not work as I understand it (i.e automatically including jspubtd.h if jsval is used as a type in an IDL)?

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

What went wrong? It's entirely possible I missed things like [ptr] native JSValPtr(jsval); in nsIJSON.idl.

Comment 3

[Matheus Kerschbaum]

Attachment: patch

This one works.

(In reply to Ms2ger from comment #2)
> What went wrong? It's entirely possible I missed things like [ptr] native
> JSValPtr(jsval); in nsIJSON.idl.

nsIJSON is fine, possibly because no one uses it anymore IIRC. Other interfaces had problems though:

http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/nsIXPConnect.idl#69 doesn't work.

http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/nsIXPCSecurityManager.idl doesn't work, even though it doesn't use jsval.

http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsIVariant.idl#118 doesn't work.

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Matheus Kerschbaum from comment #3)
> Created attachment 552481 [details] [diff] [review]
> patch
> 
> This one works.
> 
> (In reply to Ms2ger from comment #2)
> > What went wrong? It's entirely possible I missed things like [ptr] native
> > JSValPtr(jsval); in nsIJSON.idl.
> 
> nsIJSON is fine, possibly because no one uses it anymore IIRC. Other
> interfaces had problems though:
> 
> http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/
> nsIXPConnect.idl#69 doesn't work.
> 
> http://mxr.mozilla.org/mozilla-central/source/js/src/xpconnect/idl/
> nsIXPCSecurityManager.idl doesn't work, even though it doesn't use jsval.

I think it's file to leave these two with manual inclusions.

> http://mxr.mozilla.org/mozilla-central/source/xpcom/ds/nsIVariant.idl#118
> doesn't work.

Looks like I didn't check return values; patch coming up.

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch to check method return values

Could you check if this fixes nsIVariant for you?

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

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

LGTM, but don't get this landed before comm-central uses pyidl.

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

comm-central is using pyidl for interfaces in mozilla-central.  It's only interfaces in comm-central that are being built with xpidlc.

Comment 8

[Matheus Kerschbaum]

(In reply to Ms2ger from comment #5)
> Created attachment 552863 [details] [diff] [review]
> Patch to check method return values
> 
> Could you check if this fixes nsIVariant for you?

I don't have bugzilla rights to mark this as f+, but the patch does work.

Comment 9

[Matheus Kerschbaum]

Attachment: patch

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Comment on attachment 552863 [details] [diff] [review]
Patch to check method return values

Great, thanks.

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

http://hg.mozilla.org/try/rev/2b7f988f5a6a
http://hg.mozilla.org/try/rev/e0634a052db4