As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
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 | Splinter Review
patch v2 (9.62 KB, patch)
2013-05-08 10:06 PDT, :aceman
standard8: review+
Details | Diff | Splinter Review

Description User image :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 User image :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 User image 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 User image 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 User image :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 User image :aceman 2013-04-10 03:21:49 PDT
I mean the notification and preferences options.
We can remove the configure option.
Comment 6 User image 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 User image :aceman 2013-05-08 10:06:32 PDT
Created attachment 747012 [details] [diff] [review]
patch v2

OK, here is the updated patch.
Comment 8 User image 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 User image :aceman 2013-06-10 12:50:38 PDT
I am getting used to that :)
Comment 10 User image 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 User image :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 User image Mike Kaply [:mkaply] 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.