Closed Bug 1150674 Opened 5 years ago Closed 6 months ago

Remove trivial uses of nsIProgrammingLanguage from comm-central

Categories

(MailNews Core :: Backend, task)

task
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 68.0

People

(Reporter: mccr8, Assigned: clokep)

References

Details

Attachments

(1 file, 1 obsolete file)

I'm working towards removing nsIProgrammingLanguage.h from mozilla-central.  Right now, it looks like none of these changes should break comm-central, but eventually they will require fixing.

- In bug 1148070, I changed |nsIClassInfo::getHelperForLanguage(foo)| to |getXPCHelper()|.  There are a few places that define this method.  Just change the methods |getHelperForLanguage()| that take an argument and return null to methods called |getScriptableHelper()| that take no arguments and return null.  I'm not sure it even really matters if you don't define it.

- In bug 1147572, I removed GetImplementationLanguage from nsIClassInfo.  Any place that is implementing classinfo and uses a line that looks like this can be deleted:
  implementationLanguage: Components.interfaces.nsIProgrammingLanguage.JAVASCRIPT

- /calendar/base/backend/libical/calUtils.h includes nsIProgrammingLanguage.h but doesn't use it.
Blocks: 1147947
Most of the uses seem to be in chat. The one odd occurrence in /suite/feeds also does not seem to deserve classifying this under mailnews core/feed reader.

Andrew, thanks for notifying us.

1)
/chat/components/src/imContacts.js
    line 241 -- getHelperForLanguage: function(language) null,
    line 372 -- getHelperForLanguage: function(language) null,
    line 903 -- getHelperForLanguage: function(language) null,
    line 1218 -- getHelperForLanguage: function(language) null,

/chat/modules/imXPCOMUtils.jsm 
    line 197 -- getHelperForLanguage: function(language) null,


2)
/chat/components/src/imContacts.js
    line 242 -- implementationLanguage: Ci.nsIProgrammingLanguage.JAVASCRIPT,
    line 373 -- implementationLanguage: Ci.nsIProgrammingLanguage.JAVASCRIPT,
    line 904 -- implementationLanguage: Ci.nsIProgrammingLanguage.JAVASCRIPT,
    line 1219 -- implementationLanguage: Ci.nsIProgrammingLanguage.JAVASCRIPT,

/chat/modules/imXPCOMUtils.jsm
    line 200 -- implementationLanguage: Ci.nsIProgrammingLanguage.JAVASCRIPT,

/suite/feeds/src/FeedConverter.js 
    line 132 -- implementationLanguage: Components.interfaces.nsIProgrammingLanguage.JAVASCRIPT,
    line 463 -- implementationLanguage: Components.interfaces.nsIProgrammingLanguage.JAVASCRIPT
    line 530 -- implementationLanguage: Components.interfaces.nsIProgrammingLanguage.JAVASCRIPT
Component: Feed Reader → Backend
Version: unspecified → 39
Attached patch Chat part v1 (obsolete) — Splinter Review
Attachment #8589571 - Flags: review?(florian)
Comment on attachment 8589571 [details] [diff] [review]
Chat part v1

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

::: chat/components/src/imContacts.js
@@ +1191,5 @@
>  
>  
>  function ContactsService() { }
>  ContactsService.prototype = {
> +  __proto__: ClassInfo("imIContactsService", "Contacts"),

I don't think services need to implement nsIClassInfo.

::: chat/modules/imXPCOMUtils.jsm
@@ +193,5 @@
>        [Ci.nsIClassInfo, Ci.nsISupports].concat(this._interfaces);
>      countRef.value = interfaces.length;
>      return interfaces;
>    },
> +  getXPCHelper: function() null,

It looks like this is called getScriptableHelper, not getXPCHelper.
Attachment #8589571 - Flags: review?(florian) → review-
It looks like emk is going to remove nsIProgrammingLanguage in bug 1149830, so you'll need to fix this in Thunderbird.

(In reply to Andrew McCreight [:mccr8] from comment #4)

It looks like emk is going to remove nsIProgrammingLanguage in bug 1149830,
so you'll need to fix this in Thunderbird.

bug 1149830 was done in 57. Do we still need to complete our piece?

Type: defect → task
Flags: needinfo?(clokep)

Bug 1387706 removed the nsIProgrammingLanguage stuff (that ported bug 1149830). Some of this was also handled in bug 1529287 (which ported bug 1526382).

It looks like there's still a getHelperForLanguage in chat/modules/imXPCOMUtils.jsm. I think this can just be removed, but I'm not 100% sure. Florian had previously suggested changing this to getScriptableHelper, but that doesn't seem to be used anywhere. Anyone know if that is a thing? (I also see bug 1161674 and bug 1322930?)

Flags: needinfo?(clokep) → needinfo?(florian)

getHelperForLanguage doesn't exist anywhere else in m-c or c-c, so yeah, it can certainly be removed. Bug 1161674 is puzzling, but so old that I don't think it matters anymore, I just closed it as WFM.

getScriptableHelper is part of the nsIClassInfo interface, https://searchfox.org/comm-central/rev/43cb0b22a7c2702ef98aebb1bf99a7a6197ea340/mozilla/xpcom/components/nsIClassInfo.idl#31 says:

   /**
     * Return an object to assist XPConnect in supplying JavaScript-specific
     * behavior to callers of the instance object, or null if not needed.
     */
    nsIXPCScriptable getScriptableHelper();

We should just do that, ie change
getHelperForLanguage: language => null,
to
getScriptableHelper: () => null,

Flags: needinfo?(florian)
Attached patch Chat part v2Splinter Review
Attachment #8589571 - Attachment is obsolete: true
Attachment #9065182 - Flags: review?(florian)
Comment on attachment 9065182 [details] [diff] [review]
Chat part v2

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

Thanks!
Attachment #9065182 - Flags: review?(florian) → review+

Pushed by clokep@gmail.com:
https://hg.mozilla.org/comm-central/rev/13360f79c2b5
Rename getHelperForLanguage to getScriptableHelper. r=florian

Status: NEW → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Assignee: nobody → clokep
Target Milestone: --- → Thunderbird 68.0
You need to log in before you can comment on or make changes to this bug.