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

RESOLVED FIXED in Thunderbird 44.0

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: aleth, Assigned: aleth)

Tracking

({regression})

Trunk
Thunderbird 44.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird42 unaffected, thunderbird43 fixed, thunderbird44 fixed)

Details

Attachments

(2 attachments, 5 obsolete attachments)

The chat.prpls.forcePurple pref doesn't currently exist for Thunderbird, as libpurple is not shipped by default there.
Assignee: nobody → a.ahmed1026
Status: NEW → ASSIGNED
Attachment #8658346 - Flags: review?(aleth)
Why is that pref needed on Thunderbird?
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!
(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
Attachment #8658346 - Attachment is obsolete: true
Attachment #8658346 - Flags: review?(aleth)
Attachment #8658354 - Flags: review?(aleth)
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.
Attachment #8658354 - Flags: review-
(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.
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.
Posted patch request4all.diff (obsolete) — Splinter Review
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)
Attachment #8658354 - Attachment is obsolete: true
Attachment #8658354 - Flags: review?(aleth)
Attachment #8660131 - Flags: review?(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.
Attachment #8660131 - Flags: review?(clokep) → review?(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.
Attachment #8660131 - Flags: review?(florian) → review+
Assignee: a.ahmed1026 → aleth
Posted image 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"?
(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.
Attachment #8660131 - Attachment is obsolete: true
Attachment #8661740 - Flags: review?(clokep)
Comment on attachment 8661740 [details] [diff] [review]
Only use requestBuddyInfo for JS prpls

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

Typo.
Attachment #8661740 - Flags: review?(clokep) → review-
Attachment #8661740 - Attachment is obsolete: true
Attachment #8661745 - Flags: review?(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. ;)
Attachment #8661745 - Flags: review?(clokep) → review+
https://hg.mozilla.org/comm-central/rev/8ffef885f4b502eaedf95e260237f59c1b9ed3d6
Bug 1200549 - Only use requestBuddyInfo in tooltips for JS prpls. r=clokep
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 44.0
[Approval Request Comment]
Regression affects TB43.
Attachment #8661745 - Attachment is obsolete: true
Attachment #8664578 - Flags: review+
Attachment #8664578 - Flags: approval-comm-aurora?
Comment on attachment 8664578 [details] [diff] [review]
Patch, checked in version

http://hg.mozilla.org/releases/comm-aurora/rev/21eeb3aa2f96
Attachment #8664578 - Flags: approval-comm-aurora? → approval-comm-aurora+
You need to log in before you can comment on or make changes to this bug.