indrora / Atomic

Atomic: an IRC client from the ashes of yaaic
https://indrora.github.io/Atomic/
GNU General Public License v3.0
124 stars 29 forks source link

Sent messages echoed back when connected through ZNC #118

Closed dgw closed 8 years ago

dgw commented 8 years ago

Apparently I'm not the first to report this, but @indrora has been unable to reproduce. Hopefully others can chime in with their experiences. With some other data points—versions of Atomic and ZNC in use and whether this behavior occurs with that combination—maybe this can be pinned to a new capability in ZNC that Atomic isn't handling (or handles incorrectly), or a regression in ZNC that needs to be reported and fixed there.

The issue manifests itself as follows:

I'll start: Using Atomic 1.19 from F-Droid with ZNC 1.6.0, I am seeing my own messages twice. I do not experience this behavior in other clients, either on Android or desktop.

dgw commented 8 years ago

Have it on good authority that @indrora uses ZNC 1.4, presumably with the latest (or a development) version of Atomic.

That's already a good starting point to look for differences, just between ZNC 1.4 and 1.6.

dgw commented 8 years ago

ZNC 1.6 added self-message support, and I have a theory. pIRCbot, which (if I'm reading the code for Atomic right) handles the IRC traffic for Atomic, appears to request the old znc.in/self-message capability if the server offers it in CAP :LS. Presumably it's also handling the echoed reply from ZNC, and sending it off to be posted to the conversation.

However, MsgHandler is posting the message to the conversation window as soon as it's sent. So it appears immediately, then is pushed to the conversation a second time when pIRCbot receives the echoed message from the ZNC server.

I was considering upgrading my ZNC from 1.6.0 to 1.6.2 (sacrificing 190 days of uptime in the process) to see if the latest stable version fixed this, but now that I've poked around I think the issue is squarely on the Atomic side.

Thoughts, @indrora? I might have missed something, since poking around was mostly motivated by insomnia and I'm quite tired.

indrora commented 8 years ago

The logic should be "request every cap we are handed back to us", so if ZNC has a new one that echoes back lines, I'll be willing to bet that's what it is.

I'm not sure what purpose the " send me my own lines" feature tries to achieve.

Upon testing with another client that does the same thing and a recent build of znc on my local box (an older hexchat from 2013, znc 1.6.0) I can confirm that it's a thing. Hexchat follows the same logic that we do, and apparently has better handling ("why would we need our own messages? Ignored! ")

Debian hasn't gotten to znc1.6 yet (or it has and I just haven't restarted the bugger) so I hadn't been able to test.

Not sure what I should do though. I'm tempted to dig around the znc codebase and figure out what the rationale was.

On January 30, 2016 8:23:26 AM MST, dgw notifications@github.com wrote:

ZNC 1.6 added self-message support, and I have a theory. pIRCbot, which (if I'm reading the code for Atomic right) handles the IRC traffic for Atomic, appears to request the old znc.in/self-message capability if the server offers it in CAP :LS. Presumably it's also handling the echoed reply from ZNC, and sending it off to be posted to the conversation.

However, MsgHandler is posting the message to the conversation window as soon as it's sent. So it appears immediately, then is pushed to the conversation a second time when pIRCbot receives the echoed message from the ZNC server.

I was considering upgrading my ZNC from 1.6.0 to 1.6.2 (sacrificing 190 days of uptime in the process) to see if the latest stable version fixed this, but now that I've poked around I think the issue is squarely on the Atomic side.

Thoughts, @indrora? I might have missed something, since poking around was mostly motivated by insomnia and I'm quite tired.


Reply to this email directly or view it on GitHub: https://github.com/indrora/Atomic/issues/118#issuecomment-177208953

Sent from my Android device with K-9 Mail. Please excuse my brevity.

dgw commented 8 years ago

As I understand it, the point of self-message/echo-message is 1) to better support bouncers forwarding sent messages to other connected clients; and 2) to allow clients to display the message using tags (server-time, mostly) so it appears the same way to the local user as it does to everyone else in the channel.

Until echo-message and its kin, it was kind of undefined what to do if a client received a message that appeared to be from itself. Some drop them, some post them as normal, some put them in the server log/lobby (this is especially true for PRIVMSGs sent to users instead of channels).

Obviously it would fix this to just not have MsgHandler post the message to the conversation itself, but that behavior really needs to be suppressed only if self-messages are enabled for the current server… and I'm too tired now to do more code spelunking to see if that info's available to MsgHandler. (I suspect it isn't.)

Debian stable is indeed stuck on ZNC 1.4 (1.5 years old according to the #znc topic on freenode). Because my server runs Ubuntu 12.04 LTS (I should update that when 16.04 drops…) I'd be stuck with ZNC 0.206 if not for building my own version.

indrora commented 8 years ago

Well.

In a clandestine meander, turns out I'm not doing The Bad Thing™. The offending line says no, we explicitly ask for znc.in/self-message.

It wasn't documented well at the onset, and I'll probably re-enable it later, but for now:

fgsfds

dgw commented 8 years ago

The biggest of :+1:s for you, thanks!

SaphireLattice commented 8 years ago

Huh, the '2.1' version doesn't has such problem (didn't checked the 1.9 mentioned in the issue). Sending messages trough ZNC 1.6.3 works fine, i get only one message. Though i do get WHO, WHOIS, PING and so on from other clients. But messages don't duplicate.