Closed Bug 1646049 Opened 5 years ago Closed 5 years ago

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

Categories

(Chat Core :: IRC, defect, P1)

Product:
Chat Core
Component:
IRC
Type:
defect

Tracking

(thunderbird77 wontfix, thunderbird78 fixed, thunderbird79 fixed)

Status:
RESOLVED FIXED
Milestone:
Instantbird 79
Tracking Flags:
Tracking Status
thunderbird77 --- wontfix
thunderbird78 --- fixed
thunderbird79 --- fixed

People

(Reporter: jorgk-bmo, Assigned: clokep)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Patch v1
2.41 KB, patch
mkmelin
: review+
jorgk-bmo
: feedback+
wsmwk
: approval-comm-beta+
Details | Diff | Splinter Review

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

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?

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.

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.

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.

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.

Regressed by: 1601102
Summary: Private IRC chats not working - own posts are not shown (TB 78b2) → Private IRC chats not working under certain circumstances - own posts are not echoed back (and hence not shown) (TB 78b2)

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!

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.

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.

Flags: needinfo?(clokep)

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

  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.

Flags: needinfo?(clokep)

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.

Summary: Private IRC chats not working under certain circumstances - own posts are not echoed back (and hence not shown) (TB 78b2) → Private IRC chats not working when ZNC is used - own posts are not echoed back (and hence not shown) (TB 78b2)
Blocks: tb78found

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.

(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.

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.

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.

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

(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. :)

ZNC dev here.

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

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?

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.

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.

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).

Summary: Private IRC chats not working when ZNC is used - own posts are not echoed back (and hence not shown) (TB 78b2) → Private IRC chats not working when server supports echo-message - own posts are not echoed back (and hence not shown) (TB 78b2)

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

Alexey, thanks for your support!

(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.

(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.

(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.

(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.

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.

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.

Attached patch Patch v1 — — Splinter Review

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".

Assignee: nobody → clokep
Status: NEW → ASSIGNED
Attachment #9159462 - Flags: review?(mkmelin+mozilla)
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.
Attachment #9159462 - Flags: feedback+
Attachment #9159462 - Flags: review?(mkmelin+mozilla) → review+
Keywords: checkin-needed-tb
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.
Attachment #9159462 - Flags: approval-comm-beta?

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

Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Keywords: checkin-needed-tb
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 79
Comment on attachment 9159462 [details] [diff] [review] Patch v1 Approved for beta - risk assessment in request comments sounds reasonable
Attachment #9159462 - Flags: approval-comm-beta? → approval-comm-beta+
Regressions: 1649445
No longer regressions: 1649445
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: