955690 - /me in a line that isn't the first is sent as a raw string inside the message instead of an action line

Status: NEW

(Chat Core :: IRC, defect)

Description

[Florian Quèze [:florian]]

*** Original post on bio 2242 at 2013-11-09 23:17:00 UTC ***

Having "blahblah\n/me action" in the input box and sending causes us to send "/me" instead of whatever we are expected to send.

Comment 1

[Patrick Cloke [:clokep]]

*** Original post on bio 2242 at 2013-11-09 23:23:41 UTC ***

Just to be clear, does this cause us to send an ACTION message or are we just displaying it as an action (because of our plaintext parsing of /me) on the incoming message?

Comment 2

[Florian Quèze [:florian]]

*** Original post on bio 2242 at 2013-11-09 23:29:54 UTC ***

(In reply to comment #1)
> Just to be clear, does this cause us to send an ACTION message

No.

> are we just
> displaying it as an action (because of our plaintext parsing of /me) on the
> incoming message?

We are. But other IRC clients (like instantbot's logs) aren't.

Comment 3

[Florian Quèze [:florian]]

*** Original post on bio 2242 at 2013-11-09 23:38:39 UTC ***

This vaguely sounds like a regression from bug 955045 (bio 1615).

Comment 4

[Patrick Cloke [:clokep]]

What's the expected result of this?

Comment 5

[Florian Quèze [:florian]]

The expected result is that typing

blah<enter>/me tests<enter>

or

blah<shift+enter>/me tests<enter>

sends the same thing.

Comment 6

[Pavan Karthik [:matrixisreal]]

Hi,

Is this bug still reproducable?

and by blah\n/me action you mean blah<shift+enter>/me tests<enter>?

Comment 7

[Patrick Cloke [:clokep]]

Yes, this is still reproducible, you can't see it on Instantbird because of a different bug, unfortunately.

Typing "blah\n/me action" (yes, you end \n using shift+enter) sends two messages:
PRIVMSG #ib-test blah
PRIVMSG #ib-test :/me action

It should send:
PRIVMSG #ib-test blah
PRIVMSG #ib-test :\x01ACTION action\x01

Comment 8

[Pavan Karthik [:matrixisreal]]

Attachment: v1.patch

this works, but it touches the same functions as couple of my previous functions,
So this will need some discussion.

Comment 9

[arlolra]

Presumably, this is a more general problem for commands and not just action msgs?

Comment 10

[Patrick Cloke [:clokep]]

arlolra, it's an issue for commands, yes. I think this actually used to work when we did splitting by lines in the UI instead of in the prpl...

Comment 11

[Patrick Cloke [:clokep]]

Comment on attachment 8829245 [details] [diff] [review]
v1.patch

Review of attachment 8829245 [details] [diff] [review]:
-----------------------------------------------------------------

I don't really like the behavior of this. I don't think prpls should be doing command parsing themselves. We should probably talk about this on IRC.

Comment 12

[Pavan Karthik [:matrixisreal]]

Attachment: v2.patch

Version 2:
Split the messages at every line break before prpls get the control.

Comment 13

[Patrick Cloke [:clokep]]

Comment on attachment 8841331 [details] [diff] [review]
v2.patch

Review of attachment 8841331 [details] [diff] [review]:
-----------------------------------------------------------------

Hey Florian, do you remember why we *stopped* splitting lines in the UI? Or was it optional per protocol or something?

Comment 14

[Florian Quèze [:florian]]

Comment on attachment 8841331 [details] [diff] [review]
v2.patch

(In reply to Patrick Cloke [:clokep] from comment #13)
> Comment on attachment 8841331 [details] [diff] [review]
> v2.patch
> 
> Review of attachment 8841331 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hey Florian, do you remember why we *stopped* splitting lines in the UI? Or
> was it optional per protocol or something?

I don't remember for sure, no. But if I had to make a guess I would say it was related to making our code more OTR friendly.

Comment 15

[Patrick Cloke [:clokep]]

Comment on attachment 8841331 [details] [diff] [review]
v2.patch

I'm unsure this is the correct approach. Regardless this patch would need to be applied to the Thunderbird IM code now (mail/components/im).