807848 - Account Mailbox does not appear in Folder Pane unless 'toolkit.telemetry.prompted' set to 'True'

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[Immy]

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121010144125

Steps to reproduce:

Upgraded to 16.0.2 on 4x Terminal Servers. 


Actual results:

Some end users could not see any account folders at all unless 'user_pref("toolkit.telemetry.prompted", 2);' is changed to 'user_pref("toolkit.telemetry.prompted", True);'  

The built in Error console showed: 
Timestamp: 02/11/12 11:25:24 AM
Error: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.setBoolPref]
Source File: chrome://messenger/content/specialTabs.js
Line: 784

When clicked on the error link above it showed:
 // Set pref to indicate we've shown the notification.
    prefs.setBoolPref(kTelemetryPrompted, true);

Which is odd because this is supposed to be an integer afaik.

We use a log on 'user.js' script which sets this value to '2'

See also: https://getsatisfaction.com/mozilla_messaging/topics/empty_folder_panes_in_thunderbird_16_0_deleting_foldertree_json_doesnt_fix_it




Expected results:

Display account and associated mailboxes in the folder pane.

Comment 1

[Wayne Mery (:wsmwk)]

several reports in getsatisfaction

Comment 2

[Wayne Mery (:wsmwk)]

confirming based on multiple reports. will let other sort out the how, why, etc

Comment 3

[:aceman]

Seems true, in mozilla-central it is used as an int pref, but in some tests also as bool (sets to 'true'). Maybe the semantics changed recently:

http://mxr.mozilla.org/comm-central/search?string=PREF_TELEMETRY_PROMPTED&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central

http://mxr.mozilla.org/comm-central/search?string=toolkit.telemetry.prompted&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=comm-central

I can confirm the bug, if I set toolkit.telemetry.prompted to 2 via about:config. The question is how it can naturally get that value.

Comment 4

[:aceman]

Do we need to just copy the logic used in Firefox at http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/nsBrowserGlue.js#958 ?

Comment 5

[[e-mail]]

I think you mean the logic at lines 924-934, in which case, yes.

Comment 6

[:aceman]

But it seems TB does not have the 2nd version labels. As the pref has no default defined in toolkit, it seems it is fine for TB to specify it as bool.

