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)

x86
Other
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: aleth)

Details

Attachments

(1 file, 2 obsolete files)

*** 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
*** 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) ;)
Attached patch Patch (obsolete) — Splinter Review
*** 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: nobody → aleth
Status: NEW → ASSIGNED
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-
*** Original post on bio 2121 at 2013-08-23 10:46:54 UTC *** The question is whether this check should be in joinChat() instead.
*** 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.
*** 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.
Attached patch Patch (obsolete) — Splinter Review
*** 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)
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 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-
Attached patch PatchSplinter Review
*** 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)
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 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+
Whiteboard: [checkin-needed]
*** 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.

Attachment

General

Creator:
Created:
Updated:
Size: