Last Comment Bug 842073 - Migrate toolkit.telemetry.prompted pref from bool to int to match Firefox.
: Migrate toolkit.telemetry.prompted pref from bool to int to match Firefox.
Status: RESOLVED FIXED
[gs]
:
Product: Thunderbird
Classification: Client Software
Component: Folder and Message Lists (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 24.0
Assigned To: :aceman
:
Mentors:
https://getsatisfaction.com/mozilla_m...
: 1043432 (view as bug list)
Depends on: 807848
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-16 13:08 PST by :aceman
Modified: 2015-08-06 08:22 PDT (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
+
fixed
-


Attachments
patch (12.74 KB, patch)
2013-02-16 14:30 PST, :aceman
standard8: feedback+
Details | Diff | Review
patch v2 (9.62 KB, patch)
2013-05-08 10:06 PDT, :aceman
standard8: review+
Details | Diff | Review

Description :aceman 2013-02-16 13:08:12 PST
+++ 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 2013-02-16 14:30:41 PST
Created attachment 714838 [details] [diff] [review]
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) 2013-04-10 02:11:39 PDT
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 [:tchevalier] 2013-04-10 03:12:31 PDT
(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 2013-04-10 03:20:35 PDT
Thanks.
It think Tb does not have Health report so we need to keep it.
Comment 5 :aceman 2013-04-10 03:21:49 PDT
I mean the notification and preferences options.
We can remove the configure option.
Comment 6 Mark Banner (:standard8) 2013-05-08 02:34:23 PDT
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 2013-05-08 10:06:32 PDT
Created attachment 747012 [details] [diff] [review]
patch v2

OK, here is the updated patch.
Comment 8 Mark Banner (:standard8) 2013-06-10 12:26:12 PDT
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 2013-06-10 12:50:38 PDT
I am getting used to that :)
Comment 10 Ryan VanderMeulen [:RyanVM] 2013-06-11 05:08:03 PDT
This landed with bug 807848 in the commit message :(

https://hg.mozilla.org/comm-central/rev/2870d0f37de0
Comment 11 :aceman 2013-06-11 05:20:28 PDT
The bugs are related, fix the problem in two stages, so it is not a great problem.
Comment 12 Mike Kaply [:mkaply] (Out June 27-July 5) 2015-08-06 08:22:37 PDT
*** Bug 1043432 has been marked as a duplicate of this bug. ***

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