Filter Kick Message is broke

[MikeJJ]

This has been broke for ages. "Send kick messages to status bar".
I believe i have seen the cause of the problem, but don't want to attempt a fix for fear of breaking something.

In Hubframe.cpp to be filtered the messages need to goto
void HubFrame::on(StatusMessage, Client*, const string& line) throw() {

But in NMDCHub.cpp
void NmdcHub::onLine(const string& aLine) throw() {
these messages avoid the parts which send StatusMessage, and end up getting sent via normal Message, missing the kick filter.

[MikeJJ (mrmikejj)]

Thanks Pretorian for the advice. :)
Just moved the Filtering checker from StatusMessage to Message.

Since the "normal" message didn't get passed through Text::toDOS() i removed this as well for the filtered message, but if it's needed i can make a new patch with it in.

[Jacek Sieka (arnetheduck)]

anything that goes to multi-line edit boxes needs toDOS, although this is a border case since mulitline kick messages are unlikely (toDOS converts to dos line endings)...anyway, I moved toDOS to addChat so that it'll always be used...simpler that way...

A cleaner solution would probably be to add an enum to Message which would say what type of message it is (and remove the StatusMessage signal) and filter by text in nmdc and do it cleanly in adc

[MikeJJ (mrmikejj)]

I've seen quite a few multiline kick messages (mainly from hubs abusing the filter to spam the status bar with advertisements.

Could do with a little advice for the cleaner solution . . . i assumed you mean sending messages like:
fire(ClientListener::Message(), this, ClientListener::MESSAGE, unescape(message), *ou);
fire(ClientListener::Message(), this, ClientListener::STATUS_MESSAGE, unescape(line));
fire(ClientListener::Message(), this, ClientListener::SILENT_STATUS_MESSAGE, unescape(line));

(I had to swap the order for normal messages from "user, message" to "message, user").
But I fail at finding a working default for OnlineUser& for when that doesn't get passed (i.e. from the StatusMessages).

virtual void on(Message, Client*, MessageType messageType, const string&, const OnlineUser& = NULL, bool = false) throw() { }
(It doesn't like NULL)
./dcpp/ClientListener.h:47: error: default argument for parameter of type 'const dcpp::OnlineUser&' has type 'int'

Or am i going about this all wrong ?

[poy (poy)]

the OnlineUser argument is only used to get the nick of the sender, so perhaps the message could be formatted with Util::formatMessage before it is sent to the listener?

or if you want to keep that argument and give it a default value, you can either call its constructor (const OnlineUser& ou = OnlineUser(...) but the OnlineUser constructor takes many parameters...) or use a pointer instead (OnlineUser* pOU = NULL).

[MikeJJ (mrmikejj)]

Thanks poy. I went with the first way you suggested (Util::formatMessage before sent to listener) as i figured it's better than sending stuff which is never used.
Please check this carefully though :)
I also made some of the status messages in ADCHub.cpp translatable.

Also a comment about this:
 } else if(messageType == ClientListener::STATUS_MESSAGE) {
  speak(ADD_STATUS_LINE, msg);
This is different from the original code, since was sending Status Messages to
  speak(ADD_CHAT_LINE, msg);
But i assumed that was wrong, since like that, there is no "real" status messages.

As for kick message filtering "cleanly in adc" i was at a loss of how / where to do this, so left it.

[Jacek Sieka (arnetheduck)]

Hm, can't say I like formatting in the client lib - it's clearly a gui thing (and perhaps mods what to format differently using colors etc)...perhaps it would make sense with two types of messages after all, usermessage and statusmessage where the first has an onlineuser and the second a type flag...

in adc the kick message should arrive with qui so there's no need to send a normal message first - then the qui message would have the kick type...this is related to #186675

[MikeJJ (mrmikejj)]

Something like this then ?

[Pseudonym (404emailnotfound)]

+void HubFrame::on(StatusMessage, Client*, const string& line, int statusFlags) throw() {
+ if(SETTING(FILTER_MESSAGES) && ClientListener::FLAG_IS_SPAM) {
+ speak(ADD_SILENT_STATUS_LINE, line);
  } else {
- speak(ADD_CHAT_LINE, Util::formatMessage(from.getIdentity().getNick(), msg, thirdPerson));
+ speak(ADD_STATUS_LINE, line);
  }
 }

This is wrong; I think you mean:
if(SETTING(FILTER_MESSAGES) && (statusFlags & ClientListener::FLAG_IS_SPAM))

otherwise, it reduces to just checking if FILTER_MESSAGES is true :)

[MikeJJ (mrmikejj)]

oh, okay, thanks Pseudonym :)

[MikeJJ (mrmikejj)]

Regarding:
Also a comment about this:
 } else if(messageType == ClientListener::STATUS_MESSAGE) {
  speak(ADD_STATUS_LINE, msg);
This is different from the original code, since was sending Status Messages to
  speak(ADD_CHAT_LINE, msg);
But i assumed that was wrong, since like that, there is no "real" status messages.

This seems to have had unexpected side effects (well i didn't expect them anyway). i.e. the chat messages the hub sends using e.g. +me now get filtered to the Status Bar. They also don't turn up in main chat, even though i have "view status messages in main chat" turned on. Well ones i send do goto main, but ones other people send don't.