Closed Bug 955532 Opened 6 years ago Closed 6 years ago

MUC participants don't leave the MUC when they go offline

Categories

(Chat Core :: Yahoo! Messenger, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: qheaden)

Details

Attachments

(1 file, 3 obsolete files)

*** Original post on bio 2095 at 2013-08-02 11:58:00 UTC ***

STR
Join a MUC
Disconnect the account
Watch (from another instance) as nothing changes in the MUC.
Attached patch Patch v1 (obsolete) — Splinter Review
*** Original post on bio 2095 as attmnt 2679 at 2013-08-02 18:32:00 UTC ***

This patch fixes the issue by closing the conferences (and conversations) on disconnect.
Attachment #8354448 - Flags: review?(clokep)
Assignee: nobody → qheaden
Status: NEW → ASSIGNED
Comment on attachment 8354448 [details] [diff] [review]
Patch v1

*** Original change on bio 2095 attmnt 2679 at 2013-08-02 18:51:48 UTC ***

I think I was not clear enough in the bug report. The situation I was describing was when a participant in a MUC goes offline. I was running two instances of IB, and disconnected one of them, which had a Yahoo user who was a participant in a chatroom that the other instance (with a different username) also was in. The instance that remains connected does not notice the fact that a participant left the room.

Regarding this patch, we generally don't close conversations only because the account is disconnected.
Attachment #8354448 - Flags: review?(clokep) → review-
*** Original post on bio 2095 at 2013-08-02 20:07:01 UTC ***

Okay, we can keep the conversations open, but we need to send a disconnect packet for each open conference. The reason the participant stays connected is because YahooAccount.disconnect() doesn't send any disconnect notifications to the conference room.
Attached patch Patch 2 (obsolete) — Splinter Review
*** Original post on bio 2095 as attmnt 2722 at 2013-08-16 16:56:00 UTC ***

This patch fixes the issue without automatically closing the chat window. Whenever the account is logged off, a conference logoff packet is sent to the Yahoo server, and the "left" property on the conference is set to true.
Attachment #8354491 - Flags: review?(clokep)
Comment on attachment 8354448 [details] [diff] [review]
Patch v1

*** Original change on bio 2095 attmnt 2679 at 2013-08-16 16:56:33 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354448 - Attachment is obsolete: true
*** Original post on bio 2095 at 2013-08-16 17:02:38 UTC ***

Does this rejoin left conferences on reconnect?
*** Original post on bio 2095 at 2013-08-16 19:07:21 UTC ***


(In reply to comment #5)
> Does this rejoin left conferences on reconnect?

No, it doesn't, and it can't. Once you leave a Yahoo conference, you cannot "join" it back. The only way to get back in is to be invited.
Comment on attachment 8354491 [details] [diff] [review]
Patch 2

*** Original change on bio 2095 attmnt 2722 at 2013-08-21 15:11:09 UTC ***

>+    this._account._session.sendConferenceLogoff(this._account.cleanUsername,
>+                                            this.getParticipantNames(),
>+                                            this._roomName);
Nit: Spacing here looks wrong.

>@@ -191,16 +198,20 @@ YahooAccount.prototype = {
>   disconnect: function(aSilent) {
>+    // Report logoffs on any conference.
This comment doesn't say much more than the code, maybe "Log off of every conference the user is currently in." or something (that still isn't great).

>+    for (let conf of this._conferences)
>+      conf[1].reportLogoff();
Attachment #8354491 - Flags: review?(clokep) → review-
Attached patch Patch 3 (obsolete) — Splinter Review
*** Original post on bio 2095 as attmnt 2744 at 2013-08-21 18:04:00 UTC ***

This patch addresses the feedback given in the last review. As was mentioned, the comment explaining that we are logging out of the conferences was unnecessary, so I removed it altogether. The code is self-explanatory.
Attachment #8354513 - Flags: review?(clokep)
Comment on attachment 8354491 [details] [diff] [review]
Patch 2

*** Original change on bio 2095 attmnt 2722 at 2013-08-21 18:04:23 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354491 - Attachment is obsolete: true
Comment on attachment 8354513 [details] [diff] [review]
Patch 3

*** Original change on bio 2095 attmnt 2744 at 2013-08-21 18:17:31 UTC ***

Where did the reportLogoff function go? Was this patch only half the changes or something?

Also, I kind of would like a comment there, but what that phrases it slightly different then the code.
Attachment #8354513 - Flags: review?(clokep) → review-
Attached patch Patch 4Splinter Review
*** Original post on bio 2095 as attmnt 2745 at 2013-08-21 18:30:00 UTC ***

I added back a more meaningful comment above the conference logoff code in the disconnect() method.
Attachment #8354514 - Flags: review?(clokep)
Comment on attachment 8354513 [details] [diff] [review]
Patch 3

*** Original change on bio 2095 attmnt 2744 at 2013-08-21 18:30:07 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354513 - Attachment is obsolete: true
Comment on attachment 8354514 [details] [diff] [review]
Patch 4

*** Original change on bio 2095 attmnt 2745 at 2013-08-21 18:31:27 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354514 - Flags: review?(clokep) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 2095 at 2013-08-21 23:01:26 UTC ***

http://hg.instantbird.org/instantbird/rev/cecd59b0227e
Status: ASSIGNED → RESOLVED
Closed: 6 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.