Closed
Bug 955559
Opened 11 years ago
Closed 11 years ago
The /join command shouldn't send a JOIN command to the server if we are already in the channel
Categories
(Chat Core :: IRC, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: florian, Assigned: aleth)
Details
Attachments
(1 file, 2 obsolete files)
1.06 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** 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
Assignee | ||
Comment 1•11 years ago
|
||
*** 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) ;)
Assignee | ||
Comment 2•11 years ago
|
||
*** 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 ***
Attachment #8354522 -
Flags: review?(clokep)
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment 3•11 years ago
|
||
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.
Attachment #8354522 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 4•11 years ago
|
||
*** Original post on bio 2121 at 2013-08-23 10:46:54 UTC ***
The question is whether this check should be in joinChat() instead.
Reporter | ||
Comment 5•11 years ago
|
||
*** 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•11 years ago
|
||
*** 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.
Assignee | ||
Comment 7•11 years ago
|
||
*** Original post on bio 2121 as attmnt 2754 at 2013-08-23 10:56:00 UTC ***
Thanks for catching that.
Attachment #8354523 -
Flags: review?(clokep)
Assignee | ||
Comment 8•11 years ago
|
||
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 ***
Attachment #8354522 -
Attachment is obsolete: true
Comment 9•11 years ago
|
||
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;
>+
Attachment #8354523 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 10•11 years ago
|
||
*** 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 ***
Attachment #8354524 -
Flags: review?(clokep)
Assignee | ||
Comment 11•11 years ago
|
||
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 ***
Attachment #8354523 -
Attachment is obsolete: true
Comment 12•11 years ago
|
||
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 ***
Attachment #8354524 -
Flags: review?(clokep) → review+
Updated•11 years ago
|
Whiteboard: [checkin-needed]
Comment 13•11 years ago
|
||
*** Original post on bio 2121 at 2013-08-23 21:46:39 UTC ***
Thanks.
http://hg.instantbird.org/instantbird/rev/df197dc3277d
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
You need to log in
before you can comment on or make changes to this bug.
Description
•