1644667 - ContactProperties -> PreferDisplayName varies between a string number and a string boolean

Status: RESOLVED FIXED

(MailNews Core :: Address Book, defect)

Description

[Mark Banner (:standard8)]

In my profile that I've had for many years, I noticed an issue with PreferDisplayName on address book cards.

On my older cards, the value is either "1" or "0" (yes, strings not numbers), on newer cards it is "true" or "false".

Whilst I can't see any adverse affects on the Thunderbird UI code, it does confuse the Conversations add-on as that is expecting the string numbers.

I'm not sure when this regressed from the address book side. My profile has been around many years, and I see these values changing from the numbers to the booleans when I change the value on the card properties display in Thunderbird 68.

I think there's two issues here:

1. Thunderbird has a varying internal representation (this might need splitting off).
2. The address book API should be made to only return pure booleans, rather than boolean strings for these values.

Comment 1

[Geoff Lankow (:darktrojan)]

It looks like bug 474721 introduced this. (It might've been there earlier but this is what I found.)

There's also this, which at appears to be a problem at first glance: https://searchfox.org/comm-central/rev/024cb64ede43e4cfca243bf69e75c75a31d39adc/mail/base/modules/DisplayNameUtils.jsm#106

I think in the near-term at least this API will only return strings. All of the values are currently stored as strings. We could try to work out which values are which type, but you'd still end up with 0 or 1 or false or true unless we start adding special cases. I'm not totally opposed to doing that eventually but now's probably not the best time to be changing the behaviour of an established API.

Comment 2

[Mark Banner (:standard8)]

(In reply to Geoff Lankow (:darktrojan) from comment #1)

It looks like bug 474721 introduced this. (It might've been there earlier but this is what I found.)

I think that bug introduced the actual property, I don't think it introduced the bug.

There's also this, which at appears to be a problem at first glance: https://searchfox.org/comm-central/rev/024cb64ede43e4cfca243bf69e75c75a31d39adc/mail/base/modules/DisplayNameUtils.jsm#106

I think in the near-term at least this API will only return strings. All of the values are currently stored as strings. We could try to work out which values are which type, but you'd still end up with 0 or 1 or false or true unless we start adding special cases. I'm not totally opposed to doing that eventually but now's probably not the best time to be changing the behaviour of an established API.

To clarify, doing card.getProperty("PreferDisplayName", "1") in the current code is returning a pure true/false value - so that DisplayNameUtils code does actually work, if it was returning a string then it wouldn't.

There is something in the API which is making the booleans into boolean strings.

I can cope with this for Conversations, but this is definitely something that's inconsistent within Thunderbird and within the API.

Comment 3

[Geoff Lankow (:darktrojan)]

Moving this to the Address Book component since we worked out it's actually a problem there, not with the API.

Comment 4

[Geoff Lankow (:darktrojan)]

Attachment: 1644667-card-property-types-1.diff

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9155575 [details] [diff] [review]
1644667-card-property-types-1.diff

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

the -> they in the patch description.

Other than that, looks good. r=mkmelin

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/248d56f36138
When setting contact properties, convert values to string. r=mkmelin

Comment 7

[Magnus Melin [:mkmelin]]

Comment on attachment 9155575 [details] [diff] [review]
1644667-card-property-types-1.diff

Correctness fix for recent address book work.

Comment 8

[Wayne Mery (:wsmwk)]

Comment on attachment 9155575 [details] [diff] [review]
1644667-card-property-types-1.diff

Approved for beta

Comment 9

[Geoff Lankow (:darktrojan)]

Thunderbird 78.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/d9deffe5067f