MSN group name display support

[manphiz]

MSN group support[1] is recently added to MSN, and further expanded by other usage, such as xiaoi and bbqun, which is mainly used in China. It's becoming more and more prevalent. The current mechanism is performed as a bot who forwards messages to all registered members. Since it has it's own passport name, the messages forwarded display this name instead of the original sender's name.

The original patch comes from Gene[2], who finds the message is carrying an additional property "P4-Context" in message header, which in turn contains the original sender's name. The patch is attached below. The current approach is provided by inlining the sender's name in the message, which is quick and dirty, but works. Hope someone's willing to provide a refinement with full support of MSN group.

[1] http://groups.msn.com/

[2] http://www.iuiuiu.com/plog/post/1/337

[manphiz]

New patch refined from Chaos Eternal's original one, which eliminates a memory leakage

[manphiz]

New patch for 2.3.0 from Gene, apply to msnp9 branch as well.

[river]

msn group chat patched libmsn.dll for pidgin 2.3.1 in http://www.iuiuiu.com/plog/post/1/337 I hope add is code in pidgin soon.

[manphiz]

Since Official xiaoi bot now supports command "/showname" to embed original speaker's name in chat message, I guess pidgin won't need to do anything anymore. Xiaoi users, please use "/showname" to activate this function.

[datallah]

I guess I'm going to close this as it isn't really acceptable to append it to the message.

[rsvargas]

There is this site who is offering a Group Chat Service ( http://www.messengergroupchat.com/ ) and the mechanism seems to be equal to the MSN group, a bot who forwards messages to members and using the P4-Context header with the senders nickname.

Maybe there is a need to change the nickname of the sender when this header is sent...

[khc]

Is this something official'ish?

[rsvargas]

I don't think so...

But is probably a good idea, since the official client does change the sender's nickname to whatever comes into that header.

[khc]

that's officialish enough for me

[SuperMMX]

Change Alias

[SuperMMX]

I added a new patch, which adds a special alias in the buddy structure, and set the special alias when "P4-Context" exists.

[SuperMMX]

second try that don't change the conv tab title

[SuperMMX]

My first patch will change the conversation tab title, and the second patch fixes that problem.

[khc]

isn't the special alias going to change on every message? I am not sure that I like this hack

[SuperMMX]

Yes, it will change on every message. But, there should be somewhere to store such information. I don't like the idea of passing it around as a parameter.

Any other better ideas ?

[khc]

is the P4-Context the display name of the buddy or the passport?

[SuperMMX]

It is the display name (nick name), not the passport. The nick name changing function is provided by the group robot, and group users can change it at their own.

[khc]

I don't have good ideas, since no other protocol seem to have similar things. I can only suggest minimizing hackery. Looking at the code, it looks like just passing the display name instead of passport to serv_got_chat_in may sort of work?

[khc]

actually no, that won't work because it's really an IM, not a Chat, so serv_got_im is used and passing the displayname to that won't work

[VuDu]

Any news on this problem? The patches seem to be to older versions. Pidgin 2.5.2 still shows wrong display names. :(

[rekkanoryo]

Ticket #5980 has been marked as a duplicate of this ticket.

[SuperMMX]

Another idea is to encapsulate the parameters into a structure, like PurpleMessage?, which contains the gc, who, msg, and other information, and also the possible special alias. So we can set the special alias in protocol plugin and display it in UI.

but this will involve *A LOT* of interface changes that are probably not acceptable at all.

For now, we don't have any better ideas and the patch works well, why not apply it at first and fix it later?

[VuDu]

Which patch? The .diff you submitted? If it works, what's making you search for alternatives?

[manphiz]

If the current solution is incomplete or doesn't work cleanly, it'll be better to leave it as-is. The "/showname" switch is now provided by xiaoi to support unofficial MSN client such as WebMSN, mobile MSN, which works on pidgin as well. So there is one solution(though not elegant), why will we need another(unelegant one as well)?

To enable this feature, just type "/showname". A second issue of the command turns this feature off.

[VuDu]

But xiaoi isn't the only offering this service, MessengerGroupChat? is another one and as far as I know, there's no /showname command on MessengerGroupChat?.

[datallah]

Ticket #7344 has been marked as a duplicate of this ticket.

[manphiz]

Replying to VuDu:

But xiaoi isn't the only offering this service, MessengerGroupChat? is another one and as far as I know, there's no /showname command on MessengerGroupChat?.

Then wishlist them to add one :) After all such group chat services are unofficial. Though it'll be good to support them in pidgin, there are lots more other clients that don't, and it'll be unfortunate to do the same trick for all of them.

[VuDu]

I'm not sure about other clients behavior, because at least aMSN supports it. That and this discussion probably points to pidgin having made a wrong approach from the beginning to the way it handles the nicknames on MSN. But that's ok, requesting that for the MessengerGroupChat? is as valid as requesting this to be implemented on Pidgin.

Lastly, I'd like to know if any of these patches "works". ;)

[SuperMMX]

@VuDu? and manphiz

What khc and I were talking about is not the functionality of the patch, but the way to implement it. I didn't try the http://www.messengergroupchat.com/, but according to rsvargas' comment, the result should be the same.

Now because of the limitation of current code, there is no better/clean way to make it not hackery. So I suggested another way, which will modify current infrastructure, and probably not acceptable in the near future.

Please spend a little bit time trying the patch, and give some feedback if it doesn't work.

[VuDu]

Hi SuperMMX, patching latest stable sources with msn_group_member_nick2.diff fails. What do we need to patch anything else before that?

[VuDu]

(There should be someway to edit comments, at least before anyone replying.)

What I wanted to ask was what's the "procedure" to follow to test this. There are several patches submitted here... I assumed the last one was the "definitive".

[SuperMMX]

I am sorry that the patch doesn't get applied cleanly, because the code is always evolving. You can change the code manually in the appropriate positions which are indicated in the patch with function names.

You must use the original format when downloading the patch. Hope this helps.

[SuperMMX]

@manphiz

I tried "showname" just now, yes, it can show the nick, but it only put the nick before the actual message. So in Pidgin conversation, you will see your local alias for the group before the actual nick for every message, like this: "MyGroup?: NICK: MESSAGE", it looks uncomfortable to me, at least.

[VuDu]

It's working =) Thanks, SuperMMX! Maybe it'd be a good idea to provide a patch that worked with the stable sources ;)

