1534175 - If a mailing list member's address is removed, the list can't be added to

Status: RESOLVED WONTFIX

(MailNews Core :: Address Book, enhancement)

Description

[Geoff Lankow (:darktrojan)]

+++ This bug was initially created as a clone of Bug #1522453 +++

Make a mailing list
Add members with addresses
Remove a member's address
Add more members … fail.

Comment 1

[Geoff Lankow (:darktrojan)]

This works without the UI calling editMailListToDatabase, which counts the number of list members with email addresses and saves that number as the total count.

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1534175-list-length-1.diff

This is a counting error. The code doesn't prevent the presence of an email-less card, but it doesn't count it, which means a card is not seen by anything that enumerates the list. (Except the list's addressLists property, which is an in-memory array.)

Comment 3

[Magnus Melin [:mkmelin]]

Comment on attachment 9049863 [details] [diff] [review]
1534175-list-length-1.diff

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

r=mkmelin

::: mailnews/addrbook/test/unit/test_bug1534175.js
@@ +1,1 @@
> +function run_test() {

please add a short comment about what this test is about, so people do not need to go to the bug report

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/0020655906af
Store accurate length of an address list, if a member has no address; r=mkmelin

Comment 5

[Geoff Lankow (:darktrojan)]

Comment on attachment 9049863 [details] [diff] [review]
1534175-list-length-1.diff

I guess we want this on ESR.

Comment 6

[Jorg K (CEST = GMT+2)]

How soon? Ignore all precaution and ship it in TB 60.6? Or have it on a beta first?

Comment 7

[John Bieling (:TbSync)]

Can you put it into a candidate, like last time? I have time to check today.

Comment 8

[Jorg K (CEST = GMT+2)]

No, try builds of ESR currently don't work, bug 1534204. You'd have to try it on a Daily of today which is also pretty broken, bug 1533085. Maybe the UI you need still works.

I'm asking about the risk here. I've been criticised for running an aggressive uplift policy, and it's already bitten us twice. One bite was in the context of the address book, where allocating unique IDs made address auto-completion in the compose window's addressing widget very slow for some users with large address books. So although the bug might be fixed, some other issues might have been introduced. One never knows.

Comment 9

[John Bieling (:TbSync)]

You do not need to push this fast "just for me". I have mitigation in place for this bug in my AddOn and everybody else had to cope with this bug for years, probably no one noticed.

Comment 10

[John Bieling (:TbSync)]

I played around with daily (67.0a1 (2019-03-11) (64-bit)) and it looks like this specific bug is fixed, but there is another counting error, which i will describe in the main bug.

Comment 11

[Geoff Lankow (:darktrojan)]

I'd rather have this bug than the problems it caused. Backed out for now.

Comment 12

[Jorg K (CEST = GMT+2)]

Can bug 1534596 and bug 1534598 be closed by saying: "Fixed by backout of bug 1534175"?

Comment 13

[Geoff Lankow (:darktrojan)]

Seems reasonable.

Comment 14

[Geoff Lankow (:darktrojan)]

Upon further investigation, I've decided to leave this bug unfixed. The rabbit hole goes too deep.

As I've said elsewhere I'm not opposed to arbitrary collections of contacts (with or without email addresses), but the address list feature isn't up to it. Somewhere post-68 the address book code needs some major work and I hope we can then add some useful features like tags.