842073 - Migrate toolkit.telemetry.prompted pref from bool to int to match Firefox.

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[:aceman]

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

In bug 807848 we made TB accept the pref as bool or int and convert the value to bool, to still be backward compatible. However, Firefox has the pref as int, to we prepared for that. With the landed patch, all TB17+ versions are compatible with old versions but also with future versions that will have the pref as int.

In this bug we switch the pref from bool to int, migrating existing profiles. But only after a transitional period of several months, probably landing at the time of Thunderbird 24.

Comment 1

[:aceman]

Attachment: patch

So this is merged patch from bug 807848. I really do not like the configure stuff, but it is the same what firefox does. It just means some files need to be preprocessed just for this. I don't know why there can't be some JS constant, that would be better.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 714838 [details] [diff] [review]
patch

So I suspect that MOZ_TELEMETRY_DISPLAY_REV was just because it was included in more than one file. Their recent code doesn't seem to be using this, maybe there's something we could pick up?

Comment 3

[Théo Chevalier]

(In reply to Mark Banner (:standard8) from comment #2)
> Comment on attachment 714838 [details] [diff] [review]
> patch
> 
> So I suspect that MOZ_TELEMETRY_DISPLAY_REV was just because it was included
> in more than one file. Their recent code doesn't seem to be using this,
> maybe there's something we could pick up?

Exactly. So you can remove it if you prefer. Note that on desktop the Telemetry prompt has been replaced by FHR notification.
FHR notification: Bug 804745
Removing telemetry notification: Bug 829881

Comment 4

[:aceman]

Thanks.
It think Tb does not have Health report so we need to keep it.

Comment 5

[:aceman]

I mean the notification and preferences options.
We can remove the configure option.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 714838 [details] [diff] [review]
patch

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

Looks fine, I'd just like to give it a quick run once we're up to date, and then we'll land it at the start of the 24 cycle.

::: configure.in
@@ +6167,5 @@
>  AC_SUBST(MAIL_PKG_SHARED)
>  
>  AC_SUBST(MOZILLA_OFFICIAL)
>  
> +AC_DEFINE_UNQUOTED(MOZ_TELEMETRY_DISPLAY_REV, 2)

Ok, so lets not add this to the configure/makefile system as I don't think we need it.

::: mail/base/jar.mn
@@ +46,5 @@
>      content/messenger/ABSearchDialog.js             (content/ABSearchDialog.js)
>      content/messenger/FilterListDialog.xul          (content/FilterListDialog.xul)
>      content/messenger/FilterListDialog.js           (content/FilterListDialog.js)
>  *   content/messenger/plugins.js                    (content/plugins.js)
> +*   content/messenger/specialTabs.js                (content/specialTabs.js)

We won't need to preprocess if we're not adding the var to configure

Comment 7

[:aceman]

Attachment: patch v2

OK, here is the updated patch.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 747012 [details] [diff] [review]
patch v2

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

Looks great, thanks for following through with this, especially as its taken a few versions :-)

Comment 9

[:aceman]

I am getting used to that :)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

This landed with bug 807848 in the commit message :(

https://hg.mozilla.org/comm-central/rev/2870d0f37de0

Comment 11

[:aceman]

The bugs are related, fix the problem in two stages, so it is not a great problem.