[VuDu]

About your patch, I think its encoding problems because I created one .diff just like yours and it worked. ;) Here's my version: http://vudu.pastebin.com/m2879950b

[felipec]

The 'special alias' in the patch is wrong, the alias is not for per 'buddy' it's per passport and per conversation.

Imagine someone who joins the conversation but is not on your buddy list. You want the alias to be seen on the conversation, but nowhere else. That's what the P4-Context field is for.

Stretching this concept the same passport can be in two separate conversations with a different 'special alias', so again, setting one 'special alias' for a buddy is wrong.

Can somebody write a patch that allows an on-the-fly alias update of a certain passport in a certain conversation?

[SuperMMX]

Actually it is per IM message in the conversation with the bot passport. And the real passport of the message sender is unknown. It is very possible that some user in the "group" is not in your buddy list.

It seems that you get the whole picture wrong.

[felipec]

Replying to SuperMMX:

Actually it is per IM message in the conversation with the bot passport. And the real passport of the message sender is unknown. It is very possible that some user in the "group" is not in your buddy list. It seems that you get the whole picture wrong.

I'm talking about the patch. The patch is not per conversation.

msn_group_member_nick2.diff is adding a 'special_alias' field for a buddy, so a buddy can't have a different one in another conversation.

And sure, in the official client you don't see who sent the message, but that's a disadvantage of the UI, the protocol is using the passport all the time.

[SuperMMX]

Replying to felipec:

I'm talking about the patch. The patch is not per conversation. msn_group_member_nick2.diff is adding a 'special_alias' field for a buddy, so a buddy can't have a different one in another conversation.

I know it is some kind of hack, but currently there is no other better ways to implement it, so I store such information in the buddy structure. See my comment for a better solution (encapsulating all the necessary parameters into a structure, like PurpleMessage?).

And I don't understand the meaning of another conversation here. Why a user has more than one conversation with the same buddy ?

And sure, in the official client you don't see who sent the message, but that's a disadvantage of the UI, the protocol is using the passport all the time.

The debug message looks like this:

(19:05:38) msn: Message SB RECV:
{MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

P4-Context: TestName

X-MMS-IM-Format: FN=SimSun; EF=; CO=0; CS=86; PF=0

ok
}

Where is the passport of the actual message sender ? I didn't see it.

[rekkanoryo]

I think that an important piece being missed here is that these "chats" are actually just one-on-one IM's with a bot. Everyone in the same "chat" is really only talking with the same single bot. You won't have multiple conversations at the same time with the same bot, so it doesn't really matter if this is screwing with the alias. However, if it's true that the official client doesn't support this, I'm inclined to reject it for that reason.

[SuperMMX]

I just installed Windows Live Messenger Version 2008 Build 8.5.1302.1018, it supports the group member names without more further actions.

[rekkanoryo]

Replying to SuperMMX:

I just installed Windows Live Messenger Version 2008 Build 8.5.1302.1018, it supports the group member names without more further actions.

I realized a few hours after I posted my last comment (by which time I was at work and not able to comment here) that supporting this doesn't change our over-the-wire behavior at all, which means the official client supporting it isn't quite as important as it would be if this modified the over-the-wire behavior. Still, it's good to know what the official client does support.

[felipec]

Replying to SuperMMX:

Replying to felipec:

I'm talking about the patch. The patch is not per conversation. msn_group_member_nick2.diff is adding a 'special_alias' field for a buddy, so a buddy can't have a different one in another conversation.

I know it is some kind of hack, but currently there is no other better ways to implement it, so I store such information in the buddy structure. See my comment for a better solution (encapsulating all the necessary parameters into a structure, like PurpleMessage?).

Temporary hacks that mess with libpurple core are bad. If I was a libpurple dev I wouldn't merge your patch until it's done properly.

And I don't understand the meaning of another conversation here. Why a user has more than one conversation with the same buddy ?

Two group chats, with different people, the same buddy is in both. Or you have a group chat with your buddy there, and also IM.

And sure, in the official client you don't see who sent the message, but that's a disadvantage of the UI, the protocol is using the passport all the time.

The debug message looks like this: {{{ (19:05:38) msn: Message SB RECV: {MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 P4-Context: TestName? X-MMS-IM-Format: FN=SimSun?; EF=; CO=0; CS=86; PF=0 ok } }}} Where is the passport of the actual message sender ? I didn't see it.

I wrote the code to display that debugging message, I didn't think adding the passport was necessary, so what?

Look at what's sent over the wire: http://www.hypothetic.org/docs/msn/switchboard/messages.php

[SuperMMX]

Replying to felipec:

