1646049 - Private IRC chats not working when server supports echo-message - own posts are not echoed back (and hence not shown) (TB 78b2)

Status: RESOLVED FIXED

(Chat Core :: IRC, defect, P1)

Description

[Jorg K (CEST = GMT+2)]

Chat is broken in TB 78. Private chats only show what your chat partner sends, not what you type yourself.

Comment 1

[Patrick Cloke [:clokep]]

This seems to be working fine for me in 78b1, I wasn't aware a beta 2 was even built yet though.

I tested both IRC and XMPP in both group and individual chats and I saw both sets of messages.

What network are you testing on?

Comment 2

[Jorg K (CEST = GMT+2)]

TB 78b2 isn't out, but I grabbed a copy from the tree. Looks like the issue only happens in my particular setup at pEp which is a bit complicated, involving an ssh tunnel and a ZNC. I set up an account at chat.freenode.net and there a private chat works. Strange since TB 68 works on the pEp setup, but not TB 78. A "public" room works, too, but not private. I'll have to check it out further.

Comment 3

[Patrick Cloke [:clokep]]

It could be a bug in the implementation of bug 1601102 (echo-message support). The first few messages after connecting should show whether this is supported or not in the protocol log.

Comment 4

[Jorg K (CEST = GMT+2)]

It might be related to
NotSupportedError: CustomElementRegistry.define: 'conversation-browser' has already been defined as a custom element conversation-browser.js:853
<anonymous> chrome://chat/content/conversation-browser.js:853
<anonymous> chrome://messenger/content/customElements.js:34
<anonymous> chrome://messenger/content/customElements.js:37
observe resource://gre/modules/MailGlue.jsm:198
initHTMLDocument resource:///modules/imThemes.jsm:741
onStateChange chrome://chat/content/conversation-browser.js:62

The setup I have opens various private chats automatically. It also opens a private chat to myself, which is undesired, and there I get to see what I typed. Strange. I'll take out that bug and see whether it fixes it for me.

Comment 5

[Jorg K (CEST = GMT+2)]

I reverted
https://hg.mozilla.org/comm-central/rev/792c378e19bc#l1.15 which has changed since and
https://hg.mozilla.org/comm-central/rev/792c378e19bc#l2.17
and now it works again.

Would be good to fix this in real life soon to make TB 78 usable for my purposes. If you need help with testing, let me know. For now, I've just unpacked omni.ja and edited the JS files manually.

Comment 6

[Patrick Cloke [:clokep]]

It would probably be useful to have the protocol log of:

1. Logging in.
2. Sending a message.
3. Receiving a message.

Along with a description of what happens in #2 and #3.

The log shouldn't have anything sensitive, though it will have server names and usernames. You can send it directly to me (or it is fairly easy to anonymize) if you prefer!

Comment 7

[Jorg K (CEST = GMT+2)]

OK. Meanwhile I continued hacking. I reverted ircBase.jsm, function privmsg() to it's original state. The change in irc.js is necessary to make a private room work, but in a public room I see the message twice now, but other participants only see it once.

Comment 8

[Jorg K (CEST = GMT+2)]

The issue appears to be that the server has the "echo-message" capability, but only echoes back in public rooms not in private ones. So changing sendMsg() (in irc.js) to always call this.writeMessage() gives two own messages in public rooms. I can't see any parameter to check in sendMsg() to distinguish public from private, so I can't hack it there. Since I'm connecting using ZNC, the server is the ZNC proxy, right?

How do you switch on chat logging? I don't see anything in options, preferences or account settings. And Google doesn't seen to know either.

Comment 9

[Jorg K (CEST = GMT+2)]

Any news here? Can you give me some instructions for the logging you require? Maybe it's related to the use of ZNC.

Comment 10

[Patrick Cloke [:clokep]]

1. Click "Show Accounts" on the chat tab.
2. Right click on the account in question
3. Choose "Copy Debug Log"

The debug logging will be in your clipboard. It resets at each connection I believe and has a limited buffer, so if you have a lot of messages go by the initial connection negotiation will be lost.

Comment 11

[Jorg K (CEST = GMT+2)]

