Closed
Bug 955570
Opened 10 years ago
Closed 10 years ago
Sender name not set in Yahoo conversations
Categories
(Chat Core :: Yahoo! Messenger, defect)
Chat Core
Yahoo! Messenger
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: aleth, Assigned: qheaden)
References
Details
Attachments
(2 files, 1 obsolete file)
2.95 KB,
image/png
|
Details | |
1.20 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2132 at 2013-08-27 13:17:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Reporter | ||
Comment 1•10 years ago
|
||
*** Original post on bio 2132 as attmnt 2792 at 2013-08-27 13:17:00 UTC *** STR Receive a message from a Yahoo contact
Reporter | ||
Updated•10 years ago
|
Summary: Sender name not set correctly in Yahoo conversations → Sender name not set in Yahoo conversations
Reporter | ||
Comment 2•10 years ago
|
||
*** Original post on bio 2132 at 2013-08-27 13:28:48 UTC *** To clarify: It's the user's own name that is not set. The name of the contact one is talking to is set correctly. The STR are to indicate that an incoming message opened the conversation (if that makes a difference -- it shouldn't).
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 2132 as attmnt 2801 at 2013-08-29 02:33:00 UTC *** Good catch aleth! This patch fixes the issue, and I placed the sender name in a separate variable to make the code cleaner instead of putting it inline with the function call.
Attachment #8354571 -
Flags: review?(clokep)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → qheaden
Status: NEW → ASSIGNED
Comment 4•10 years ago
|
||
Comment on attachment 8354571 [details] [diff] [review] Patch 1 *** Original change on bio 2132 attmnt 2801 at 2013-08-29 10:39:18 UTC *** >+ let senderName = this._account.imAccount.alias || this._account.cleanUsername; Should this be cleanUsername or the real username? Also, I don't think you should be touching imAccount here.
Attachment #8354571 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 2132 at 2013-08-29 14:33:58 UTC *** (In reply to comment #3) > Comment on attachment 8354571 [details] [diff] [review] (bio-attmnt 2801) [details] > Patch 1 > > >+ let senderName = this._account.imAccount.alias || this._account.cleanUsername; > Should this be cleanUsername or the real username? Also, I don't think you > should be touching imAccount here. I think it should be cleanUsername. Remember that JS-Yahoo allows people to put their @yahoo.* address in their username. I don't think that should show next to each message sent. cleanUsername strips that.
Comment 6•10 years ago
|
||
*** Original post on bio 2132 at 2013-08-29 15:00:12 UTC *** (In reply to comment #4) > (In reply to comment #3) > > Comment on attachment 8354571 [details] [diff] [review] (bio-attmnt 2801) [details] > > Patch 1 > > > > >+ let senderName = this._account.imAccount.alias || this._account.cleanUsername; > > Should this be cleanUsername or the real username? Also, I don't think you > > should be touching imAccount here. > > I think it should be cleanUsername. Remember that JS-Yahoo allows people to put > their @yahoo.* address in their username. I don't think that should show next > to each message sent. cleanUsername strips that. Yes, I remember that is what it's doing, my thought process was that it should match what the user typed in. I'm OK using this though. See [1] for using the alias. [1] http://log.bezut.info/instantbird/today/#m177
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 2132 as attmnt 2803 at 2013-08-29 20:39:00 UTC *** This patch uses the new preferred way of using the user's alias as a sender name.
Attachment #8354573 -
Flags: review?(aleth)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8354571 [details] [diff] [review] Patch 1 *** Original change on bio 2132 attmnt 2801 at 2013-08-29 20:39:03 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354571 -
Attachment is obsolete: true
Reporter | ||
Comment 9•10 years ago
|
||
*** Original post on bio 2132 at 2013-08-30 09:56:42 UTC *** Do you know how setting the _alias flag ends up in msg.alias? (I see XMPP sets it this way too, but it puzzles me.)
Comment 10•10 years ago
|
||
*** Original post on bio 2132 at 2013-08-30 10:19:52 UTC *** (In reply to comment #7) > Do you know how setting the _alias flag ends up in msg.alias? (I see XMPP sets > it this way too, but it puzzles me.) http://lxr.instantbird.org/instantbird/source/chat/modules/jsProtoHelper.jsm#382 382 _alias: "", 383 get alias() this._alias || this.who,
Reporter | ||
Comment 11•10 years ago
|
||
Comment on attachment 8354573 [details] [diff] [review] Patch 2 *** Original change on bio 2132 attmnt 2803 at 2013-08-30 10:24:14 UTC *** Thanks!
Attachment #8354573 -
Flags: review?(aleth) → review+
Reporter | ||
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 12•10 years ago
|
||
Comment on attachment 8354573 [details] [diff] [review] Patch 2 *** Original change on bio 2132 attmnt 2803 at 2013-08-30 10:28:56 UTC *** See how XMPP does it: http://lxr.instantbird.org/instantbird/source/chat/protocols/xmpp/xmpp.jsm#259 259 let alias = this.account.alias || this.account.statusInfo.displayName;
Attachment #8354573 -
Flags: review+ → review-
Reporter | ||
Comment 13•10 years ago
|
||
*** Original post on bio 2132 at 2013-08-30 10:37:27 UTC *** (In reply to comment #10) > 259 let alias = this.account.alias || this.account.statusInfo.displayName; If we decide to do this generally (rather than just for XMPP) we should discuss it, abstract it and do it in jsProtoHelper. That would be a separate bug imho. An argument against would be that the displayName is not visible to the receiving end of the conversation in Yahoo, and there is then nothing showing the username (which the receiving end uses for the same purpose) on the IB end.
Comment 14•10 years ago
|
||
Comment on attachment 8354573 [details] [diff] [review] Patch 2 *** Original change on bio 2132 attmnt 2803 at 2013-08-30 10:44:22 UTC *** Removing my r-, I looked at this a bit too quickly, sorry. Showing the display name in the conversation makes sense only if we sent the display name to the server and the remote contact is likely to see the display name in the conversation too. It appears you aren't sending the display name to the Yahoo server. Is this a bug, or something the protocol doesn't support? Anyway, if it's a bug, you can add the display name as alias on the messages at the same time as you send it to the server in a follow up bug.
Attachment #8354573 -
Flags: review-
Reporter | ||
Comment 15•10 years ago
|
||
Comment on attachment 8354573 [details] [diff] [review] Patch 2 *** Original change on bio 2132 attmnt 2803 at 2013-08-31 12:36:30 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354573 -
Flags: review+
Comment 16•10 years ago
|
||
*** Original post on bio 2132 at 2013-09-06 00:18:03 UTC *** http://hg.instantbird.org/instantbird/rev/9db515149f68 Please answer the following: (In reply to comment #12) > It appears you aren't sending the display name to the Yahoo server. Is this a > bug, or something the protocol doesn't support? Anyway, if it's a bug, you can > add the display name as alias on the messages at the same time as you send it > to the server in a follow up bug.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
Assignee | ||
Comment 17•10 years ago
|
||
*** Original post on bio 2132 at 2013-09-06 05:43:26 UTC ***
> Please answer the following:
> (In reply to comment #12)
> > It appears you aren't sending the display name to the Yahoo server. Is this a
> > bug, or something the protocol doesn't support? Anyway, if it's a bug, you can
> > add the display name as alias on the messages at the same time as you send it
> > to the server in a follow up bug.
I think its a bit complicated. I'm not sure if you can send any random display name. Even so, I don't think it would be a good thing, as people could take on the usernames of other Yahoo accounts, which would be confusing. Yahoo profiles do support aliases, but to work with them, I think you need to start messing around with one or more of Yahoo's APIs, which is another complicated issue.
You need to log in
before you can comment on or make changes to this bug.
Description
•