Closed Bug 954160 Opened 8 years ago Closed 8 years ago

In Message Styles preview are emails shown instead of nicks

Categories

(Instantbird :: Preferences, defect)

defect
Not set
trivial

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bugzilla, Assigned: florian)

Details

Attachments

(2 files, 1 obsolete file)

*** Original post on bio 725 by Michal Stanke <michal.stanke AT mikk.cz> at 2011-03-05 19:58:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Attached image screenshot
*** Original post on bio 725 as attmnt 551 by michal.stanke AT mikk.cz at 2011-03-05 19:58:00 UTC ***

0.3a2pre (20110304041807)
*** Original post on bio 725 at 2011-03-05 22:02:08 UTC ***

I don't really see this as a bug, a lot of protocols use email style names for the login names. (XMPP for example does this.)
*** Original post on bio 725 at 2011-03-06 00:02:32 UTC ***

Well, it's a bug in the sense that it's not the behavior the developer expected, and it makes some localized strings (the first names of the 2 people talking in the preview conversation) useless.

The cause is probably a trivial mistake in https://hg.instantbird.org/instantbird/rev/29458856cc04.
Even though the bug is not really harmful from a user point of view, I would like to know what the mistake is.
Severity: normal → trivial
Status: UNCONFIRMED → NEW
Ever confirmed: true
*** Original post on bio 725 by Michal Stanke <michal.stanke AT mikk.cz> at 2011-03-06 09:03:05 UTC ***

new Message(msg.nick1, msg.message1, {outgoing: true, who: msg.buddy1, time: makeDate("10:42:22"), _conversation: conv}),
new Message(msg.nick1, msg.message2, {outgoing: true, who: msg.buddy1, time: makeDate("10:42:25"), _conversation: conv}),
new Message(msg.nick2, msg.message3, {incoming: true, who: msg.buddy2, time: makeDate("10:43:01"), _conversation: conv})

'This' should be replaced by:

new Message(msg.buddy1, msg.message1, {outgoing: true, who: msg.nick1, time: makeDate("10:42:22"), _conversation: conv}),
new Message(msg.buddy1, msg.message2, {outgoing: true, who: msg.nick1, time: makeDate("10:42:25"), _conversation: conv}),
new Message(msg.buddy2, msg.message3, {incoming: true, who: msg.nick2, time: makeDate("10:43:01"), _conversation: conv})

But I am not sure about the function of "msg.buddy1" and "msg.buddy2".
Attached patch messagestyle.js patch (obsolete) — Splinter Review
*** Original post on bio 725 as attmnt 552 by michal.stanke AT mikk.cz at 2011-03-06 10:49:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attached patch PatchSplinter Review
*** Original post on bio 725 as attmnt 553 at 2011-03-06 12:26:00 UTC ***

(In reply to comment #3)

> But I am not sure about the function of "msg.buddy1" and "msg.buddy2".

This is all confusing because the old code was broken too.
msg.buddy<number> here is the screenname of the person talking, msg.nick<number> is the display name (alias) of the person.

Most themes display only the display name, but some themes may also use the screenname (for a tooltip for example).

The old code used to set "who" (which is supposed to be the screenname) correctly, and then override it with the display name (which should be in the alias field). If the alias is not set, using it will fallback automatically in the theme system to the screenname (who).

Uh... I'm not sure if I'm actually clarifying things or making them even more confusing. Sorry... :-/.

Michal: thanks for reporting the issue and helping figure out what was wrong. Your patch (attachment 8352293 [details] [diff] [review] (bio-attmnt 552)) correctly restores the old behavior, but as I just tried to explain, I believe it was already broken before, the brokenness was just not visible.
Attachment #8352294 - Flags: review?(clokep)
Comment on attachment 8352293 [details] [diff] [review]
messagestyle.js patch

*** Original change on bio 725 attmnt 552 at 2011-03-06 12:26:18 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352293 - Attachment is obsolete: true
Comment on attachment 8352294 [details] [diff] [review]
Patch

*** Original change on bio 725 attmnt 553 at 2011-03-06 16:57:44 UTC ***

I don't have too much comment about this besides that you're setting _alias manually...while the _ usually implies a private variable? Not really worth making a setter for it though.
Attachment #8352294 - Flags: review?(clokep) → review+
*** Original post on bio 725 at 2011-03-06 19:14:56 UTC ***

(In reply to comment #6)
> (From update of attachment 8352294 [details] [diff] [review] (bio-attmnt 553) [details])
> I don't have too much comment about this besides that you're setting _alias
> manually...while the _ usually implies a private variable? Not really worth
> making a setter for it though.

Yes, that's ugly (by the way, I was already doing it for _conversation). I don't think it would be worth creating a setter either.
*** Original post on bio 725 at 2011-03-07 17:41:23 UTC ***

Fixed, https://hg.instantbird.org/instantbird/rev/2bb56b783838
Assignee: nobody → florian
Status: NEW → RESOLVED
Closed: 8 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → 0.3a2
You need to log in before you can comment on or make changes to this bug.