1200549 - imtooltip.xml, line 230: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getCharPref]

Status: RESOLVED FIXED

(Thunderbird :: Instant Messaging, defect)

Description

[aleth [:aleth]]

The chat.prpls.forcePurple pref doesn't currently exist for Thunderbird, as libpurple is not shipped by default there.

Comment 1

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 1 - add forcePurple pref for TB

Comment 2

[Florian Quèze [:florian]]

Why is that pref needed on Thunderbird?

Comment 3

[aleth [:aleth]]

Comment on attachment 8658346 [details] [diff] [review]
rev 1 - add forcePurple pref for TB

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

::: mail/app/profile/all-thunderbird.js
@@ +860,5 @@
>  // calendar promotion status
>  pref("mail.calendar-integration.opt-out", false);
> +
> +// This is used to enable JS-prpls or its features, but it will be disabled if
> +// its value contains prpl-jabber.

Please use the same comment as in all-instantbird.js, thanks!

Comment 4

[Abdelrahman Ahmed [:abdelrahman]]

(In reply to Florian Quèze [:florian] [:flo] from comment #2)
> Why is that pref needed on Thunderbird?

We use this pref to check JS-XMPP is enabled [1] to request information for tooltips.
[1] https://dxr.mozilla.org/comm-central/source/chat/content/imtooltip.xml#230

Comment 5

[Abdelrahman Ahmed [:abdelrahman]]

Attachment: rev 2 - add forcePurple pref for TB

Comment 6

[Patrick Cloke [:clokep]]

Comment on attachment 8658354 [details] [diff] [review]
rev 2 - add forcePurple pref for TB

This seems like the wrong approach. Thunderbird purposefully does not need this pref.

Comment 7

[aleth [:aleth]]

(In reply to Patrick Cloke [:clokep] from comment #6)
> This seems like the wrong approach. Thunderbird purposefully does not need
> this pref.

Feel free to suggest the right approach.

I'd be OK with #ifdef MOZ_THUNDERBIRD, as the file is (sadly) already preprocessed.

I'd also be OK with simplifying the hack and assuming that all JS prpls either return displayable requestBuddyInfo results or none at all. But then you need a non-hacky way of checking whether a prpl is a JS prpl.

Comment 8

[aleth [:aleth]]

OK, so after some discussion, we're not going to add the pref to TB. #ifdef is out because of the TB libpurple addon.

So we'll go with calling requestBuddyInfo for all JS prpls. You can check for this using implementationLanguage in nsIClassInfo, which the protocol object implements (https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/nsIClassInfo).

The default JS implementation seems to be slightly wrong (https://dxr.mozilla.org/comm-central/source/chat/modules/jsProtoHelper.jsm#181), it should send back an empty enumerator.

Comment 9

[aleth [:aleth]]

Attachment: request4all.diff

OK, we've discussed this a bit and maybe the simplest thing to do is simply to turn on requestBuddyInfo in tooltips for buddies on all protocols, and see if anyone complains.

I suspect this won't be a good idea for XMPP (unfiltered vcards can be quite long and can contain icon data) if we don't pref on JS-XMPP for the next release.

Alternatively/additionally we could 
1) make requestBuddyInfo never return anything for libpurple prpls (it's only called for tooltips)
2) only call if (!!prpl.wrappedJSObject)

Comment 10

[Patrick Cloke [:clokep]]

Comment on attachment 8660131 [details] [diff] [review]
request4all.diff

I'm going to bump this to Florian, he likely has a longer memory on this code than I do.

Comment 11

[Florian Quèze [:florian]]

Comment on attachment 8660131 [details] [diff] [review]
request4all.diff

This doesn't seem exactly right, but I don't want to block this as I don't have strong feelings either way.

Comment 12

[aleth [:aleth]]

Attachment: Screen Shot

An example of a requestBuddyInfo tooltip from libpurple XMPP.

What do we think of this? "Not too bad really" or "better not use it"?

Comment 13

[Florian Quèze [:florian]]

(In reply to aleth [:aleth] from comment #12)
> Created attachment 8661498 [details]
> Screen Shot
> 
> An example of a requestBuddyInfo tooltip from libpurple XMPP.
> 
> What do we think of this? "Not too bad really" or "better not use it"?

I'm more on the 'better not use it' side, especially due to the HTML markup being shown as plain text.

Comment 14

[aleth [:aleth]]

Attachment: Only use requestBuddyInfo for JS prpls

Comment 15

[aleth [:aleth]]

Comment on attachment 8661740 [details] [diff] [review]
Only use requestBuddyInfo for JS prpls

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

Typo.

Comment 16

[aleth [:aleth]]

Attachment: Only use requestBuddyInfo for JS prpls v2

Comment 17

[Patrick Cloke [:clokep]]

Comment on attachment 8661745 [details] [diff] [review]
Only use requestBuddyInfo for JS prpls v2

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

r+ if you add to the comment saying this is a terrible terrible hack. ;)

Comment 18

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/8ffef885f4b502eaedf95e260237f59c1b9ed3d6
Bug 1200549 - Only use requestBuddyInfo in tooltips for JS prpls. r=clokep

Comment 19

[aleth [:aleth]]

Attachment: Patch, checked in version

[Approval Request Comment]
Regression affects TB43.

Comment 20

[Kent James (:rkent)]

Comment on attachment 8664578 [details] [diff] [review]
Patch, checked in version

http://hg.mozilla.org/releases/comm-aurora/rev/21eeb3aa2f96