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

Status: VERIFIED FIXED

(Thunderbird :: Address Book, defect, P1)

Description

[Geoff Lankow (:darktrojan)]

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.

Comment 1

[Magnus Melin [:mkmelin]]

Tests could be added/adapted in https://searchfox.org/comm-central/source/mail/components/addrbook/test/browser/browser_contact_tree.js

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1639750-ab-sort-1.diff

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.

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9158410 [details] [diff] [review]
1639750-ab-sort-1.diff

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

LGTM, r=mkmelin

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7e71e1df6485
Overhaul sorting of address book contact list. r=mkmelin

Comment 5

[Wayne Mery (:wsmwk)]

Comment on attachment 9158410 [details] [diff] [review]
1639750-ab-sort-1.diff

Approved for beta

Any simple manual tests you can recommend to testers?

Comment 6

[Wayne Mery (:wsmwk)]

(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?

Comment 7

[Rob Lemley [:rjl]]

Thunderbird 78.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/040be9666a91

Comment 8

[WaltS48 [:walts48]]

Still seeing this in testing 78.0b4.

Comment 9

[Geoff Lankow (:darktrojan)]

Attachment: 1639750-ab-sort-2.diff

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

Comment 10

[Pulsebot]

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

Comment 11

[Wayne Mery (:wsmwk)]

Comment on attachment 9159985 [details] [diff] [review]
1639750-ab-sort-2.diff

Approved for beta

Comment 12

[Rob Lemley [:rjl]]

Thunderbird 78.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/8a29fc272c72