(In reply to Cj from comment #0)
> Which is odd because this is supposed to be an integer afaik.
> 
> We use a log on 'user.js' script which sets this value to '2'

It is an integer in Firefox, but a bool in TB. Why do you set it to 2?

Comment 7

[Immy]

(In reply to :aceman from comment #6)
> But it seems TB does not have the 2nd version labels. As the pref has no
> default defined in toolkit, it seems it is fine for TB to specify it as bool.
> 
> (In reply to Cj from comment #0)
> > Which is odd because this is supposed to be an integer afaik.
> > 
> > We use a log on 'user.js' script which sets this value to '2'
> 
> It is an integer in Firefox, but a bool in TB. Why do you set it to 2?

It was set to '2' historically (I didn't do the coding), maybe based on shared usage between Firefox & TB. See https://bugzilla.mozilla.org/show_bug.cgi?id=697860 It used to work fine until 16.x.x In testing I've found that changing it to 'True' is user.js doesn't overwrite it prefs.js; it's effect seems to be random, only some users seem to affected, including a new user I set up.

Comment 8

[:aceman]

But Firefox and Thunderbird never share a profile. But as the pref name is the same I also this we should use it the same (at least the type should be the same).

This needs decision what we want to do here. If we just convert it to int or we also want all the new features as in https://bugzilla.mozilla.org/show_bug.cgi?id=697860#c15 .

Comment 9

[Mark Banner (:standard8)]

@Cj: Thunderbird has never set the preference to '2'. When Firefox first introduced telemetry it was a boolean for them. Sometime in the last year (iirc) they changed it to an integer.

@aceman: I think the easy short-term improvement would be to try/catch on that preference get. The slower improvement would be to pick up the rest of the changes they've done for the telemetry prompts.

Comment 10

[Immy]

(In reply to Mark Banner (:standard8) from comment #9)
> @Cj: Thunderbird has never set the preference to '2'. When Firefox first
> introduced telemetry it was a boolean for them. Sometime in the last year
> (iirc) they changed it to an integer.

Noted. Our user.js script is used for both TB & FF (although they could be split) but it has now been changed to 'True'. The odd thing is why then does it only affect some users, and what is going on with the other reports of this error that are not in a enterprise environment?

Comment 11

[:aceman]

I wonder if this doesn't cause problems in Seamonkey. They may share code that handles the pref as int (from FF) and code that handles it as bool (from mailnews).

Standard8, do we also care about showing the new string (do we actually have a new version) or should we just silently bump all profiles to the value 2?

Comment 12

[Mark Banner (:standard8)]

(In reply to :aceman from comment #11)
> Standard8, do we also care about showing the new string (do we actually have
> a new version) or should we just silently bump all profiles to the value 2?

I don't see any harm in adding the new functionality. I believe its just trying to get an actionable result.

Comment 13

[:aceman]

Attachment: patch

This should do this:
if getting toolkit.telemetry.prompted throws, or the returned int is lower than the current prompt revision, show telemetry prompt. After the prompt is closed, toolkit.telemetry.prompted is set to the current revision. Then
1. if it was dismissed via the close button, nothing else is set.
2. if Yes was clicked, toolkit.telemetry.enabled is set to true.
3. if No was clicked, toolkit.telemetry.rejected is set to true. (And toolkit.telemetry.enabled is left at the default of false. I added it to all-thunderbird.js as firefox also has a default for it)

This seems to match the FF implementation in bug 697860 comment 19.

Comment 14

[:aceman]

This is not a problem in Seamonkey because they probably use prompt on the browser part and there is no reference to the pref in /mailnews and the /suite specific code.

Comment 15

[:aceman]

If we also want to hide the close button as FF bug 697860 does, just tell me.

Comment 16

[Théo Chevalier]

Hi, we recently landed some changes to this prompt in Firefox in bug 699806, and dependent bugs.

You need to update toolkit.telemetry.rejected and toolkit.telemetry.prompted on the pref pane, too. (See bug 737600 for what we do for Firefox)

Having a toolkit.telemetry.rejected pref that is not synced with current toolkit.telemetry.enabled state caused us a lot of troubles in bug 699806 (see the thread...).

If later we want to easily make telemetry opt-out, and, more important respect user privacy (because of that, unfortunately, there is some edge cases we couldn't handle: we couldn't say if the user made a choice or if he was in the default case.).

>user_pref("toolkit.telemetry.prompted", 999);
Also, why are you doing that? (Actually, I don't understand what's the problem with "2")
We still want to be able to reprompt already opted-in users if our policy changes, even on TB.

Sid, from a privacy point of view, am I correct?


(This is not the aim of this bug, but maybe you could be interested for TB about enabling telemetry by default for Daily/Earlybird, I can backport my patches if you want. Just tell me)

Comment 17

[Sid Stamm [:geekboy or :sstamm]]

(In reply to Théo Chevalier [:tchevalier] from comment #16)
> We still want to be able to reprompt already opted-in users if our policy
> changes, even on TB.
> 
> Sid, from a privacy point of view, am I correct?

I'm not sure I follow the rest of the bug, but if our policy changes (and we prompted the first time) we should get consent again, yes.

Comment 18

[Matt]

(In reply to Théo Chevalier [:tchevalier] from comment #16)

> 
> (This is not the aim of this bug, but maybe you could be interested for TB
> about enabling telemetry by default for Daily/Earlybird, I can backport my
> patches if you want. Just tell me)

Protz, following the discussion on TBPlanning, is there anything to add here?

Comment 19

[:aceman]

(In reply to Théo Chevalier [:tchevalier] from comment #16)
> Hi, we recently landed some changes to this prompt in Firefox in bug 699806,
> and dependent bugs.
> 
> You need to update toolkit.telemetry.rejected and toolkit.telemetry.prompted
> on the pref pane, too. (See bug 737600 for what we do for Firefox)
Thanks, I will look at those.

> >user_pref("toolkit.telemetry.prompted", 999);
> Also, why are you doing that? (Actually, I don't understand what's the
> problem with "2")
> We still want to be able to reprompt already opted-in users if our policy
> changes, even on TB.
No, this 999 value is set only in tests. We do not want the prompts there so I gave high value to skip any revisions.

Comment 20

[Martin Sapsed]

(In reply to Mark Banner (:standard8) from comment #9)
> @Cj: Thunderbird has never set the preference to '2'. When Firefox first
> introduced telemetry it was a boolean for them. Sometime in the last year
> (iirc) they changed it to an integer.

Sorry to drag this up again. Looking once more at moving from 10.x ESR to 17 ESR and came across the problem I had the first time I tried 17 - no folder view. I too had the telemetry pref set to 2. It seems quite an arrogant statement to say "Thunderbird has never set the preference to '2'" when so many people have that value and almost certainly didn't set it to that by hand! There *must* have been some version of Thunderbird, perhaps a long time ago, that set this pref to 2 and the developers ought to accept that and deal with it.

Comment 21

[:aceman]

Martin, we have a patch ready, standard8 just has to review it.

Comment 22

[Mark Banner (:standard8)]

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

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

Sorry for the delay in this, looks good.

::: mail/app/profile/all-thunderbird.js
@@ +361,5 @@
>  
>  pref("view_source.syntax_highlight", false);
>  
>  pref("toolkit.telemetry.infoURL", "http://www.mozilla.org/thunderbird/legal/privacy/#telemetry");
> +pref("toolkit.telemetry.enabled", false);

This is already defined in all.js so we don't need to define it here.

Comment 23

[Mark Banner (:standard8)]

I just noticed, this breaks the downgrade situation:

WARNING: Trying to set pref toolkit.telemetry.prompted to with the wrong type!: file /Users/moztest/comm/main/src/mozilla/modules/libpref/src/prefapi.cpp, line 754
JavaScript error: chrome://messenger/content/specialTabs.js, line 779: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.setBoolPref]


Could we land a patch on our existing branches to cope with that situation do you think?

Comment 24

[:aceman]

Can we land the patch as is on the branches (17+) ? This is reported starting at 16. So migrating 17 <-> 21 in any direction will work?
Or where do we want to downgrade?

Comment 25

[:aceman]

Attachment: patch v2

Comment 26

[Ryan VanderMeulen [:RyanVM]]

This doesn't apply cleanly to comm-central.

Comment 27

[:aceman]

Yeah, Services conversions were faster :) Update coming in a moment.

Comment 28

[:aceman]

Attachment: patch v3

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/d01165786209

Leaving open per the whiteboard (though I'll point out that we typically close bugs when they hit trunk and use the status flags for tracking uplifts).

Comment 30

[:aceman]

It seems I forgot to make the necessary changes to the preferences pane :( Do we back this out?
https://bug737600.bugzilla.mozilla.org/attachment.cgi?id=690021

standard8, do we inherit MOZ_TELEMETRY_DISPLAY_REV from configure.in (in that patch) ro do we need/want to port it to ours?

Comment 31

[:aceman]

Attachment: patch - preferences

Comment 32

[Joe Sabash [:JoeS1]]

Since the last checkin here I get a blank folder pane when reverting to a 2 day old build with the same profile. Regression?

Comment 33

[:aceman]

Yes, that is unfortunate side effect that standard8 talks about in comment 23.
You can't safely downgrade in nightlies. But when we push it to all branches, you will be able to migrate between the newest versions on each branch (e.g. 17.0.3, beta after the date of checkin, earlybird after the date of checkin, nightly after 2013-02-08). So all supported versions.

Comment 34

[Mark Banner (:standard8)]

Ok, I think there are several options we have here:

1) Back out the patch, land a patch that "copes" with either boolean/int value on all branches. Then when we hit say Gecko 24, re-land this patch for the next ESR.

2) Use a different preference name

3) Land this patch everywhere and ignore the backwards compatibility issue.


I'm currently swaying towards option 1 which would give us at least some overlap for backwards compatibility.

I think 2 would be confusing as the toolkit pref is specified in all.js, so it would appear anyway, and it would be another pref different which isn't good for enterprises.

Option 3 makes it harder for regression testing, although I guess setting the value to True in user.js might work around it (though they would have to know they need to do that, which makes it less attractive as an option).

aceman, thoughts?

Comment 35

[:aceman]

(In reply to Mark Banner (:standard8) from comment #34)
> Ok, I think there are several options we have here:
> 
> 1) Back out the patch, land a patch that "copes" with either boolean/int
> value on all branches. Then when we hit say Gecko 24, re-land this patch for
> the next ESR.

The existing patch copes with bool/int if we push it to all branches. My question about backout was doe to the missing changes in preferences dialog. But I have already uploaded it so if you review it, we can land it too and all is OK.

Comment 36

[:aceman]

(In reply to Mark Banner (:standard8) from comment #34)
> Ok, I think there are several options we have here:
> 
> 1) Back out the patch, land a patch that "copes" with either boolean/int
> value on all branches. Then when we hit say Gecko 24, re-land this patch for
> the next ESR.
> 
> 2) Use a different preference name
This would not solve any problem. As diverging just the value (not even the pref name) caused this bug in the first place. The docs probably say to set it to 2 for Firefox and users expected that to work on TB too.

> 3) Land this patch everywhere and ignore the backwards compatibility issue.

I am not sure what is the difference in patch for 3) (the current one) and for 1). I think we agreed that the current patch is compatible enough (all newest versions on all branches of 17+) after we check it into all branches. So what is different in option 1)?

Comment 37

[Mark Banner (:standard8)]

The difference in 1, is that we have about four-six months with no pref value change (or UI etc). When we then do the change later on, there's then those four-six months worth of builds that will cope with testers (and potentially users) switch back and forth.

If we did option three, then we don't get a grace period of allowing users to go back again.

Comment 38

[Wayne Mery (:wsmwk)]

me being one who bounces around randomly between releases and builds, I think I favor choice #1

Comment 39

[:aceman]

OK, so you propose a patch that accepts bool AND int and converts it to bool (so that older nightlies in the current era still work)? That one gets in all branches. Then later we check-in the real patch into all branches, that accepts bool AND int but converts it to int?

Comment 40

[:aceman]

Attachment: patch for transitional period

Comment 41

[:aceman]

If I understand it correctly, the idea is to backout patch v3 and immediately check in the transitional patch.

Comment 42

[:aceman]

Comment on attachment 711959 [details] [diff] [review]
patch - preferences

So this one can wait for the backout and then merged with patch v3.

Comment 43

[Mark Banner (:standard8)]

Comment on attachment 714080 [details] [diff] [review]
patch for transitional period

Ok, I've just tested it and it looks good. I'll land it in a moment with the backout from trunk.

Going to land it on aurora and esr17 as well as agreed.

Comment 44

[Mark Banner (:standard8)]

Backout: https://hg.mozilla.org/comm-central/rev/0879e29b38b9
Landing: https://hg.mozilla.org/comm-central/rev/61f22c6eaf0f

Comment 45

[Mark Banner (:standard8)]

Branch landings:

https://hg.mozilla.org/releases/comm-aurora/rev/ec4828fcca96
https://hg.mozilla.org/releases/comm-esr17/rev/66f7060807f0 (with minor bitrot fix)

aceman: I'd like to track this for landing in 24, but with the branch landings for this patch that is difficult, would you mind moving the combined patch to a new bug, and we'll track it there?

Comment 46

[:aceman]

Sure, no problem.

Comment 47

[Ryan VanderMeulen [:RyanVM]]

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

Comment 48

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ryan VanderMeulen [:RyanVM][UTC-4] from comment #47)
> https://hg.mozilla.org/comm-central/rev/2870d0f37de0

This was actually bug 842073, sorry.

Comment 49

[Derek Roberts]

Still happening for me in Thunderbird 24.2, driving me mad

No mailboxes appear unless I open prefs.js, where I find

user_pref("toolkit.telemetry.prompted", 2);

And change it to 

user_pref("toolkit.telemetry.prompted", false);

Whereupon after restart the telemetry prompt appears _again_ 

Even though this is set

user_pref("toolkit.telemetry.rejected", true);

And next time I exit thunderbird it's set back to:

user_pref("toolkit.telemetry.prompted", 2);

And my Accounts aren't visible any more.

How cam I KILL TELEMETRY for EVER ?

Comment 50

[:aceman]

user_pref("toolkit.telemetry.prompted", 2) is the correct value for TB24 (see bug 842073). Please open a new bug if it does not work for you and post contents of Tools -> Error console there.