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

RESOLVED FIXED in Thunderbird 44.0

Status

Thunderbird
Instant Messaging
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: aleth, Assigned: aleth)

Tracking

({regression})

Trunk
Thunderbird 44.0
regression

Thunderbird Tracking Flags

(thunderbird42 unaffected, thunderbird43 fixed, thunderbird44 fixed)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

2 years ago
The chat.prpls.forcePurple pref doesn't currently exist for Thunderbird, as libpurple is not shipped by default there.
Created attachment 8658346 [details] [diff] [review]
rev 1 - add forcePurple pref for TB
Assignee: nobody → a.ahmed1026
Status: NEW → ASSIGNED
Attachment #8658346 - Flags: review?(aleth)
Why is that pref needed on Thunderbird?
(Assignee)

Comment 3

2 years ago
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
Created attachment 8658354 [details] [diff] [review]
rev 2 - add forcePurple pref for TB
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-
(Assignee)

Comment 7

2 years ago
(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.
(Assignee)

Comment 8

2 years ago
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.
(Assignee)

Comment 9

2 years ago
Created attachment 8660131 [details] [diff] [review]
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)
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
(Assignee)

Comment 12

2 years ago
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"?
(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.
(Assignee)

Comment 14

2 years ago
Created attachment 8661740 [details] [diff] [review]
Only use requestBuddyInfo for JS prpls
Attachment #8660131 - Attachment is obsolete: true
Attachment #8661740 - Flags: review?(clokep)
(Assignee)

Comment 15

2 years ago
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-
(Assignee)

Comment 16

2 years ago
Created attachment 8661745 [details] [diff] [review]
Only use requestBuddyInfo for JS prpls v2
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+
(Assignee)

Comment 18

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

Updated

2 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 44.0
(Assignee)

Comment 19

2 years ago
Created attachment 8664578 [details] [diff] [review]
Patch, checked in version

[Approval Request Comment]
Regression affects TB43.
Attachment #8661745 - Attachment is obsolete: true
Attachment #8664578 - Flags: review+
Attachment #8664578 - Flags: approval-comm-aurora?

Comment 20

2 years ago
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+

Updated

2 years ago
status-thunderbird42: --- → unaffected
status-thunderbird43: --- → fixed
status-thunderbird44: --- → fixed
You need to log in before you can comment on or make changes to this bug.