954869 - Adding two buddies with the same nick, but on different IRC accounts leads to strange behaviour

Status: NEW

(Chat Core :: IRC, defect)

Description

[aleth [:aleth]]

*** Original post on bio 1434 at 2012-05-16 00:25:00 UTC ***

STR:
Add an IRC buddy on irc.mozilla org
Add an IRC buddy with the same nick on freenode.org. (Pick a different tag if necessary)

The second IRC buddy now behaves strangely: 
It's a freenode account buddy, but hovering over it shows mozilla account info.
Deleting it seems to delete both buddies.
But errors like the following still occur:
Timestamp: 05/16/2012 02:16:47 AM
Error: this._preferredAccount is null
Source File: components/imContacts.js
Line: 994

It should be possible to have two IRC contacts, on different accounts, with the same nick. This is not that unusual. (And then to merge these contacts.)

(If not possible for some reason, it should not be possible to add such a second buddy ;) )

Comment 1

[aleth [:aleth]]

*** Original post on bio 1434 at 2012-05-16 00:28:26 UTC ***

Should have added, to reproduce it is helpful if both these buddies are online.

Comment 2

[Patrick Cloke [:clokep]]

*** Original post on bio 1434 at 2012-05-16 00:50:32 UTC ***

Might be related to bug 953652 (bio 206).

Comment 3

[aleth [:aleth]]

*** Original post on bio 1434 at 2012-05-16 08:44:07 UTC ***

I suspect what may happen here is that when the second (freenode in this example) buddy is added (to tag X), no corresponding buddy is added, but instead, the existing buddy gets an additional tag X.

Comment 4

[Florian Quèze [:florian]]

*** Original post on bio 1434 at 2012-05-16 10:02:01 UTC ***

(In reply to comment #3)
> I suspect what may happen here is that when the second (freenode in this
> example) buddy is added (to tag X), no corresponding buddy is added, but
> instead, the existing buddy gets an additional tag X.

I believe what happens is: the add buddy correctly adds a new account buddy for the twitter buddy, but not a new buddy because there's a check for buddies to ensure that we don't create duplicated buddies with the same protocol and username.

See http://lxr.instantbird.org/instantbird/source/chat/components/src/imContacts.js#1302

We may need to make the (normalized?)username for IRC buddies [e-mail] instead of just "flo", and use "flo" as the display name. Handling that change in a backward compatible way sounds like it's going to be painful thought :(.

Comment 5

[Patrick Cloke [:clokep]]

*** Original post on bio 1434 at 2012-05-16 12:11:44 UTC ***

(In reply to comment #4)
> See
> http://lxr.instantbird.org/instantbird/source/chat/components/src/imContacts.js#1302

This reasons for this was discussed at [1]:
11:01:44 <aleth> I don't think I ever quite realized chat buddies were only identified by name and protocol, not by name and account. What was the reason for that design decision?
11:33:58 <flo> aleth: the reason is that if you have the same buddy in the server side buddy if of several different AIM accounts, they get merged automatically
11:37:36 <aleth> Hmm. So a special case became the default behaviour?
11:37:40 <aleth> clokep: This bug 954869 (bio 1434) IRC behaviour, is it a regression?
11:42:34 <clokep_work> aleth: I have no idea if that's a regression
11:44:36 <flo> aleth: not a regression
11:45:01 <flo> aleth: IRC is the special case, isn't it?
11:45:16 <flo> the normalized name is supposed to be unique across all accounts of the protocol

It seems to me that there's something more wrong than just with the IRC code currently though, as the buddies are auto-merged for AIM, but you still can't select which account to talk to them with. Is the best approach changing the normalized username or is there a deeper issue we should be fixing?

[1] http://log.bezut.info/instantbird/120516/#m183

Comment 6

[Florian Quèze [:florian]]

*** Original post on bio 1434 at 2012-05-16 12:16:09 UTC ***

(In reply to comment #5)

> the buddies are auto-merged for AIM, but you still can't
> select which account to talk to them with.

That's bug 953652 (bio 206), and it's a UI issue.

This bug (here), as I understand it, is that we are auto-merging account buddies that don't necessarily represent the same person.

Comment 7

[aleth [:aleth]]

*** Original post on bio 1434 at 2012-05-18 20:17:00 UTC ***

(In reply to comment #4)
> (In reply to comment #3)
> I believe what happens is: the add buddy correctly adds a new account buddy for
> the twitter buddy, but not a new buddy because there's a check for buddies to
> ensure that we don't create duplicated buddies with the same protocol and
> username.

I think this is the right explanation, with the following twist. The code

http://lxr.instantbird.org/instantbird/source/chat/components/src/imContacts.js#1323

shows the duplicate check will only cause an error if the same tag is being used. If the tag happens to be different to that of the existing buddy of the same name, a second buddy is created with the different tag. But because it does not store that this one is meant for a different account, weirdness ensues, explaining the behaviour in the bug report.

Question: Isn't this a bug? Shouldn't the duplicate check fire even if the tags differ? Since this is not the correct route to add a second tag to the original buddy, what we are left with generically is a duplicate in a different tag (not good).

> We may need to make the (normalized?)username for IRC buddies
> [e-mail] instead of just "flo", and use "flo" as the display name.
> Handling that change in a backward compatible way sounds like it's going to be
> painful thought :(.

So I think this is the answer. Backwards compatibility will require falling back to the first matching account if the username does not contain a "@".

Comment 8

[Florian Quèze [:florian]]

*** Original post on bio 1434 at 2012-05-18 21:52:23 UTC ***

The first part of comment 7 doesn't make any sense to me. Either I'm tired, or you didn't understand the difference between a buddy (imIBuddy) and an accountBuddy (prplIAccountBuddy). If you think it's the former, feel free to ping me about this so that I look again later :).

Things that make me believe you could be confused:
- the code you linked checks for duplicate account buddies, but you say it's checking for duplicate buddies
- you say that buddies have tags. They don't. Each account buddy has one (and only one) tag, representing the group stored on the server side list of the account. Contacts (way higher level) can have multiple tags, in addition to the tags they inherit from their account buddies.