*** Original post on bio 1300 at 2012-02-29 10:00:00 UTC *** STR: 1. Join a channel. 2. /part the channel 3. Close the tab. Result: I received a message "10:26:56 - concrete.mozilla.org: There is no channel: Mic." Expected result: no message, the tab should just go away. Here's the output on the error console, with logging level = 2 (blank lines mean that a new error message starts, time order is from earliest on top to latest at the end): Sending: JOIN :#mictest onTransportStatus(STATUS_SENDING_TO) onTransportStatus(STATUS_RECEIVING_FROM) :Mic!Instantbir@<bla..blub> JOIN :#mictest :concrete.mozilla.org MODE #mictest +n :concrete.mozilla.org 353 Mic = #mictest :@Mic :concrete.mozilla.org 366 Mic #mictest :End of /NAMES list. Sending: PART :#mictest onTransportStatus(STATUS_SENDING_TO) onTransportStatus(STATUS_RECEIVING_FROM) :Mic!Instantbir@<bla..blub> PART #mictest Sending: PART :#mictest onTransportStatus(STATUS_SENDING_TO) onTransportStatus(STATUS_RECEIVING_FROM) :concrete.mozilla.org 403 Mic #mictest :No such channel

*** Original post on bio 1300 as attmnt 1210 at 2012-03-01 02:16:00 UTC *** This checks now if we're in the room before parting! (It also makes a small whitespace change and sets the socket to log using DEBUG instead of LOG.)

*** Original post on bio 1300 as attmnt 1212 at 2012-03-01 11:45:00 UTC *** flo pointed out on IRC that the log change would also affect how messages are logged, not just the socket status. We don't want this and I'll need to do more extensive changes to the socket code. A revised patch is attached without that change.

Comment on attachment 8352961 [details] [diff] [review] Patch *** Original change on bio 1300 attmnt 1210 at 2012-03-01 11:45:58 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 1300 at 2012-03-01 12:39:31 UTC *** Comment on attachment 8352963 [details] [diff] [review] (bio-attmnt 1212) Patch v2 >diff --git a/chat/protocols/irc/irc.js b/chat/protocols/irc/irc.js >--- a/chat/protocols/irc/irc.js >+++ b/chat/protocols/irc/irc.js >@@ -113,17 +113,17 @@ ircChannel.prototype = { > if (msg) > params.push(msg); > > this._account.sendMessage("PART", params); > }, > > unInit: function() { > // Tell the server about the part if connected. >- if (this._account.connected) >+ if (this._account.connected && !this.left) > this.part(); Shouldn't this be done in the close method instead of the unInit method? The comment at http://lxr.instantbird.org/instantbird/source/chat/components/public/prplIConversation.idl#82 says that unInit is also called during shutdown, it may be the reason why we part channels instead of sending quit when disconnected because of shutdown. (By the way, that comment is obsolete, as purpleCoreService most likely isn't relevant any more now that bug 954193 (bio 759) is fixed.)

*** Original post on bio 1300 at 2012-03-01 13:17:27 UTC *** (In reply to comment #3) > Comment on attachment 8352963 [details] [diff] [review] (bio-attmnt 1212) [details] > Patch v2 > > >diff --git a/chat/protocols/irc/irc.js b/chat/protocols/irc/irc.js > >--- a/chat/protocols/irc/irc.js > >+++ b/chat/protocols/irc/irc.js > >@@ -113,17 +113,17 @@ ircChannel.prototype = { > > if (msg) > > params.push(msg); > > > > this._account.sendMessage("PART", params); > > }, > > > > unInit: function() { > > // Tell the server about the part if connected. > >- if (this._account.connected) > >+ if (this._account.connected && !this.left) > > this.part(); > > Shouldn't this be done in the close method instead of the unInit method? It probably should be. The close() method is called when the conversation is closed from the UI? The comment just says /* Close the conversation */ which doesn't really give any extra information. > The comment at > http://lxr.instantbird.org/instantbird/source/chat/components/public/prplIConversation.idl#82 > says that unInit is also called during shutdown, it may be the reason why we > part channels instead of sending quit when disconnected because of shutdown. That is probably the problem! I'll put up a new patch later that handles this better. > (By the way, that comment is obsolete, as purpleCoreService most likely isn't > relevant any more now that bug 954193 (bio 759) is fixed.) We should fix that! :)

*** Original post on bio 1300 at 2012-03-01 14:58:25 UTC *** (In reply to comment #4) > The close() method is called when the conversation is > closed from the UI? I think so. > The comment just says /* Close the conversation */ which > doesn't really give any extra information. Fix it? :)

*** Original post on bio 1300 as attmnt 1213 at 2012-03-01 22:39:00 UTC *** This replaces the unInit function with the close function and fixes the comment about the close method.

Comment on attachment 8352963 [details] [diff] [review] Patch v2 *** Original change on bio 1300 attmnt 1212 at 2012-03-01 22:39:47 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 1300 as attmnt 1214 at 2012-03-01 22:50:00 UTC *** This keeps the unInit method and also removes an extra debug statement that I found. :(

Comment on attachment 8352964 [details] [diff] [review] Patch v3 *** Original change on bio 1300 attmnt 1213 at 2012-03-01 22:50:41 UTC was without comment, so any subsequent comment numbers will be shifted ***

*** Original post on bio 1300 at 2012-03-02 00:17:38 UTC *** A slightly modified patch was checked in as http://hg.instantbird.org/instantbird/rev/c525086f3a9c It calls the GenericConvChatPrototype close and unInit methods.

Comment on attachment 8352965 [details] [diff] [review] Patch v4 *** Original change on bio 1300 attmnt 1214 at 2012-03-02 15:39:55 UTC *** Updated patch given over IRC and checked in.