1520737 - New UID does not get stored persistently on mailing list cards

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[John Bieling (:TbSync)]

User Agent: Mozilla/5.0 (iPad; CPU OS 12_1_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.0 Mobile/15E148 Safari/604.1

Steps to reproduce:

I log the new UID of mailing list cards to the console, everytime the address book observer fires onItemPropertyChanged.

onItemPropertyChanged: function addressbookListener_onItemPropertyChanged(aItem, aProperty, aOldValue, aNewValue) {
if (aItem instanceof Components.interfaces.nsIAbCard && aItem.isMailList) {
Services.console.logStringMessage("[UID] " + aItem.getProperty("UID", ""));
}
}

Actual results:

Each time the mailing list was modified, it returned a new UID.

Expected results:

UID should be persistent.

Comment 1

[John Bieling (:TbSync)]

I am on TB 60.4.

This may be related to the general issue, that properties added to mailing list cards get lost after some time. The only property that sticks (to my knowledge at least) is NickName.

If this gets confirmed it would be great if this could get fixed for all properties (including custom properties) and not just for the UID.

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1520737-uid-test-1.diff

Comment 3

[Geoff Lankow (:darktrojan)]

(In reply to john.bieling from comment #1)

If this gets confirmed it would be great if this could get fixed for all properties (including custom properties) and not just for the UID.

The directory object of a mailing list only exists in memory. It is created from the list's card object when read (I think) and not stored anywhere, thus any properties set on it disappear when it does.

Let me state, again, how much I hate the address book code.

Comment 4

[Magnus Melin [:mkmelin]]

Comment on attachment 9037451 [details] [diff] [review]
1520737-uid-test-1.diff

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

LGTM, r=mkmelin

Comment 5

[John Bieling (:TbSync)]

Do I understand this right, that this patch really only fixes the UID and not the general possibility to store properties on maillist?

Do you see a chance to add one more general-use field, which we can use to store arbitrary information, like carddav etags and other sync related information? Or must I create a new bug report for that?

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/9945bbe9adaf
Ensure UID property is persistent for mailing list cards; r=mkmelin

Comment 7

[Geoff Lankow (:darktrojan)]

John, if possible, please check I haven't missed something when the next Daily is released.

(In reply to john.bieling from comment #5)

Do I understand this right, that this patch really only fixes the UID and not the general possibility to store properties on maillist?

Do you see a chance to add one more general-use field, which we can use to store arbitrary information, like carddav etags and other sync related information? Or must I create a new bug report for that?

That'd better go in another bug. I think it should already be possible to set a property on a list's nsIAbCard rather than the its nsIAbDirectory, but I haven't checked it works.

Comment 8

[John Bieling (:TbSync)]

@General properties in maillist cards:
It does not work, properties of maillist cards get lost after some time, but I will check again and open a new bug.

@Test UID:
The new UID is not accessible via webextension ContactProperties object, is that intentionally or am I missing something? How can we access the UID from an webExtension?

Comment 9

[Geoff Lankow (:darktrojan)]

It's just the "id" property of a book/list/contact when in WebExtension land.

Comment 10

[John Bieling (:TbSync)]

@id: Thanks, that works.
What is the first daily which should include this fix? It looks like it works with 66.0a1 (2019-01-22), is that possible?

Can we have this fix backported to TB60? I need it for propper CardDav Sync with TbSync, owl will probably need it too.

Comment 11

[Geoff Lankow (:darktrojan)]

Yes, it's in 2019-01-22. I'm okay having it backported, just wanted to check I'd fixed it properly first.

Comment 12

[Geoff Lankow (:darktrojan)]

Comment on attachment 9037451 [details] [diff] [review]
1520737-uid-test-1.diff

This needs changes for ESR, so will post a new patch.

Comment 13

[Geoff Lankow (:darktrojan)]

Attachment: 1520737-uid-test-esr-1.diff

Same change, test adapted for enumeration changes.

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=706f84e3a6a913b0d86c802bd8004940bff8fb38

Comment 14

[Jorg K (CEST = GMT+2)]

TB 65 beta 4:
https://hg.mozilla.org/releases/comm-beta/rev/e8a0177c73cb217793252e6f18737818fe87f32f

Comment 15

[Jorg K (CEST = GMT+2)]

TB 60.6 ESR:
https://hg.mozilla.org/releases/comm-esr60/rev/5ed6cf41ec2cec637d21bcac9cd154ea239f1c75

Comment 16

[John Bieling (:TbSync)]

I can confirm, that the UID is now persistent.

Comment 17

[John Bieling (:TbSync)]

I tested the try build provied by Jorg here:
https://bugzilla.mozilla.org/show_bug.cgi?id=1522453#c31