Closed
Bug 1200549
Opened 9 years ago
Closed 9 years ago
imtooltip.xml, line 230: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getCharPref]
Categories
(Thunderbird :: Instant Messaging, defect)
Thunderbird
Instant Messaging
Tracking
(thunderbird42 unaffected, thunderbird43 fixed, thunderbird44 fixed)
RESOLVED
FIXED
Thunderbird 44.0
Tracking | Status | |
---|---|---|
thunderbird42 | --- | unaffected |
thunderbird43 | --- | fixed |
thunderbird44 | --- | fixed |
People
(Reporter: aleth, Assigned: aleth)
References
Details
(Keywords: regression)
Attachments
(2 files, 5 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
aleth
:
review+
rkent
:
approval-comm-aurora+
|
Details | Diff | Splinter Review |
The chat.prpls.forcePurple pref doesn't currently exist for Thunderbird, as libpurple is not shipped by default there.
Comment 1•9 years ago
|
||
Comment 2•9 years ago
|
||
Why is that pref needed on Thunderbird?
Assignee | ||
Comment 3•9 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!
Comment 4•9 years ago
|
||
(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•9 years ago
|
||
Attachment #8658346 -
Attachment is obsolete: true
Attachment #8658346 -
Flags: review?(aleth)
Attachment #8658354 -
Flags: review?(aleth)
Comment 6•9 years ago
|
||
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•9 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•9 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•9 years ago
|
||
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 10•9 years ago
|
||
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 11•9 years ago
|
||
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+
Updated•9 years ago
|
Assignee: a.ahmed1026 → aleth
Assignee | ||
Comment 12•9 years ago
|
||
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•9 years ago
|
||
(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•9 years ago
|
||
Attachment #8660131 -
Attachment is obsolete: true
Attachment #8661740 -
Flags: review?(clokep)
Assignee | ||
Comment 15•9 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•9 years ago
|
||
Attachment #8661740 -
Attachment is obsolete: true
Attachment #8661745 -
Flags: review?(clokep)
Comment 17•9 years ago
|
||
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•9 years ago
|
||
https://hg.mozilla.org/comm-central/rev/8ffef885f4b502eaedf95e260237f59c1b9ed3d6
Bug 1200549 - Only use requestBuddyInfo in tooltips for JS prpls. r=clokep
Assignee | ||
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 44.0
Assignee | ||
Comment 19•9 years ago
|
||
[Approval Request Comment]
Regression affects TB43.
Attachment #8661745 -
Attachment is obsolete: true
Attachment #8664578 -
Flags: review+
Attachment #8664578 -
Flags: approval-comm-aurora?
Comment 20•9 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•9 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.
Description
•