New UID does not get stored persistently on mailing list cards

RESOLVED FIXED in Thunderbird 66.0

Status

defect
RESOLVED FIXED
4 months ago
2 months ago

People

(Reporter: john.bieling, Assigned: darktrojan)

Tracking

Thunderbird 66.0

Thunderbird Tracking Flags

(thunderbird_esr6066+ fixed, thunderbird65 fixed, thunderbird66 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

4 months ago

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.

Reporter

Comment 1

4 months ago

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.

Assignee

Comment 2

4 months ago
Assignee: nobody → geoff
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #9037451 - Flags: review?(mkmelin+mozilla)
Assignee

Comment 3

4 months ago

(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 on attachment 9037451 [details] [diff] [review]
1520737-uid-test-1.diff

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

LGTM, r=mkmelin
Attachment #9037451 - Flags: review?(mkmelin+mozilla) → review+
Assignee

Updated

4 months ago
Keywords: checkin-needed
Reporter

Comment 5

4 months ago

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

4 months ago

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/9945bbe9adaf
Ensure UID property is persistent for mailing list cards; r=mkmelin

Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
Assignee

Updated

4 months ago
Target Milestone: --- → Thunderbird 66.0
Assignee

Comment 7

4 months ago

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.

Reporter

Comment 8

4 months ago

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

Assignee

Comment 9

4 months ago

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

Reporter

Comment 10

4 months ago

@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.

Assignee

Comment 11

4 months ago

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

Assignee

Comment 12

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

This needs changes for ESR, so will post a new patch.
Attachment #9037451 - Flags: approval-comm-beta?

Updated

4 months ago
Attachment #9037451 - Flags: approval-comm-beta? → approval-comm-beta+

Updated

2 months ago
Attachment #9038394 - Flags: approval-comm-esr60? → approval-comm-esr60+
Reporter

Comment 16

2 months ago

I can confirm, that the UID is now persistent.

Reporter

Comment 17

2 months ago

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

You need to log in before you can comment on or make changes to this bug.