Closed Bug 1639750 Opened 3 months ago Closed 2 months ago

New address book contacts added to end of list, not in correct sort order

Categories

(Thunderbird :: Address Book, defect, P1)

Tracking

(thunderbird78 fixed)

VERIFIED FIXED
Thunderbird 79.0
Tracking Status
thunderbird78 --- fixed

People

(Reporter: darktrojan, Assigned: darktrojan)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

In the address book window, if a new contact is added to the displayed directory, it is added to the end of the list, instead of wherever in the list it should be based on the current sort order.

Theres two places we add cards to the list that need fixing. The comparator we use will need to be extracted for this purpose. Bonus points if finding the right place can be done by bisection rather than comparing every row.

Keywords: regression
Assignee: nobody → lasana
Status: NEW → ASSIGNED
Assignee: lasana → geoff
Priority: P2 → P1

There's much more here that needed fixing than I first thought. I've cached the cell texts for better performance (and cleared the cache when notified of an updated contact), switched to using Intl.Collator instead of simple string comparison, and added more testing. Arguably there's much more that needs testing, like sorting on different columns, but I have some confidence that that works.

Attachment #9158410 - Flags: review?(mkmelin+mozilla)
Attachment #9158410 - Flags: approval-comm-beta?
Comment on attachment 9158410 [details] [diff] [review]
1639750-ab-sort-1.diff

Review of attachment 9158410 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, r=mkmelin
Attachment #9158410 - Flags: review?(mkmelin+mozilla) → review+
Target Milestone: --- → Thunderbird 79.0

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/7e71e1df6485
Overhaul sorting of address book contact list. r=mkmelin

Status: ASSIGNED → RESOLVED
Closed: 2 months ago
Resolution: --- → FIXED
Comment on attachment 9158410 [details] [diff] [review]
1639750-ab-sort-1.diff

Approved for beta

Any simple manual tests you can recommend to testers?
Flags: needinfo?(geoff)
Attachment #9158410 - Flags: approval-comm-beta? → approval-comm-beta+

(In reply to Wayne Mery (:wsmwk) from comment #5)

Any simple manual tests you can recommend to testers?

Never mind, it's in comment 0

Arguably there's much more that needs testing, like sorting on different columns, but I have some confidence that that works.

Worthy of follow up bug?

Still seeing this in testing 78.0b4.

Wow. I really screwed that up. What the hell was I thinking?

Flags: needinfo?(geoff)
Attachment #9159985 - Flags: review?(mkmelin+mozilla)
Attachment #9159985 - Flags: approval-comm-beta?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #9159985 - Flags: review?(mkmelin+mozilla) → review+

Pushed by mkmelin@iki.fi:
https://hg.mozilla.org/comm-central/rev/2e8aebfa1995
Fix sorting of address book contact list (attempt 2). r=mkmelin

Status: REOPENED → RESOLVED
Closed: 2 months ago2 months ago
Resolution: --- → FIXED
Comment on attachment 9159985 [details] [diff] [review]
1639750-ab-sort-2.diff

Approved for beta
Attachment #9159985 - Flags: approval-comm-beta? → approval-comm-beta+
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.