955559 - The /join command shouldn't send a JOIN command to the server if we are already in the channel

Status: RESOLVED FIXED

(Chat Core :: IRC, defect)

Description

[Florian Quèze [:florian]]

*** Original post on bio 2121 at 2013-08-22 23:12:00 UTC ***

Steps to reproduce:
1. Join a channel.
2. Put the conversation on hold.
3. Forget about it.
4. Attempt to join it (doing it from the Join Chat dialog will reopen the existing conversation on hold; doing it with the /join command will just do nothing).

The debug log shows:
prpl-irc: Sending:
JOIN #channel

Comment 1

[aleth [:aleth]]

*** Original post on bio 2121 at 2013-08-23 10:00:36 UTC ***

While this is a bug, I suspect what really annoyed here was bug 954270 (bio 837) ;)

Comment 2

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 2121 as attmnt 2753 at 2013-08-23 10:44:00 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 3

[Patrick Cloke [:clokep]]

Comment on attachment 8354522 [details] [diff] [review]
Patch

*** Original change on bio 2121 attmnt 2753 at 2013-08-23 10:46:29 UTC ***

I'm pretty sure this will break if we join a channel, part it and want to rejoin.

Comment 4

[aleth [:aleth]]

*** Original post on bio 2121 at 2013-08-23 10:46:54 UTC ***

The question is whether this check should be in joinChat() instead.

Comment 5

[Florian Quèze [:florian]]

*** Original post on bio 2121 at 2013-08-23 10:50:12 UTC ***

(In reply to comment #1)
> While this is a bug, I suspect what really annoyed here was bug 954270 (bio 837) ;)

No, what really annoyed was a broken local debug build. This bug just added to my confusion about what was going on.

Comment 6

[Patrick Cloke [:clokep]]

*** Original post on bio 2121 at 2013-08-23 10:53:26 UTC ***

(In reply to comment #4)
> The question is whether this check should be in joinChat() instead.

Yes.

Comment 7

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 2121 as attmnt 2754 at 2013-08-23 10:56:00 UTC ***

Thanks for catching that.

Comment 8

[aleth [:aleth]]

Comment on attachment 8354522 [details] [diff] [review]
Patch

*** Original change on bio 2121 attmnt 2753 at 2013-08-23 10:56:12 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 9

[Patrick Cloke [:clokep]]

Comment on attachment 8354523 [details] [diff] [review]
Patch

*** Original change on bio 2121 attmnt 2754 at 2013-08-23 11:44:15 UTC ***

>diff --git a/chat/protocols/irc/irc.js b/chat/protocols/irc/irc.js
>@@ -1246,16 +1246,19 @@ ircAccount.prototype = {
>     // A channel prefix is required. If the user didn't include one,
>     // we prepend # automatically to match the behavior of other
>     // clients. Not doing it used to cause user confusion.
>     if (this.channelPrefixes.indexOf(channel[0]) == -1)
>       channel = "#" + channel;
Nit: Add an empty line here.

Please also include a comment saying what this check is doing ("Don't try to rejoin a room the user is already in!").
>+    if (this.hasConversation(channel) && !this.getConversation(channel).left)
>+      return;
>+

Comment 10

[aleth [:aleth]]

Attachment: Patch

*** Original post on bio 2121 as attmnt 2755 at 2013-08-23 11:46:00 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 11

[aleth [:aleth]]

Comment on attachment 8354523 [details] [diff] [review]
Patch

*** Original change on bio 2121 attmnt 2754 at 2013-08-23 11:46:47 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 12

[Patrick Cloke [:clokep]]

Comment on attachment 8354524 [details] [diff] [review]
Patch

*** Original change on bio 2121 attmnt 2755 at 2013-08-23 11:52:31 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 13

[Patrick Cloke [:clokep]]

*** Original post on bio 2121 at 2013-08-23 21:46:39 UTC ***

Thanks.
http://hg.instantbird.org/instantbird/rev/df197dc3277d