Bug 1615981 reported the telemetry wrong for IM accounts, it used the "name" field which is human readable, not the "normalizedName" which is not.

You can see this on https://stats-stage.thunderbird.net/#telemetry where there are bogus entries:

• im_твиттер
• im_google talk

Andrei / Philipp -- do you have any thoughts on if we should try to clean-up the old data? This essentially leaks user's localizations too, which I think is already reported via telemetry anyway so is probably not a large deal.

(In reply to Patrick Cloke [:clokep] from comment #3)

Andrei / Philipp -- do you have any thoughts on if we should try to clean-up the old data? This essentially leaks user's localizations too, which I think is already reported via telemetry anyway so is probably not a large deal.

Nah, it's not sensitive data and we already have anything this might reveal.

The patch is good and will clean this data up a lot. Thanks.

Also, we should definitely get this into 78, because clients without the fix will report the old names, and 78 clients will need to be included in Telemetry queries until 91 reaches >80% usage or the data will be inaccurate.

The patch doesn't update the default set of 0 values in the probe, even though some names change (like im_xmpp -> im_jabber) from what I can tell. I also think this will lead to yahoo getting reported as im_jabber since it doesn't set its normalized name.

(In reply to Martin Giger [:freaktechnik] from comment #7)

The patch doesn't update the default set of 0 values in the probe, even though some names change (like im_xmpp -> im_jabber) from what I can tell. I also think this will lead to yahoo getting reported as im_jabber since it doesn't set its normalized name.

Yeah I just realized this. :(

Note that jsProtoHelper sets the normalizedName automatically based on the name if one is not given: https://searchfox.org/comm-central/rev/9162fde155e18e51ff6668916ecf4f0ce188315a/chat/modules/jsProtoHelper.jsm#1129-1133

I think we should clean all this up in the future to not have both a normalizedName and an id, but not 100% sure how to do that at the moment.

(In reply to Patrick Cloke [:clokep] from comment #8)

Note that jsProtoHelper sets the normalizedName automatically based on the name if one is not given: https://searchfox.org/comm-central/rev/9162fde155e18e51ff6668916ecf4f0ce188315a/chat/modules/jsProtoHelper.jsm#1129-1133

Oh right, and the yahoo account doesn't inherit from XMPP, so it'll work correctly. I'm not even sure how I thought it would inherit from XMPP, since the protocol is disabled (like twitter and facebook).

This has a few additional changes:

• It manually sets the normalized name for all protocols (IRC and Yahoo depended on the auto-generated ones).
• Fixes the default account types in the probes.
• Reports something for unknown protocols.
• Updates comments more.

(In reply to Martin Giger [:freaktechnik] from comment #12)

::: chat/components/src/imAccounts.jsm
@@ +101,5 @@

 return "";

},
get normalizedName() {

• // Use the ID, but remove the 'prpl-' prefix.
• return this.id.replace(/^prpl-/, "");

This makes me wonder, did we ever offer to delete log storage of unknown
accounts, because if we did that must not have worked then.

I think if you delete an account that is an unknown protocol it will attempt to delete them. I think that right now it will cause it to look for a bogus path that will not be deleted.

::: chat/modules/jsProtoHelper.jsm
@@ -1138,5 @@

},

• // This is more aggressive than the account normalization of just
• // toLowerCase() since prpl names must be only letters/numbers.
• get normalizedName() {
• return this.name.replace(/[^a-z0-9]/gi, "").toLowerCase();

Should this instead throw a not implemented exception, or do we not do that
for getters? (I only saw it being used for methods)

We could if you want, but I don't think it is necessary. FWIW If you fail to implement this in your protocol than your protocol will fail to load properly.

Fixes the eslint issue (and updates the commit entry).

Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/c285e183b52b
Fix reported account type for im accounts. r=freaktechnik

Comment on attachment 9212868 [details] [diff] [review]
Patch v2.1

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Clients are currently reporting incorrect telemetry for chat accounts. Thunderbird 78 users will need to be in telemetry for a while (even for a while after TB 91 is released).
• User impact if declined: None
• Fix Landed on Version: 89
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The patch slightly changes some of the registration code of chat accounts, this should break immediately if it is not working and was tested on trunk.

The result of it breaking is that chat accounts will not register properly and will be unable to connect.

• String or UUID changes made by this patch:

Comment on attachment 9212868 [details] [diff] [review]
Patch v2.1

This is not the ESR approval flag you're looking for :P

Oops! Thank you very much RyanVM!

(In reply to Ryan VanderMeulen [:RyanVM] from comment #17)

Comment on attachment 9212868 [details] [diff] [review]
Patch v2.1

patch not applicable to 88 beta?

Comment on attachment 9212868 [details] [diff] [review]
Patch v2.1

Yeah this would make sense to uplift to beta!

[Approval Request Comment]
See comment 16

Comment on attachment 9212868 [details] [diff] [review]
Patch v2.1

[Triage Comment]
Approved for beta

Thunderbird 88.0b3:
https://hg.mozilla.org/releases/comm-beta/rev/c997c8985823

Comment on attachment 9212868 [details] [diff] [review]
Patch v2.1

[Triage Comment]
Approved for esr78

Thunderbird 78.10.0:
https://hg.mozilla.org/releases/comm-esr78/rev/d99b3cd90236