I can see there is discussion about ZNC in bug 1601102. My ZNC version is 1.7.2+deb3 and I did set this option as AutoClearQueryBuffer=false suggested in https://wiki.znc.in/Query_buffers#Self_messages. It was true before but the change didn't help.

Comment 12

[Jorg K (CEST = GMT+2)]

Any update on this? I've sent a log a while ago. I can't use TB 78 beta due to this bug since my company requires "always available" chat using ZNC. Sadly there doesn't appear an option to switch off echo-message in ZNC.

Comment 13

[Magnus Melin [:mkmelin]]

Would this be about support for private mode "p" of the channel? https://tools.ietf.org/html/rfc2811#section-4 listed as TODO at https://searchfox.org/comm-central/rev/93db6b3c1403b18025e1ad81466979c5b4cd2772/chat/protocols/irc/irc.jsm#610

Comment 14

[Patrick Cloke [:clokep]]

(In reply to Jorg K (CEST = GMT+2) from comment #12)

Any update on this? I've sent a log a while ago. I can't use TB 78 beta due to this bug since my company requires "always available" chat using ZNC. Sadly there doesn't appear an option to switch off echo-message in ZNC.

I haven't had a chance to look at it yet. As a volunteer, it is on my to do list, but not sure when I'll get to it. I'm sorry it is making it difficult for you to use Thunderbird at the moment.

(In reply to Magnus Melin [:mkmelin] from comment #13)

Would this be about support for private mode "p" of the channel? https://tools.ietf.org/html/rfc2811#section-4 listed as TODO at https://searchfox.org/comm-central/rev/93db6b3c1403b18025e1ad81466979c5b4cd2772/chat/protocols/irc/irc.jsm#610

It might be, but I'd be surprised by this! From section 4.2.6:

The channel flag 'p' is used to mark a channel "private" and the channel flag 's' to mark a channel "secret". Both properties are similar and conceal the existence of the channel from other users.

This means that there is no way of getting this channel's name from the server without being a member. In other words, these channels MUST be omitted from replies to queries like the WHOIS command.

So sounds like it probably isn't related.

Comment 15

[Patrick Cloke [:clokep]]

I had a brief chat with the IRCv3 folks and they confirmed that echo-message should work for all PRIVMSGs and NOTICEs sent. (There isn't a limitation on sending it to a channel vs. an individual user or even talking to yourself.) It is possible there's something wrong with the ZNC plugin here though.

Comment 16

[Patrick Cloke [:clokep]]

I wonder if adding support here for znc.in/self-message would help too? It is possible that something odd is happening where the server itself is saying it supports echo-message, but ZNC doesn't know we support it for some reason?

I'm unsure if we would want to enable both at the same time or just znc.in/self-message.

Comment 17

[Jorg K (CEST = GMT+2)]

Well, it half works, I get my posts echoed in a public chat. (Have you read comment #11?)

Comment 18

[Patrick Cloke [:clokep]]

(In reply to Jorg K (CEST = GMT+2) from comment #11)

I can see there is discussion about ZNC in bug 1601102. My ZNC version is 1.7.2+deb3 and I did set this option as AutoClearQueryBuffer=false suggested in https://wiki.znc.in/Query_buffers#Self_messages. It was true before but the change didn't help.

Yes, that's what made me wonder if we do need to support znc.in/self-message separately.

It could be interesting to find replace echo-message with znc.in/self-message in all of the IRC files. Everything SHOULD work fine with that change. :)

Comment 19

[Jorg K (CEST = GMT+2)]

You mean to replace all these:
https://searchfox.org/comm-central/search?q=echo-message&path=&case=false&regexp=false
That didn't work.

Comment 20

[Jorg K (CEST = GMT+2)]

Also see: https://github.com/znc/znc/issues/1732

Comment 21

[Alexey]

ZNC dev here.

Please show raw log of what CAPs are requested and how private messages look now, e.g. using znc -D

Comment 22

[Jorg K (CEST = GMT+2)]

I've send the full log to Alexey in a PM. Here an excerpt:

Public channel:
[2020-06-25 01:19:30.746934] (joerg/local) CLI -> ZNC [PRIVMSG #thunderbird :test, please ignore]
[2020-06-25 01:19:30.747033] (joerg/local) ZNC -> CLI [@time=2020-06-24T23:19:30.746Z :joerg!~joerg@127.0.0.1 PRIVMSG #thunderbird :test, please ignore]
[2020-06-25 01:19:30.747135] (joerg/local) ZNC -> IRC [PRIVMSG #thunderbird :test, please ignore] (queued)

Private channel:
[2020-06-25 01:11:19.877087] (joerg/local) CLI -> ZNC [PRIVMSG huss :still testing chat in TB 78, please ignore \x09 \x09\x09\x09\x09 \x09 \x09 \x09 \x09\x09 \x09 ]
[2020-06-25 01:11:19.877199] (joerg/local) ZNC -> CLI [@time=2020-06-24T23:11:19.877Z :joerg!~joerg@127.0.0.1 PRIVMSG huss :still testing chat in TB 78, please ignore \x09 \x09\x09\x09\x09 \x09 \x09 \x09 \x09\x09 \x09 ]
[2020-06-25 01:11:19.877311] (joerg/local) ZNC -> IRC [PRIVMSG huss :still testing chat in TB 78, please ignore \x09 \x09\x09\x09\x09 \x09 \x09 \x09 \x09\x09 \x09 ]

So as far as I can see, ZNC sends the received message back to the client and also to the IRC server. Or am I misinterpreting that?

Comment 23

[Alexey]

So as far as I can see, ZNC sends the received message back to the client and also to the IRC server.

That's correct. That's what echo-message means - client receives its messages back. Both private and in channels.

znc.in/self-message is not needed, it is subset of echo-message, and won't change anything in this case. znc.in/self-message only tells server that client won't explode when receiving messages which look like sent as that same client. It's mostly useful if you have several clients connected, so that all clients can get the whole conversation, and not only one side of it. Looks like this exact feature is not implemented in TB.

If echo-message is requested, support for znc.in/self-message is implied.

Comment 24

[Alexey]

Actually, there may be 1 reason to request znc.in/self-message:

ZNC 1.6.x didn't support echo-message yet, but already knew about znc.in/self-message. So if you want to receive the whole conversation in the multi-client scenario on that old ZNC version (which shouldn't be used anymore), znc.in/self-message will achieve that.

Comment 25

[Jorg K (CEST = GMT+2)]

I think the problem is bigger than using ZNC. ZNC is just one "server" that supports echo-message and plays the message back to the client. I wasn't able to reproduce the issue on Freenode, but then I don't know whether it supports echo-message (haven't checked).

Comment 26

[Alexey]

Yep, which is why I advised against special-casing ZNC here, and sticking to echo-message. And freenode doesn't support echo-message yet.

Comment 27

[Jorg K (CEST = GMT+2)]

Alexey, thanks for your support!

Comment 28

[Jorg K (CEST = GMT+2)]

(In reply to Patrick Cloke [:clokep] from comment #14)

As a volunteer, it is on my to do list, but not sure when I'll get to it. I'm sorry it is making it difficult for you to use Thunderbird at the moment.

I suggested a backout of bug 1601102 since this is likely to affect many users.

Comment 29

[Patrick Cloke [:clokep]]

(In reply to Alexey from comment #24)

Actually, there may be 1 reason to request znc.in/self-message:

ZNC 1.6.x didn't support echo-message yet, but already knew about znc.in/self-message. So if you want to receive the whole conversation in the multi-client scenario on that old ZNC version (which shouldn't be used anymore), znc.in/self-message will achieve that.

Good to know the behavior is essentially the same! I think we had decided in bug 1601102 not to implement znc.in/self-message since it is only for older versions!

I do not believe backing this out is the correct approach, it is likely a pretty simple bug to fix on our side.

Comment 30

[Patrick Cloke [:clokep]]

(In reply to Jorg K (CEST = GMT+2) from comment #28)

(In reply to Patrick Cloke [:clokep] from comment #14)

As a volunteer, it is on my to do list, but not sure when I'll get to it. I'm sorry it is making it difficult for you to use Thunderbird at the moment.

I suggested a backout of bug 1601102 since this is likely to affect many users.

It likely affects very few. echo-message is enabled on very few networks.

Comment 31

[Jorg K (CEST = GMT+2)]

(In reply to Patrick Cloke [:clokep] from comment #30)

It likely affects very few. echo-message is enabled on very few networks - https://ircv3.net/support/networks#networks-1.

Thanks for the additional information. So in bug 1601102 you've implemented a new feature for a small group of users. Exactly that group of users, and everyone using ZNC, will now be inconvenienced. Can there be a clearer case for a backout? The feature wasn't correctly/fully implemented and should hence be withdrawn from all releases. That also takes the pressure off developers and those managing the release.

Comment 32

[Magnus Melin [:mkmelin]]

The reason I'd not backout but just disable (maybe behind a pref), is that code landed long ago (=> conflicts) and it would be easier to disable it that way unless we find a real solution in time.

Comment 33

[Jorg K (CEST = GMT+2)]

Good point, there are conflicts :-( - OK, not querying the capability fixes the issue:
https://searchfox.org/comm-central/rev/a8444d358c7abb921d81ee97d73b6f6ba26c7c8a/chat/protocols/irc/irc.jsm#2325
You can wrap a pref around that, but I don't see the point.

EDIT: Actually, there were two small merge conflicts, so not a big deal really.

Comment 34

[Patrick Cloke [:clokep]]

Attachment: Patch v1

I believe that this fixes the issue. To describe further what is happening:

1. When a PRIVMSG is sent to us from the server it contains the (rough) following parts: <origin> PRIVMSG <target> <msg>
2. The code in bug 1601102 correctly modified some code to identify whether message was "from" us to set the outgoing vs. incoming flag (this is done by comparing the origin to our nick that we have saved previously).
3. The code then attempts to fetch the correct conversation, but to do this it needs to check one of two places:
   1. If the target looks like a channel (essentially if it starts with #) then use the target as the name of the conversation. An example message might be :mkmelin PRIVMSG #test foo where Magnus is sending "foo" to the #test channel.
   2. Otherwise, use the origin. An example message might be :mkmelin PRIVMSG clokep foo where Magnus is sending "foo" to me.

The logic in step 3 was not updated to take into account that when a self-echoed message is received the target is always the conversation name (since the origin is always us).

This modifies the logic in step 3 to:

1. If this is a self-echo, use the target as the name of the conversation.
2. Otherwise, If the target looks like a channel (essentially if it starts with #) then use the target as the name of the conversation.
3. Otherwise, use the origin.

Note that echo-message essentially makes it so both participants receive the same message (in the same order), we need to deal with us being the sender vs. receiver properly.

As part of this patch I also dropped the check to see if echo-message is enabled since it shouldn't really matter. We should always be treating a message from our own nick as "outgoing".

Comment 35

[Jorg K (CEST = GMT+2)]

Comment on attachment 9159462 [details] [diff] [review]
Patch v1

Thanks for the fix. Tested by patching TB 78 beta and running it on the company setup with ZNC.

Comment 36

[Patrick Cloke [:clokep]]

Comment on attachment 9159462 [details] [diff] [review]
Patch v1

[Approval Request Comment]
Regression caused by (bug #): bug 1601102
User impact if declined: Users of IRC servers that have the "echo-message" capability enabled will not see their own messages in private chats.
Testing completed (on c-c, etc.): None.
Risk to taking this patch (and alternatives if risky): I think the risk is fairly low, I tested a private chat, a multi-user channel, and talking to myself with this patch and everything seemed to work OK. The code-path for non-echo-message enabled servers is essentially unmodified, so the worst case is that this doesn't fix the issue for people who are already affected.

Note that I expected the number of affected users to be low since bug 1601102 landed in TB 73 and was only now discovered.

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/44e6b9b6759a
Fix display of own messages in private IRC chats when echo-message is enabled. r=mkmelin

Comment 38

[Wayne Mery (:wsmwk)]

Comment on attachment 9159462 [details] [diff] [review]
Patch v1

Approved for beta - risk assessment in request comments sounds reasonable

Comment 39

[Rob Lemley [:rjl]]

Thunderbird 78.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/9de7a3e59b41