JS-protocols should close their conversations and remove their buddies when they are removed

RESOLVED FIXED in 1.2

Status

Chat Core
General
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: florian, Assigned: florian)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [1.2-blocking])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
*** Original post on bio 1354 at 2012-03-27 14:50:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
(Assignee)

Comment 1

4 years ago
Created attachment 8353024 [details] [diff] [review]
Patch

*** Original post on bio 1354 as attmnt 1271 at 2012-03-27 14:50:00 UTC ***

Currently libpurple accounts do this when they are uninitialized, and we ignore these signals from libpurple if they are fired during shutdown.
JS protocols can't rely on this hack, and currently don't do this at all anyway.
Attachment #8353024 - Flags: review?(clokep)
(Assignee)

Updated

4 years ago
Whiteboard: [1.2-blocking]
Comment on attachment 8353024 [details] [diff] [review]
Patch

*** Original change on bio 1354 attmnt 1271 at 2012-03-27 14:56:47 UTC ***

>diff --git a/chat/modules/jsProtoHelper.jsm b/chat/modules/jsProtoHelper.jsm
>@@ -102,16 +103,17 @@ const ForwardAccountPrototype = {
> const GenericAccountPrototype = {
>+  remove: function() {},
Should this throw NS_ERROR_NOT_IMPLEMENTED so people making protocols know they should implement it?

>   unInit: function() {},
>   connect: function() { throw Cr.NS_ERROR_NOT_IMPLEMENTED; },
>   disconnect: function() { throw Cr.NS_ERROR_NOT_IMPLEMENTED; },
>   createConversation: function(aName) { throw Cr.NS_ERROR_NOT_IMPLEMENTED; },
>   joinChat: function(aComponents) { throw Cr.NS_ERROR_NOT_IMPLEMENTED; },

>diff --git a/chat/protocols/xmpp/xmpp.jsm b/chat/protocols/xmpp/xmpp.jsm
>@@ -181,20 +181,33 @@ const XMPPMUCConversationPrototype = {
>+  _left: false,
>+  get left() this._left,
>+  set left(aLeft) {
>+    if (aLeft == this._left)
>+      return;
>+
>+    this._left = aLeft;
>+    if (this._left)
>+      this.notifyObservers(null, "update-conv-chatleft");
>+  },
>+
Please fall back to the jsProtoHelper version here.

The rest looks OK. :)
Attachment #8353024 - Flags: review?(clokep) → review-
(Assignee)

Comment 3

4 years ago
Created attachment 8353025 [details] [diff] [review]
Patch v2

*** Original post on bio 1354 as attmnt 1272 at 2012-03-27 16:09:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353025 - Flags: review?(clokep)
(Assignee)

Comment 4

4 years ago
Comment on attachment 8353024 [details] [diff] [review]
Patch

*** Original change on bio 1354 attmnt 1271 at 2012-03-27 16:09:03 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353024 - Attachment is obsolete: true
(Assignee)

Updated

4 years ago
Assignee: nobody → florian
Comment on attachment 8353025 [details] [diff] [review]
Patch v2

*** Original change on bio 1354 attmnt 1272 at 2012-03-27 16:11:08 UTC ***

Looks good! Thanks for fixing this. :)
Attachment #8353025 - Flags: review?(clokep) → review+
*** Original post on bio 1354 at 2012-03-28 10:31:25 UTC ***

Checked in as http://hg.instantbird.org/instantbird/rev/7f5dfa28bc4e
Status: NEW → RESOLVED
Last Resolved: 4 years ago
OS: Other → All
Hardware: x86 → All
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.