Closed Bug 955646 Opened 7 years ago Closed 7 years ago

Duplicate items in the new conversation tab


(Instantbird :: Conversation, defect)

Not set


(Not tracked)



(Reporter: nhnt11, Assigned: nhnt11)




(1 file, 1 obsolete file)

*** Original post on bio 2201 at 2013-10-03 22:21:00 UTC ***

This seems to be a regression caused by bug 955582 (bio 2143). I don't have reliable STR at the moment.
Blocks: 955582
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 2201 as attmnt 2927 at 2013-10-04 15:11:00 UTC ***

When a contact's status changed, it was getting removed and re-added. In the process, a new PossibleConvFromContact is created rather than modifying the existing one (which is removed). The old one was not getting removed from _convsWithUpdatedStats.

The result was that we were trying to splice a conv out of this._convs, which wasn't in the array in the first place - we were instead removing the last element in the array because indexOf returned -1. In addition, we re-added the old conv, resulting in a duplicate.

I hope looking at the code makes this easier to understand, else I'll try to explain it more clearly.

The patch adds a check in repositionConvsWithUpdatedStats to make sure we're only attempting to reposition a conv if it's still in the list. I've also taken this opportunity to ensure we don't add a conv twice to _convsWithUpdatedStats.
Attachment #8354698 - Flags: review?(aleth)
Assignee: nobody → nhnt11
Attached patch Patch 2Splinter Review
*** Original post on bio 2201 as attmnt 2928 at 2013-10-04 21:00:00 UTC ***

Sets can't have duplicate elements, so we don't need the duplicate check. D'oh!
Attachment #8354699 - Flags: review?(benediktp)
Comment on attachment 8354698 [details] [diff] [review]

*** Original change on bio 2201 attmnt 2927 at 2013-10-04 21:00:07 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354698 - Attachment is obsolete: true
Attachment #8354698 - Flags: review?(aleth)
Comment on attachment 8354699 [details] [diff] [review]
Patch 2

*** Original change on bio 2201 attmnt 2928 at 2013-10-04 21:13:00 UTC ***

I can follow the idea what was wrong here and the fix looks to be consistent with that.

Note that the trigger of the bug is not a status change alone but a new message from a a contact followed by a status change of that contact.

Thanks for fixing that!
Attachment #8354699 - Flags: review?(benediktp) → review+
Whiteboard: [checkin-needed]
*** Original post on bio 2201 at 2013-10-04 22:01:04 UTC ***
Closed: 7 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.