Closed
Bug 955048
Opened 10 years ago
Closed 10 years ago
Tags are never really removed from contacts
Categories
(Chat Core :: General, defect)
Chat Core
General
Tracking
(Not tracked)
RESOLVED
FIXED
1.2
People
(Reporter: florian, Assigned: florian)
References
Details
(Whiteboard: [1.2-blocking])
Attachments
(1 file)
4.19 KB,
patch
|
clokep
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1618 at 2012-08-05 01:40:00 UTC *** *** Due to BzAPI limitations, the initial description is in comment 1 ***
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1618 as attmnt 1790 at 2012-08-05 01:40:00 UTC *** 2 issues with removing tags from contacts made testing my patch from bug 954956 (bio 1524) difficult: - we keep lots of left-over rows in the contact_tag table, causing tags removed from an account_buddy to still be kept on the contact (after a restart!, when rereading the blist from blist.sqlite). - if a contact is removed from a tag and the contact signs on during the fading animation, the contact stays displayed in the group in the UI.
Attachment #8353549 -
Flags: review?(clokep)
Assignee | ||
Updated•10 years ago
|
Severity: normal → major
Whiteboard: [1.2-blocking]
Comment 2•10 years ago
|
||
Comment on attachment 8353549 [details] [diff] [review] Patch *** Original change on bio 1618 attmnt 1790 at 2012-08-05 04:32:30 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353549 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1618 at 2012-08-05 09:16:12 UTC *** (In reply to comment #0) > - we keep lots of left-over rows in the contact_tag table, causing tags removed > from an account_buddy to still be kept on the contact (after a restart!, when > rereading the blist from blist.sqlite). To be clear, I'm not very comfortable with the imContacts part of the patch fixing this issue, and even less with taking it that late for 1.2. I believe this new behavior will cause problems in some corner cases, for example, if a contact c1 has two buddies b1 and b2, b1 has the t1 tag on the server, b2 has t2 on the server, the user adds the tag t3 to c1 from the UI, then for some reason (the user doing the same change from another client?) b2 has t3 on the server. With this situation, if the user detaches b2 from c1 or removes b2 from his list (by deleting the associated account for example), c1 will no longer have t3. I think we should take the patch anyway because: - these corner cases causing unexpected behaviors are much less likely to happen than the user removing a tag and wondering why it's back after a restart (the current behavior). - I don't have any better solution to offer. It's the least intrusive changes I could find that fixed the main issue here. I don't think there's any "real" solution without large refactoring of our code handling tags.
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 1618 at 2012-08-05 14:58:50 UTC *** Comment on attachment 8353549 [details] [diff] [review] (bio-attmnt 1790) Patch >diff --git a/chat/components/src/imContacts.js b/chat/components/src/imContacts.js >+ // aNewTag is now inherited by the contact from an account buddy, so avoid >+ // keeping direct tag <-> contact links in the contact_tag table. >+ ContactsById[buddy.contact.id]._removeContactTagRow(aNewTag); >+ > buddy.observe(aAccountBuddy, "account-buddy-moved"); > ContactsById[buddy.contact.id]._moved(aOldTag, aNewTag); After a night of sleep, this duplication of ContactsById[buddy.contact.id] looks awful. I'm adding a let contact = ContactsById[buddy.contact.id] before the check-in.
Comment 5•10 years ago
|
||
*** Original post on bio 1618 at 2012-08-06 01:56:12 UTC *** Checked in as http://hg.instantbird.org/instantbird/rev/428d3629a9d2
Assignee: nobody → florian
Status: NEW → RESOLVED
Closed: 10 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.
Description
•