Temporary hacks that mess with libpurple core are bad. If I was a libpurple dev I wouldn't merge your patch until it's done properly.

Does my patch mess libpurple ? I don't think so. It can be removed if there is a better solution available. (There is, but it will change a lot of interfaces, so it does not apply here).

Two group chats, with different people, the same buddy is in both. Or you have a group chat with your buddy there, and also IM.

This doesn't matter at all. The group member can set different nick in different group with some command (/nn in xiaoi).

In all my comments, the *buddy* means the bot that forwards messages between group members. While the group member or message sender stands for the real person that actually sends messages, who may not in the buddy list.

I wrote the code to display that debugging message, I didn't think adding the passport was necessary, so what? Look at what's sent over the wire: http://www.hypothetic.org/docs/msn/switchboard/messages.php

I still didn't see the passport of the *original message sender* in the group.

[felipec]

Replying to SuperMMX:

Replying to felipec:

Temporary hacks that mess with libpurple core are bad. If I was a libpurple dev I wouldn't merge your patch until it's done properly.

Does my patch mess libpurple ? I don't think so. It can be removed if there is a better solution available. (There is, but it will change a lot of interfaces, so it does not apply here).

Sigh. Do you think that adding fields in the middle of structures doesn't break binary compatibility?

Two group chats, with different people, the same buddy is in both. Or you have a group chat with your buddy there, and also IM.

This doesn't matter at all. The group member can set different nick in different group with some command (/nn in xiaoi).

I don't know what's xiaoi, and I don't care, setting a nick would update the 'special alias' in all the conversations.

In all my comments, the *buddy* means the bot that forwards messages between group members. While the group member or message sender stands for the real person that actually sends messages, who may not in the buddy list.

I didn't read all the messages, I just read the patch, which is supposed to implement the 'P4-Context' but it's doing it the wrong way.

I wrote the code to display that debugging message, I didn't think adding the passport was necessary, so what? Look at what's sent over the wire: http://www.hypothetic.org/docs/msn/switchboard/messages.php

I still didn't see the passport of the *original message sender* in the group.

Didn't see where?

Can you focus on the 'P4-Context' field in general usage, instead of whatever strange thing you are trying to do with it?

Or you can ignore my comments and hope the devs will accept your patch. My bet is that it will be ignored for a few more years in the current state.

[SuperMMX]

Replying to felipec:

Sigh. Do you think that adding fields in the middle of structures doesn't break binary compatibility?

I do, so I agree that it is some kind hack. But no rush here. We can apply the patch later in next major version which is definitely broken.

Or we can move the field to the end of the structure. I added there because I think it is a logical place to go for this temporary fix.

I don't know what's xiaoi, and I don't care, setting a nick would update the 'special alias' in all the conversations.

I don't think such a nick should apply to all conversations (and there is only one currently). Because it is per message. And it should go with the message, but it is not possible without changing many interfaces and function calls.

Didn't see where?

The message dump in the link you provided. Or I misunderstood you?

I didn't read all the messages, I just read the patch, which is supposed to implement the 'P4-Context' but it's doing it the wrong way

Can you focus on the 'P4-Context' field in general usage, instead of whatever strange thing you are trying to do with it?

I have given my effort on this with a short term solution (the patch) and a long term solution (see My comment 17). Why I am arguing here is because I think we have different understanding of how P4-context should behavior. And we should be on the same page.

Or you can ignore my comments and hope the devs will accept your patch. My bet is that it will be ignored for a few more years in the current state.

I myself use the patch happily without any problems. I care because this can benefit more people and make Pidgin better.

[felipec]

Replying to SuperMMX:

Replying to felipec:

Sigh. Do you think that adding fields in the middle of structures doesn't break binary compatibility?

I do, so I agree that it is some kind hack. But no rush here. We can apply the patch later in next major version which is definitely broken. Or we can move the field to the end of the structure. I added there because I think it is a logical place to go for this temporary fix.

Exactly, temporary things aren't going to be merged.

I don't know what's xiaoi, and I don't care, setting a nick would update the 'special alias' in all the conversations.

I don't think such a nick should apply to all conversations (and there is only one currently). Because it is per message. And it should go with the message, but it is not possible without changing many interfaces and function calls.

Look, in conversation A you receive a message from foo@… with a "P4-Context" of 'Tim', then, in conversation B you receive a message from foo@… with a "P4-Context" of 'John'.

Follow the code:

conversation A: PurpleBuddy? *buddy = purple_find_buddy(cmdproc->session->account, "foo@…"); buddy->special_alias = g_strdup("Tim");

conversation B: PurpleBuddy? *buddy = purple_find_buddy(cmdproc->session->account, "foo@…"); buddy->special_alias = g_strdup("John");

If you go to conversation A, which is the value of the 'special alias'?

Didn't see where?

The message dump in the link you provided. Or I misunderstood you?

MSG example@… Mike 133\r\n

You don't see the passport there?

I didn't read all the messages, I just read the patch, which is supposed to implement the 'P4-Context' but it's doing it the wrong way

Can you focus on the 'P4-Context' field in general usage, instead of whatever strange thing you are trying to do with it?

I have given my effort on this with a short term solution (the patch) and a long term solution (see My comment 17). Why I am arguing here is because I think we have different understanding of how P4-context should behavior. And we should be on the same page.

There's a PurpleConvChatBuddy?, just set the right alias there. That structure is per-conversation.

Or you can ignore my comments and hope the devs will accept your patch. My bet is that it will be ignored for a few more years in the current state.

