Closed
Bug 955559
Opened 10 years ago
Closed 10 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•10 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•10 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•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment 3•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 years ago
|
Whiteboard: [checkin-needed]
Comment 13•10 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: 10 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
•