Closed Bug 954636 Opened 9 years ago Closed 9 years ago

buddy icons missing in JS-XMPP messages

Categories

(Instantbird :: Conversation, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: florian)

References

Details

Attachments

(1 file, 1 obsolete file)

*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-11 05:37:00 UTC ***

Since probably about 12/7/2011, the biddy icons have been missing for buddies in the conversation window.  This is evident when using a theme that supports  buddy icons.

Basically,  IB has reverted to its old standard regarding buddy icons in conversations.
*** Original post on bio 1204 at 2011-12-11 09:39:29 UTC ***

On which protocol is this?
*** Original post on bio 1204 at 2011-12-11 09:51:13 UTC ***

I can't confirm this on twitter at least.

Do you mean the icons in the messages themselves, or the icon in the bar at the top of the conversation, or both?
*** Original post on bio 1204 at 2011-12-11 11:16:51 UTC ***

My guess is he means the icons in the messages (for message themes that display them) on a protocol implemented by JS-XMPP (Facebook chat or Google Talk), but I would like a confirmation :).
*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-11 13:08:58 UTC ***

Yes, as flo explained.  Thank you.
*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-11 13:17:34 UTC ***

It is not a problem on Twitter.  Also, the conversation header is fine.
*** Original post on bio 1204 at 2011-12-12 10:44:29 UTC ***

Ok, I changed the summary to clarify.
Blocks: 954603
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86 → All
Summary: buddy conversation missing in recent nightly builds. → buddy icons missing in JS-XMPP messages
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1204 as attmnt 1078 at 2011-12-12 10:46:00 UTC ***

This implements in jsProtoHelper the behavior that was implemented in purpleMessage::GetIconURL (http://lxr.instantbird.org/instantbird/source/purple/purplexpcom/src/purpleMessage.cpp#132).

I haven't tested it at all, I just wrote the code.
Attachment #8352820 - Flags: review?(clokep)
Assignee: nobody → florian
Status: NEW → ASSIGNED
Comment on attachment 8352820 [details] [diff] [review]
Patch

*** Original change on bio 1204 attmnt 1078 at 2011-12-12 11:38:22 UTC ***

This looks good. I also didn't test this, unfortunately, since I'm not sure of a message style that has buddy icons in it.

(And I was wondering why it DOES work in Twitter: we're directly setting the iconURL of each message to the user's profile pic at http://lxr.instantbird.org/instantbird/source/chat/protocols/twitter/twitter.js#301 .)
Attachment #8352820 - Flags: review?(clokep) → review+
Attached patch Patch v2Splinter Review
*** Original post on bio 1204 as attmnt 1079 at 2011-12-12 11:53:00 UTC ***

This time it shouldn't break twitter icons... :)
Attachment #8352821 - Flags: review?(clokep)
Comment on attachment 8352820 [details] [diff] [review]
Patch

*** Original change on bio 1204 attmnt 1078 at 2011-12-12 11:53:31 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352820 - Attachment is obsolete: true
Comment on attachment 8352821 [details] [diff] [review]
Patch v2

*** Original change on bio 1204 attmnt 1079 at 2011-12-12 12:00:02 UTC ***

This actually looks good this time. :)
Attachment #8352821 - Flags: review?(clokep) → review+
*** Original post on bio 1204 at 2011-12-12 14:36:01 UTC ***

Before pushing I added a check on this.incoming to only add the icon of the conversation target on incoming messages.

Fix pushed as https://hg.instantbird.org/instantbird/rev/c5d76309b6bb
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-12 16:46:49 UTC ***

clokep, here is a message style I use that has icons, if you desire to experiment.  Thank you all for looking into this.

https://addons.instantbird.org/en-US/instantbird/addon/64
*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-14 13:16:26 UTC ***

I am in  no  way meaning to rush, but I read above that a fix was pushed but so far I have not seen any change.  So posting this update, just in case it was already pushed and expected to be working. 

version 1.2a1pre (20111213135837)
*** Original post on bio 1204 at 2011-12-14 13:31:46 UTC ***

I tested this before pushing the change, so I'm quite surprised. Can anybody else reproduce the issue deOmega is seeing?
*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-14 20:40:05 UTC ***

I have tested it with  multiple message styles and the results are the same.  This includes the new message style... https://addons.instantbird.org/en-US/instantbird/addon/311

I see my  icon in outgoing messages, and incoming messages work for twitter, but  no  incoming  message icons for any other protocol:msn, gtalk, yahoo,  aim or Facebook.
*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-19 16:48:09 UTC ***

correction:  Works on MSN and TWITTER.
*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-19 18:03:48 UTC ***

Working as of today's update.  version 1.2a1pre (20111219154444)   Thank you very much.
*** Original post on bio 1204 at 2011-12-19 18:08:08 UTC ***

On all protocols? :)
*** Original post on bio 1204 by deOmega <jahkae AT gmail.com> at 2011-12-20 21:47:46 UTC ***

Yes, working on all protocols now. :)
You need to log in before you can comment on or make changes to this bug.