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

RESOLVED FIXED in Thunderbird 24.0

Status

Thunderbird
Folder and Message Lists
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: aceman, Assigned: aceman)

Tracking

Trunk
Thunderbird 24.0

Thunderbird Tracking Flags

(thunderbird24+ fixed, thunderbird-esr17-)

Details

(Whiteboard: [gs], URL)

Attachments

(1 attachment, 1 obsolete attachment)

9.62 KB, patch
standard8
: review+
Details | Diff | Splinter Review
(Assignee)

Description

5 years ago
+++ 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.
(Assignee)

Comment 1

5 years ago
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.
Attachment #714838 - Flags: review?(mbanner)
tracking-thunderbird22: --- → +
tracking-thunderbird-esr17: ? → -
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?
(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
(Assignee)

Comment 4

4 years ago
Thanks.
It think Tb does not have Health report so we need to keep it.
(Assignee)

Comment 5

4 years ago
I mean the notification and preferences options.
We can remove the configure option.
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
Attachment #714838 - Flags: review?(mbanner) → feedback+
(Assignee)

Comment 7

4 years ago
Created attachment 747012 [details] [diff] [review]
patch v2

OK, here is the updated patch.
Attachment #714838 - Attachment is obsolete: true
Attachment #747012 - Flags: review?(mbanner)
tracking-thunderbird22: + → ---
tracking-thunderbird23: --- → +
tracking-thunderbird23: + → ---
tracking-thunderbird24: --- → +
Whiteboard: [gs][only check in at the time of TB24] → [gs]
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 :-)
Attachment #747012 - Flags: review?(mbanner) → review+
(Assignee)

Comment 9

4 years ago
I am getting used to that :)
Keywords: checkin-needed
This landed with bug 807848 in the commit message :(

https://hg.mozilla.org/comm-central/rev/2870d0f37de0
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-thunderbird24: --- → fixed
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 24.0
(Assignee)

Comment 11

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

Updated

2 years ago
Duplicate of this bug: 1043432
You need to log in before you can comment on or make changes to this bug.