Closed Bug 955711 Opened 7 years ago Closed 7 years ago

Buddy disappears when detached in the Other Contacts tag

Categories

(Instantbird :: Contacts window, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aleth, Assigned: aleth)

Details

Attachments

(1 file, 1 obsolete file)

*** Original post on bio 2262 at 2013-11-29 14:47:00 UTC ***

Add a (currently visible) tag to a contact in Other Contacts. The additional tag is shown with the contact in it, but the contact does not disapper from Other Contacts.

Detach a buddy from a merged contact in Other Contacts. The buddy disappears.
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 2262 as attmnt 3102 at 2013-11-30 15:03:00 UTC ***

Can't reproduce the first issue. Maybe I got into a strange state after discovering the second issue, which this patch fixes.

Also adds a this._observers check which prevents a shutdown error when quitting when a contact is expanded (removeObserver is called from the buddy binding destructor).
Attachment #8354885 - Flags: review?(florian)
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Summary: Contacts in Other Contacts tag behave badly → Buddy disappears when detached in the Other Contacts tag
Comment on attachment 8354885 [details] [diff] [review]
Patch

*** Original change on bio 2262 attmnt 3102 at 2013-12-01 17:25:00 UTC ***

>diff --git a/chat/components/src/imContacts.js b/chat/components/src/imContacts.js
>index 3af12db..a621805 100644
>--- a/chat/components/src/imContacts.js
>+++ b/chat/components/src/imContacts.js
>@@ -291,16 +291,24 @@ var otherContactsTag = {
>           aSubject.getTags().every(function(t) t.id in this._hiddenTags, this))
>         this._addContact(aSubject);
>     }
>     else if (aSubject.id in this._contacts &&
>              (aTopic == "contact-removed" ||
>               (aTopic == "contact-tag-added" &&
>               !(parseInt(aData) in this._hiddenTags))))
>       this._removeContact(aSubject);
>+    else if (aTopic == "contact-tag-added" &&
>+             !(aSubject.id in this._contacts) &&
>+             parseInt(aData) in this._hiddenTags &&
>+             aSubject.getTags().every(function(t) t.id in this._hiddenTags, this)) {
>+      // This happens when detaching a buddy from a contact that has only
>+      // hidden tags.
>+      this._addContact(aSubject);
>+    }

Shouldn't detaching a buddy from a contact trigger a "contact-added" notification from the detachBuddy method, so that we go through the normal code path?


>   removeObserver: function(aObserver) {
>+    if (!this.hasOwnProperty("_observers"))

I assume you copied this from the Contact.prototype.removeObserver method, but I don't see why we would need an hasOwnProperty call here. Can't we just do:
if (!this._observers)
?
Attachment #8354885 - Flags: review?(florian) → review-
Attached patch Patch2Splinter Review
*** Original post on bio 2262 as attmnt 3106 at 2013-12-01 18:38:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354890 - Flags: review?(florian)
Comment on attachment 8354885 [details] [diff] [review]
Patch

*** Original change on bio 2262 attmnt 3102 at 2013-12-01 18:38:06 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354885 - Attachment is obsolete: true
Comment on attachment 8354890 [details] [diff] [review]
Patch2

*** Original change on bio 2262 attmnt 3106 at 2013-12-01 23:18:32 UTC ***

This seems reasonable; let's try it! :)
Attachment #8354890 - Flags: review?(florian) → review+
*** Original post on bio 2262 at 2013-12-01 23:31:07 UTC ***

http://hg.instantbird.org/instantbird/rev/23b989438ddc
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.5
You need to log in before you can comment on or make changes to this bug.