1529287 - Port more of bug 1526382 to Thunderbird chat

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, task)

Description

[Patrick Cloke [:clokep]]

Bug 1528907 started this by updating the implementation of ClassInfo, but that isn't used everywhere in chat, so those need to be updated manually.

See https://searchfox.org/comm-central/source/chat/components/src/imContacts.js

I think we can grep for getInterfaces here, changes should be easy, similar to: https://hg.mozilla.org/comm-central/rev/433ea630bdb8.

Comment 1

[Patrick Cloke [:clokep]]

Attachment: Uses ClassInfo in imContacts.js

Using grep it doesn't seem that we have anymore references to getInterfaces.

I'm not 100% sure I properly used ClassInfo, but this made some errors going away.

Comment 2

[Florian Quèze [:florian]]

Comment on attachment 9045256 [details] [diff] [review]
Uses ClassInfo in imContacts.js

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

The ClassInfo helper automatically adds the nsIClassInfo and nsISupports interfaces.

When there's a single interface, you don't have to put it in an array.

Would you like to add a description for all cases, or are we happy with the default "JS Proto Object" description?

Comment 3

[Patrick Cloke [:clokep]]

Attachment: Uses ClassInfo in imContacts.js v2

Handles review comments. I was not 100% sure about leaving classID and contractID on each of these, but since there's no other way to provide those I assume we need to leave them on each class?

Comment 4

[Florian Quèze [:florian]]

Comment on attachment 9046680 [details] [diff] [review]
Uses ClassInfo in imContacts.js v2

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

(In reply to Patrick Cloke [:clokep] from comment #3)

> Created attachment 9046680 [details] [diff] [review]
> Uses ClassInfo in imContacts.js v2
> 
> Handles review comments. I was not 100% sure about leaving `classID` and `contractID` on each of these, but since there's no other way to provide those I assume we need to leave them on each class?

Uh, I didn't pay enough attention during the previous review, sorry. The cases where you have a classID and contractID are services that we access through getService where we already explicitly specify the interface we request, so I don't think implementing nsIClassInfo on these is useful.

Let's revert the changes for the ContactsService and TagsService, sorry :-/.

Comment 5

[Patrick Cloke [:clokep]]

Attachment: Patch v3

This reverts the changes to the TagsService and ContactsService.

Comment 6

[Florian Quèze [:florian]]

Comment on attachment 9046725 [details] [diff] [review]
Patch v3

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

Looks good to me. I wonder how come ClassInfo was already imported but unused in this file, and didn't cause eslint to complain.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cc990b014a70
Port more of bug 1526382 to Thunderbird chat. r=florian