Closed
Bug 954610
Opened 10 years ago
Closed 10 years ago
Regression: renamed contacts disappear from list
Categories
(Instantbird Graveyard :: Contacts window, defect)
Instantbird Graveyard
Contacts window
Tracking
(Not tracked)
RESOLVED
FIXED
1.2
People
(Reporter: benediktp, Assigned: florian)
References
Details
(Whiteboard: [1.2-blocking])
Attachments
(1 file, 2 obsolete files)
2.13 KB,
patch
|
clokep
:
review-
|
Details | Diff | Splinter Review |
*** Original post on bio 1178 at 2011-11-19 17:31:00 UTC *** When a renamed contact needs to be moved to a new position in the contact list, it disappears from the old position (as soon as it is no longer selected, which delays the animation) but does not appear in the new position. Hiding the tag and showing it again makes it reappear at the correct position. This is most likely a regression caused by the patch for bug 954206 (bio 772) which landed recently.
Assignee | ||
Updated•10 years ago
|
Whiteboard: [1.2-blocking]
Comment 1•10 years ago
|
||
*** Original post on bio 1178 as attmnt 1118 at 2012-01-14 21:53:00 UTC *** I don't think this is a regression from bug 954206 (bio 772), but the fix is an easy one-liner.
Attachment #8352861 -
Flags: review?(florian)
Updated•10 years ago
|
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Comment 2•10 years ago
|
||
Comment on attachment 8352861 [details] [diff] [review] Fix v1 *** Original change on bio 1178 attmnt 1118 at 2012-01-14 22:18:32 UTC *** So actually this doesn't seem correct, removeNode() is supposed to call the removeContact function at: http://lxr.instantbird.org/instantbird/source/instantbird/content/contact.xml#271 Looking into this more. (Note that even if we were to go this way, I was able to get duplicated contacts with this patch....so you'd still need the removeNode() function, although I'd suggest putting it into the removeContact function.)
Attachment #8352861 -
Flags: review?(florian)
Comment 3•10 years ago
|
||
*** Original post on bio 1178 at 2012-01-14 22:32:25 UTC *** OK, so I think the issue is that removeNode() has some transitions in it based on the fading out of the contact; so then when we try to add the contact back in, the element already exists and it drops out early. Something in here needs to be redesigned I think. If we add a removeContact call right before addContact (as my patch kind of suggests, but doesn't fully work). This would ensure that the contact is always (forcibly) removed before we try to add it, but that feels like a bit of a hack. Opinions?
Comment 4•10 years ago
|
||
*** Original post on bio 1178 as attmnt 1120 at 2012-01-15 15:05:00 UTC *** This is slightly less hacky version of what I described in comment 3. Pretty much what was occurring is: - removeNode for the contact element is called. - Immediately after, addContact is called. - The check for whether the contact is still in the list of contact IDs evaluates to true and the addContact method short circuits. - The the removeContact method is called (usually ~1 second later, actually). This removes the ID from the list of contacts, etc. and is clearly meant to be run before addContact. This patch: - Removes the call the contact element makes to removeContact. - Calls removeContact before addContact to ensure that the ID will definitely be deleted from the list of contact IDs.
Attachment #8352863 -
Flags: review?(florian)
Comment 5•10 years ago
|
||
Comment on attachment 8352861 [details] [diff] [review] Fix v1 *** Original change on bio 1178 attmnt 1118 at 2012-01-15 15:05:21 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352861 -
Attachment is obsolete: true
Comment 6•10 years ago
|
||
Comment on attachment 8352863 [details] [diff] [review] Fix v2 *** Original change on bio 1178 attmnt 1120 at 2012-01-15 17:25:39 UTC *** So this doesn't fully work because: 1. I call removeContact with the wrong parameter. 2. It messes up the remove animation of contacts.
Attachment #8352863 -
Flags: review?(florian)
Updated•10 years ago
|
Severity: normal → major
OS: Windows 7 → All
Hardware: x86 → All
Comment 7•10 years ago
|
||
*** Original post on bio 1178 as attmnt 1123 at 2012-01-17 00:13:00 UTC *** This is essentially the same as the previous patch, but it's a bit cleaner. The effects of this are a bit odd in that the new contact is visually added BEFORE the old contact disappears (it gets added while the old contact is disappearing). I'm unsure of this is acceptable or if we'd prefer a more invasive change.
Attachment #8352866 -
Flags: review?(florian)
Comment 8•10 years ago
|
||
Comment on attachment 8352863 [details] [diff] [review] Fix v2 *** Original change on bio 1178 attmnt 1120 at 2012-01-17 00:13:46 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352863 -
Attachment is obsolete: true
Comment 10•10 years ago
|
||
Comment on attachment 8352866 [details] [diff] [review] Fix v2-r2 *** Original change on bio 1178 attmnt 1123 at 2012-02-16 12:45:42 UTC *** This breaks the animation in the contact list (which is meant to keep the total height of the group static).
Attachment #8352866 -
Flags: review?(florian) → review-
Comment 11•10 years ago
|
||
*** Original post on bio 1178 at 2012-02-16 12:46:22 UTC *** I'm out of ideas for this without really cracking into the animation code.
Status: ASSIGNED → NEW
Assignee | ||
Comment 12•10 years ago
|
||
*** Original post on bio 1178 at 2012-03-02 11:45:09 UTC *** I believe this is a regression from https://hg.instantbird.org/instantbird/rev/ca16aba596fb (bug 954110 (bio 675)). I pushed https://hg.instantbird.org/instantbird/rev/c52ff748f62c to fix this (clokep r+'ed over IRC).
Assignee: clokep → florian
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in
before you can comment on or make changes to this bug.
Description
•