I myself use the patch happily without any problems. I care because this can benefit more people and make Pidgin better.

The only way this can benefit more people is if it gets applied, and that requires more effort.

[SuperMMX]

Replying to felipec:

MSG example@… Mike 133\r\n You don't see the passport there?

That is not the passport of the *original message sender* in the case of group service, but the passport of the bot (actual buddy in Pidgin). This is what I am emphasizing all the time.

Look, in conversation A you receive a message from foo@… with a "P4-Context" of 'Tim', then, in conversation B you receive a message from foo@… with a "P4-Context" of 'John'. Follow the code:

conversation A:
PurpleBuddy *buddy = purple_find_buddy(cmdproc->session->account, "foo@bar.com");
buddy->special_alias = g_strdup("Tim");
 
conversation B:
PurpleBuddy *buddy = purple_find_buddy(cmdproc->session->account, "foo@bar.com"); 
buddy->special_alias = g_strdup("John");

If you go to conversation A, which is the value of the 'special alias'?

I understand your concern. In this case, for the same account, the special alias is set in the order of the messages being received, because there is only one connection per account, right? And after the message is displayed, the value doesn't matter anymore.

There's a PurpleConvChatBuddy?, just set the right alias there. That structure is per-conversation.

In the group service, the conversation is not a CHAT, but an IM. So the special alias can't save in this structure.

The ultimate goal for the value of P4-Context is to be passed to purple_conversation_write", no matter saved in some structure (PurpleBuddy? in the patch) or future "PurpleMessage?" (long term solution) or as a parameter directly. It is per message, not per buddy or per conversation. I chose PurpleBuddy? because there was no side effect.

BTW, I found that there is a PurpleConvMessage? structure, which is exactly what I am looking for. But it is only used for history saving, not from protocol plugin to core.

[felipec]

Replying to SuperMMX:

Replying to felipec:

MSG example@… Mike 133\r\n You don't see the passport there?

That is not the passport of the *original message sender* in the case of group service, but the passport of the bot (actual buddy in Pidgin). This is what I am emphasizing all the time.

I don't care about that bot, I'm talking about 'P4-Context' support.

Look, in conversation A you receive a message from foo@… with a "P4-Context" of 'Tim', then, in conversation B you receive a message from foo@… with a "P4-Context" of 'John'. Follow the code:

{{{ conversation A: PurpleBuddy? *buddy = purple_find_buddy(cmdproc->session->account, "foo@…"); buddy->special_alias = g_strdup("Tim"); conversation B: PurpleBuddy? *buddy = purple_find_buddy(cmdproc->session->account, "foo@…"); buddy->special_alias = g_strdup("John"); }}}

If you go to conversation A, which is the value of the 'special alias'?

I understand your concern. In this case, for the same account, the special alias is set in the order of the messages being received, because there is only one connection per account, right? And after the message is displayed, the value doesn't matter anymore.

You don't know in how many places that alias is used.

There's a PurpleConvChatBuddy?, just set the right alias there. That structure is per-conversation.

In the group service, the conversation is not a CHAT, but an IM. So the special alias can't save in this structure. The ultimate goal for the value of P4-Context is to be passed to purple_conversation_write", no matter saved in some structure (PurpleBuddy? in the patch) or future "PurpleMessage?" (long term solution) or as a parameter directly. It is per message, not per buddy or per conversation. I chose PurpleBuddy? because there was no side effect. BTW, I found that there is a PurpleConvMessage? structure, which is exactly what I am looking for. But it is only used for history saving, not from protocol plugin to core.

From my point of view 'P4-Context' should be implemented correctly, if that helps "group service", good. But a temporary hack just for "group service" isn't going to help many people.

[SuperMMX]

You are just saying this is wrong, that is wrong, but what's correct? Saving the alias in conversation? I don't think it is *correct*. The correct way is to display the nick per message, not saving it elsewhere.

We are tech guys, please talk with code.

[felipec]

Replying to SuperMMX:

You are just saying this is wrong, that is wrong, but what's correct? Saving the alias in conversation? I don't think it is *correct*. The correct way is to display the nick per message, not saving it elsewhere.

I already told you; an alias per conversation, that is updated when a message with a new alias comes.

Do a thought experiment; which alias should you display when a message without 'P4-Context' comes around?

We are tech guys, please talk with code.

I won't touch libpurple code. If I find a way to implement it just in the protocol plug-in I would do it in msn-pecan.

[SuperMMX]

Replying to felipec:

I already told you; an alias per conversation, that is updated when a message with a new alias comes.

I already said, I don't think it is correct either. I don't want my tab title changes all the time with something not set by me. (I always set a local alias for a buddy). With my understanding, P4-Context will not change the buddy's nick permanently, but only for the message that it is associated with.

Do a thought experiment; which alias should you display when a message without 'P4-Context' comes around?

The original alias of the buddy.

I won't touch libpurple code. If I find a way to implement it just in the protocol plug-in I would do it in msn-pecan.

I don't care, as long as we can make it in a *reasonable* way. Good luck to you anyway.

[felipec]

Replying to SuperMMX:

Replying to felipec:

I already told you; an alias per conversation, that is updated when a message with a new alias comes.

I already said, I don't think it is correct either. I don't want my tab title changes all the time with something not set by me. (I always set a local alias for a buddy). With my understanding, P4-Context will not change the buddy's nick permanently, but only for the message that it is associated with.

The private alias will always override the 'P4-Context'.

Do a thought experiment; which alias should you display when a message without 'P4-Context' comes around?

The original alias of the buddy.

Well, that's a possibility. I don't think that would be easy to implement at all, so, good luck with that.

[deryni]

When, other than in this horrible hack of named chat room 'support', is the P4-Context used? Does it mean the same thing there? How does the official client handle it in those cases?

If you set the P4-Context on a normal message does the official client show your message as coming from a different sender each time? "P4-name1: Testing", "P4-name2: Test 2", etc. as opposed to "FriendlyName?1: Testing", "FriendlyName?1: Test 2"? (I'm assuming the answer to this is yes as this is probably all that is happening for this group chat hack.)

Does the official client ever send a P4-Context header?

Assuming the P4-Context header is really just a per-message display nickname then the "right" way to handle this is likely going to require some core libpurple changes. The only other option is what one of the first patches in this ticket did which is just to prefix each message with the P4-Context header contents, something which I think is probably about as far as I think we really need to go here and at very least would seem to me to make these "chat rooms" at least functional in pidgin as opposed to know where presumably they aren't at all usable.

[rekkanoryo]

We need some Pidgin developers to give a yes or no opinion on this patch.

[QuLogic]

Replying to deryni:

When, other than in this horrible hack of named chat room 'support', is the P4-Context used? Does it mean the same thing there? How does the official client handle it in those cases?

Some people like to use it to make it look like you said something you didn't. That craze seems to have died out though (or maybe it's because Pidgin doesn't show them ;) The official client uses P4-Context in place of the display name, per message.

If you set the P4-Context on a normal message does the official client show your message as coming from a different sender each time? "P4-name1: Testing", "P4-name2: Test 2", etc. as opposed to "FriendlyName?1: Testing", "FriendlyName?1: Test 2"? (I'm assuming the answer to this is yes as this is probably all that is happening for this group chat hack.)

Yes.

Does the official client ever send a P4-Context header?

Only if you install a hack, as far as I know.

Assuming the P4-Context header is really just a per-message display nickname then the "right" way to handle this is likely going to require some core libpurple changes. The only other option is what one of the first patches in this ticket did which is just to prefix each message with the P4-Context header contents, something which I think is probably about as far as I think we really need to go here and at very least would seem to me to make these "chat rooms" at least functional in pidgin as opposed to know where presumably they aren't at all usable.

I tend to agree there. I think the first patches are more suitable than the second. But I'm not too certain about the core changes in the second patch.

[felipec]

Replying to QuLogic:

I tend to agree there. I think the first patches are more suitable than the second. But I'm not too certain about the core changes in the second patch.

msn-pecan has a much better patch: http://github.com/felipec/msn-pecan/commit/1386c1f81fb3214d67f1de9b3349b85bbd79adc9

[manphiz]

Replying to felipec:

Replying to QuLogic:

I tend to agree there. I think the first patches are more suitable than the second. But I'm not too certain about the core changes in the second patch.

msn-pecan has a much better patch: http://github.com/felipec/msn-pecan/commit/1386c1f81fb3214d67f1de9b3349b85bbd79adc9

So this patch restores the buddy after outputing the message so that the title is no longer polluted?

And, as currently xiaoi's switch for displaying buddy name kind of out-of-function, and P4-Context is quite officialish recently, it'll be great to incorporate the fix for this bug.

[QuLogic]

Replying to felipec:

msn-pecan has a much better patch: http://github.com/felipec/msn-pecan/commit/1386c1f81fb3214d67f1de9b3349b85bbd79adc9

There's no such thing as purple_buddy_set_displayname.

[felipec]

Replying to QuLogic:

Replying to felipec:

msn-pecan has a much better patch: http://github.com/felipec/msn-pecan/commit/1386c1f81fb3214d67f1de9b3349b85bbd79adc9

There's no such thing as purple_buddy_set_displayname.

Now it's called purple_buddy_set_private_alias and it's defined in fix_purple.c in msn-pecan. I sent a patch for that a long time ago to pidgin devs, I don't know what happened to it.

[deryni]

The msn-pecan patch is not "good" but it does "work". The choices as I see them currently are:

1. Do nothing, leaves things as broken as they are now rendering these group chats unusable.
2. 'Tag' each message with the P4-Context header as a prefix (so <timestamp> GroupName?: P4Name: message) or some other similar tagging method.
3. Use a patch like the one Felipe has in msn-pecan which sets the user settable alias for the buddy to the value of the P4-Context header for the duration of the message and then sets it back to whatever value it had previously afterwards.
4. Determine what would be involved to allow libpurple to support additional tags of this sort (and potentially other sorts) for incoming (and outgoing?) messages.

These options are listed in roughly the order they are easy to implement (I have placed the tag option about the alias patch option despite a patch for the latter existing because I think the tagging is less of an ugly hack).

I think the first is a non-option at this point, I think the last is potentially useful but requires a longer timespan than is likely helpful, and between the middle two I like the tagging better than aliasing because it seems like less of a hack to me (as it doesn't potentially trigger tab title changes, window title changes, buddy list refreshing, etc.).

A decision needs to be made on this point and some code committed, I have given my preference but am not going to make the decision as I don't use MSN in any real way.

[felipec]

Replying to deryni:

The msn-pecan patch is not "good" but it does "work". The choices as I see them currently are: 1. Do nothing, leaves things as broken as they are now rendering these group chats unusable.

Not good.

1. 'Tag' each message with the P4-Context header as a prefix (so <timestamp> GroupName?: P4Name: message) or some other similar tagging method.

Will look horrible.

1. Use a patch like the one Felipe has in msn-pecan which sets the user settable alias for the buddy to the value of the P4-Context header for the duration of the message and then sets it back to whatever value it had previously afterwards.

Circumvent libpurple limitations but Just Works.

1. Determine what would be involved to allow libpurple to support additional tags of this sort (and potentially other sorts) for incoming (and outgoing?) messages.

It's not a tag.

Libpurple should support public alias per-message. Anything else will not cut it.

These options are listed in roughly the order they are easy to implement (I have placed the tag option about the alias patch option despite a patch for the latter existing because I think the tagging is less of an ugly hack).

IMO it's actually very clean, considering libpurple's limitations.

As I said, your option 2) will look horrible. No user will want that.

Oliver: John Oliver: hi there!

I think the first is a non-option at this point, I think the last is potentially useful but requires a longer timespan than is likely helpful, and between the middle two I like the tagging better than aliasing because it seems like less of a hack to me (as it doesn't potentially trigger tab title changes, window title changes, buddy list refreshing, etc.). A decision needs to be made on this point and some code committed, I have given my preference but am not going to make the decision as I don't use MSN in any real way.

[deryni]

Correct, leaving things as is is not an option, and yes generic tagging is not exactly appropriate (but it isn't inappropriate either, because this information *is* a tag, it is more fundamental of a tag than many other possible tags but it is nonetheless a tag).

Personally, I consider an interface that allows someone to masquerade as another user as fundamentally flawed. Does the official client support any way to determine what user actually sent a message that is displayed as being from a P4-Context header? From what I've heard it would sound like it does not, which means that I can pretend to be anyone I want which is just bad. I should have made it clear that even if pidgin were to support per-message 'aliases' I would not support using those in place of the sender name/alias but only in addition to them (I would be fine with making the per-message alias the primary identifier and only placing the buddy alias as a secondary identifier, but not with removing the buddy alias entirely).

When would "Oliver: John Oliver: hi there!" happen? Why set a P4-Context when a normal friendly name will do? What purpose does this serve in normal one-on-one conversations (other than to obscure who you really are)? Again, I personally, see no reason to support such an identity obscuring "feature".

One of the key things to realize about the arbitrary tagging idea is that with webkit themes the tags would ideally be arbitrarily placeable/displayable by the theme, so a theme could decide whether the P4-Context header should replace the buddy alias or not.

I see no reason to specifically support per-message aliases rather than supporting arbitrary per-message attributes/tags. Your simple assertion that the per-message alias is not a tag is simply not sufficient.

[felipec]

Replying to deryni:

Correct, leaving things as is is not an option, and yes generic tagging is not exactly appropriate (but it isn't inappropriate either, because this information *is* a tag, it is more fundamental of a tag than many other possible tags but it is nonetheless a tag). Personally, I consider an interface that allows someone to masquerade as another user as fundamentally flawed. Does the official client support any way to determine what user actually sent a message that is displayed as being from a P4-Context header? From what I've heard it would sound like it does not, which means that I can pretend to be anyone I want which is just bad. I should have made it clear that even if pidgin were to support per-message 'aliases' I would not support using those in place of the sender name/alias but only in addition to them (I would be fine with making the per-message alias the primary identifier and only placing the buddy alias as a secondary identifier, but not with removing the buddy alias entirely).

You are mixing public alias with private alias. The public alias can be per message, but if you have a private alias for that buddy, then that's what you'll see instead.

When would "Oliver: John Oliver: hi there!" happen? Why set a P4-Context when a normal friendly name will do? What purpose does this serve in normal one-on-one conversations (other than to obscure who you really are)? Again, I personally, see no reason to support such an identity obscuring "feature".

It would happen all the time, if your buddy doesn't have neither a public alias nor a private alias but has P4-Context then you'll see:

john.oliver@…: John Oliver: hi there!

Is that so hard to imagine?

Also, since you are not an MSN user, If I were you I wouldn't trust my own opinion.

One of the key things to realize about the arbitrary tagging idea is that with webkit themes the tags would ideally be arbitrarily placeable/displayable by the theme, so a theme could decide whether the P4-Context header should replace the buddy alias or not.

No, theme authors don't care about such details. They just care about the name that should be displayed, that's it.

I see no reason to specifically support per-message aliases rather than supporting arbitrary per-message attributes/tags. Your simple assertion that the per-message alias is not a tag is simply not sufficient.

It is *not* a tag. If you decide to display it as such you'll be doing something wrong, possibly worst than the current situation.

[rekkanoryo]

Accepting, rejecting, or requesting improvement of this patch is a blocker for the release of version 2.6.0.

[deryni]

Ticket #9396 has been marked as a duplicate of this ticket.

[rekkanoryo]

Ok, the rough consensus among Pidgin developers who have actually spoken with me about this is the following:

• If we want to support this before 3.0.0, it will be implemented as tagging, whereby the display is (timestamp) Group Name: P4-Context Value: message.
• A proper solution for 3.0.0 will be to allow prpls to include "tagging" information when passing messages to the core. This allows us to support the P4-Context header on MSN as well as support showing the thread stuff XMPP has. The UI will be able to decide how to handle this extra information, including ignoring it if the UI so chooses or allowing themes to handle or ignore it.

As such, I am rejecting all the patches on this ticket.

[felipec]

That's absurd, as I explained before; P4-Context is *not* a tag, the official client doesn't display it as a tag, and nobody is using it as a tag.

It's obvious you don't even want to understand what P4-Context is, and what this bug is about... Instead you just want to fulfil your own developer pleasure by implementing something completely unrelated and disregard all the work people have spent in this bug.

Ludicrous.

IOW libpurple will not support P4-Context properly, use msn-pecan where it works as expected.

[VuDu]

Another disappointing move by the Pidgin devs that, once again, show how few they care about MSN users. You hide behind arguments like "we want to do it the right way" or "it's not on the protocol" but the reality you're showing to the users is that you keep avoiding and delaying anything that is MSN-feature related and requires the least amount of effort. Even simple things like this one, that would be easily patched and milestoned to "implement properly" for some later version are postponed.

I'm not sure about the real numbers, but I think MSN is the most used IM protocol, at least in Europe. The Wikipedia article ( http://en.wikipedia.org/wiki/Usage_share_of_instant_messaging_clients ) gets the info from ars technica, that on April 2006 ( http://arstechnica.com/microsoft/news/2006/04/3557.ars ) says that MSN has 61% usage worldwide and is even with AOL in the US.

This was opened almost 2 years ago and was just postponed. The msgplus color tags is on the same track. This is another disappoint move from the pidgin devs, to say the least.

[felipec]

vudu: well said

The msn-pecan patch is simple, and does the right thing. And msgplus tags are supported too (no color, just stripping).

And here are the statics according to the only study I know of market usage: MSN: 48.27% AIM: 4.42% ICQ: 7.85% Yahoo: 20.91% Jabber: 1.07% Gtalk: 13.62% QQ: 3.86%

It's the average percentage by country, so it's not that good, but better than nothing.

From http://billionsconnected.com/blog/2008/08/global-im-market-share-im-usage/

[deryni]

VuDu?: Being willing to accept a patch that implements this the message prefix way now and doing it correctly in the future is *exactly* what was decided. If you want the message prefix system added feel free to supply the patch (especially as you seem to think it requires very little effort). I'm sure it does in fact require relatively little effort but as I don't use MSN I couldn't even test it if I wanted to, though I would gladly commit a patch that appeared to have been written correctly and let other people test it.

A new ticket requesting more useful support for the correct message tagging feature would be acceptable as well, this ticket does not need to stay open as it contains a large amount of discussion and argument unrelated to the implementation of that feature.

[felipec]

Replying to deryni:

VuDu?: Being willing to accept a patch that implements this the message prefix way now and doing it correctly in the future is *exactly* what was decided.

Yeah, decided by Pidgin developers who a) don't use MSN and b) don't even understand what this feature is about.

It was *not* agreed with the people actually interested in the feature, or do you see anyone else outside the dev team agreeing with that decision in this thread?

You obviously don't care about what the people interested in this feature think, otherwise you'll ask for feedback before deciding anything.

IMO, deciding it should be done with a message prefix, or tag is actually worst than saying: wontfix.

[VuDu]

Replying to deryni:

VuDu?: Being willing to accept a patch that implements this the message prefix way now and doing it correctly in the future is *exactly* what was decided.

rekkanoryo used a bold+italic "if" before the "we want to support this before 3.0.0" which doesn't *exactly* support your "*exactly* what was decided" statement, therefore what I said before. Besides that, it seems he didn't quite understood the issue, since it makes little sense to show both buddy/group name and P4-Context... just look at the behavior of the official MSN client.

If you want the message prefix system added feel free to supply the patch (especially as you seem to think it requires very little effort). I'm sure it does in fact require relatively little effort but as I don't use MSN I couldn't even test it if I wanted to, though I would gladly commit a patch that appeared to have been written correctly and let other people test it.

Weren't enough patchs already suggested for this issue? Mass rejecting patchs and then asking for another one shows lack of consideration for the effort of the rejected patchs' users. If there's anything wrong one should start where the others left before. I've used this based on the patchs submitted here and it worked just fine. I understand Gaim started as a AOL client, but that was a decade ago and now MSN has a huge weight on the IM world. Having AOL-only devs reviewing MSN-only patchs might lead to problems like this.

A new ticket requesting more useful support for the correct message tagging feature would be acceptable as well, this ticket does not need to stay open as it contains a large amount of discussion and argument unrelated to the implementation of that feature.

I don't understand why somewhere in the middle of this ticket the word "tag" started being used. We always had (and should always have) "(timestamp) alias: message". The only thing that changed is that now msn_message_get_attr(msg, "P4-Context") might not be NULL, right? Why entangle things? IMO, if it's there use it... if not, move on.

[felipec]

Replying to VuDu:

I don't understand why somewhere in the middle of this ticket the word "tag" started being used. We always had (and should always have) "(timestamp) alias: message". The only thing that changed is that now msn_message_get_attr(msg, "P4-Context") might not be NULL, right? Why entangle things? IMO, if it's there use it... if not, move on.

Correct, and it's a really really minor change: http://github.com/felipec/msn-pecan/commit/1386c1f81fb3214d67f1de9b3349b85bbd79adc9

P4-Context is meant to be a per-message nick; whatever Pidgin devs have decided to do with it is wrong.

[VuDu]

Replying to felipec:

P4-Context is meant to be a per-message nick; whatever Pidgin devs have decided to do with it is wrong.

So it seems...

If the official client receives the following:
 
MIME-Version: 1.0\r\n
Content-Type: text/plain\r\n
P4-Context: My friendlyname\r\n
\r\n
Hello

It would display:

My friendlyname says:

Hello

Documented here for the first time, five years ago.

Replying to deryni:

The msn-pecan patch is not "good" but it does "work". The choices as I see them currently are: 1. Do nothing, leaves things as broken as they are now rendering these group chats unusable. 1. 'Tag' each message with the P4-Context header as a prefix (so <timestamp> GroupName?: P4Name: message) or some other similar tagging method. 1. Use a patch like the one Felipe has in msn-pecan which sets the user settable alias for the buddy to the value of the P4-Context header for the duration of the message and then sets it back to whatever value it had previously afterwards. 1. Determine what would be involved to allow libpurple to support additional tags of this sort (and potentially other sorts) for incoming (and outgoing?) messages. These options are listed in roughly the order they are easy to implement (I have placed the tag option about the alias patch option despite a patch for the latter existing because I think the tagging is less of an ugly hack). I think the first is a non-option at this point, I think the last is potentially useful but requires a longer timespan than is likely helpful, and between the middle two I like the tagging better than aliasing because it seems like less of a hack to me (as it doesn't potentially trigger tab title changes, window title changes, buddy list refreshing, etc.). A decision needs to be made on this point and some code committed, I have given my preference but am not going to make the decision as I don't use MSN in any real way.

Alright, so the "non-option" turned out to be the choice made. IMO, put 3 in action and start planning 4 and that requires picking up the patches suggested before. What do you think?

[felipec]

Replying to VuDu:

Alright, so the "non-option" turned out to be the choice made. IMO, put 3 in action and start planning 4 and that requires picking up the patches suggested before. What do you think?

3 yes, 4 no.

A tag would not represent what really happened. IMO the options are:

1. Temporarily change the alias, just like in msn-pecan
2. Properly implement alias per-conversation
3. Properly implement alias per-message

Note that #1 is perfectly safe because since libpurple is single-threaded we don't leave a chance for the buddy list or anything else to do something with the temporary new alias.

2 would probably be fine but potentially worst than 1, but what would really reflect what is happening is 3.

Anything else would do more damage than good.

[VuDu]

Yes, I agree, my bad. Like you said a weeks ago:

Replying to felipec:

Replying to deryni:

1. Determine what would be involved to allow libpurple to support additional tags of this sort (and potentially other sorts) for incoming (and outgoing?) messages.

It's not a tag. Libpurple should support public alias per-message. Anything else will not cut it.

By deryni's point 4 I meant planning a better future implementation. If the current one, deryni's point 3 (your point 1), turns out to be the best one, great!
I don't know about other protocols, but if they benefit from it, it'd be best to delegate that to libpurple and separate the buddy alias from the message alias (your point 3) which would mean something like adding a new "message alias" attribute and a new function to specifically change that attribute without changing other fields (tab names) related to the buddy alias, right?
I'm not much into lipurple's code, so I might be shooting blindfolded.

[QuLogic]

Replying to felipec:

Correct, and it's a really really minor change: http://github.com/felipec/msn-pecan/commit/1386c1f81fb3214d67f1de9b3349b85bbd79adc9

OK, fine, I tried your patch. It doesn't work.

• Send message with P4-Context. Displayed name in conversation line changes. Good.
• Send message without P4-Context. Display name is still the same as with P4-Context. Wrong.

Additionally, calling purple_blist_alias_buddy emits a blist-node-aliased signal. Given that any plugin can listen to said signal, who knows what the ramifications could be, especially if one of those chatrooms was very busy.

[felipec]

Replying to QuLogic:

Replying to felipec:

Correct, and it's a really really minor change: http://github.com/felipec/msn-pecan/commit/1386c1f81fb3214d67f1de9b3349b85bbd79adc9

OK, fine, I tried your patch.

See the author field; it is not my patch.

It doesn't work.

How so?

* Send message with P4-Context. Displayed name in conversation line changes. Good.

Ah, so it *does* work.

* Send message without P4-Context. Display name is still the same as with P4-Context. Wrong.

If that indeed happens the patch still works, although it has some bugs.

However, I fail to see how it might fail. What is msn_message_get_attr(msg, "P4-Context") returning?

Looking at msn-pecan's purple_buddy_set_public_alias I'm starting to wonder; which buddy is the conversation picking to show the message?

Additionally, calling purple_blist_alias_buddy emits a blist-node-aliased signal. Given that any plugin can listen to said signal, who knows what the ramifications could be, especially if one of those chatrooms was very busy.

Right, I should probably copy purple_blist_server_alias_buddy and remove the signal (and probably other unnecessary stuff too). But seriously, who is listening to such a signal?

I'll better wait until I get a bug report for that, but so far, nobody has complained.

[felipec]

Replying to felipec:

Replying to QuLogic:

* Send message without P4-Context. Display name is still the same as with P4-Context. Wrong.

If that indeed happens the patch still works, although it has some bugs.

I think I found the problem, the fix and the explanation are here: http://github.com/felipec/msn-pecan/commit/3ca18140149aede5ae9b55dc590b4335ff9402b5

Looking at msn-pecan's purple_buddy_set_public_alias I'm starting to wonder; which buddy is the conversation picking to show the message?

I looked at the code and the answer is; unspecified; it could use any buddy with the same passport.

[QuLogic]

Ticket #10944 has been marked as a duplicate of this ticket.

[rekkanoryo]

Ticket #11583 has been marked as a duplicate of this ticket.

[hellmage]

It's been 3 years and several tickets have been marked as duplicate of this one. Isn't it better to fix this issue first and find an elegant resolution later?

[rekkanoryo]

Ticket #12133 has been marked as a duplicate of this ticket.