Last Comment Bug 697860 - Hide close button in the telemetry notification, and track explicit clicks on "no"
: Hide close button in the telemetry notification, and track explicit clicks on...
Status: VERIFIED FIXED
[qa!]
: verified-beta
Product: Toolkit
Classification: Components
Component: Telemetry (show other bugs)
: unspecified
: x86 Windows 7
: -- normal (vote)
: mozilla10
Assigned To: (dormant account)
:
: Georg Fritzsche [:gfritzsche]
Mentors:
: 705055 (view as bug list)
Depends on: 691951
Blocks: 659396 702281 710589
  Show dependency treegraph
 
Reported: 2011-10-27 15:34 PDT by (dormant account)
Modified: 2013-12-27 14:30 PST (History)
12 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified


Attachments
store prompt response (3.09 KB, patch)
2011-10-28 15:50 PDT, (dormant account)
gavin.sharp: review-
Details | Diff | Splinter Review
v2 (1.79 KB, patch)
2011-11-03 10:27 PDT, (dormant account)
gavin.sharp: review+
lmandel: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description (dormant account) 2011-10-27 15:34:34 PDT
We should track the difference between clicking "no" and "x" in telemetry prompt.
Comment 1 (dormant account) 2011-10-28 15:50:00 PDT
Created attachment 570397 [details] [diff] [review]
store prompt response
Comment 2 (dormant account) 2011-10-28 15:53:01 PDT
Comment on attachment 570397 [details] [diff] [review]
store prompt response

This will reprompt users with old prompt and record their answer to telemetry. Unfortunately nightly users will be reprompted again because of change how the pref is used(such is the cost of being a nightly user).
Comment 3 (dormant account) 2011-10-28 15:54:30 PDT
This patch will use telemetry.prompted to store a tristate to indicate whether the users chose(Yes/No/[x]) where clicking [x] == ignoring the prompt.
Comment 4 (dormant account) 2011-11-02 11:10:56 PDT
ping gavin?
Comment 5 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-11-02 16:10:58 PDT
Comment on attachment 570397 [details] [diff] [review]
store prompt response

Let's leave the TELEMETRY_PROMPT_REV stuff as-is and go with a new telemetry.denied pref as discussed on IRC.
Comment 6 (dormant account) 2011-11-03 10:27:18 PDT
Created attachment 571698 [details] [diff] [review]
v2
Comment 8 Marco Bonardo [::mak] 2011-11-05 02:54:19 PDT
https://hg.mozilla.org/mozilla-central/rev/1592d3bec85b
Comment 10 (dormant account) 2011-11-28 15:14:37 PST
*** Bug 705055 has been marked as a duplicate of this bug. ***
Comment 11 al_9x 2011-11-28 15:38:42 PST
(In reply to Taras Glek (:taras) from comment #6)
> Created attachment 571698 [details] [diff] [review] [diff] [details] [review]
> v2

shouldn't PREF_TELEMETRY_REJECTED be checked, to return and avoid the prompt?
Comment 12 (dormant account) 2011-11-28 15:41:19 PST
(In reply to al_9x from comment #11)
> (In reply to Taras Glek (:taras) from comment #6)
> > Created attachment 571698 [details] [diff] [review] [diff] [details] [review] [diff] [details] [review]
> > v2
> 
> shouldn't PREF_TELEMETRY_REJECTED be checked, to return and avoid the prompt?

It will be if we ever add code to reprompt(ie bump telemetry prompt version). Atm there is no such code, thus no check. Previous prompting code did not indicate if the user clicked no, so we had to reprompt.
Comment 13 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-01 14:47:28 PST
QA+ for verification using comment 3.
Comment 14 Virgil Dicu [:virgil] [QA] 2011-12-07 02:47:15 PST
Mozilla/5.0 (Windows NT 6.1; rv:11.0a1) Gecko/20111205 Firefox/11.0a1

The first notification prompt received with a new profile doesn't have a tri-state unless selecting "Learn more". Is this expected?

When selecting "NO" the toolkit.telemetry.rejected pref is set to true in about:config. With "yes" and "x" the pref is not displayed.

Should the notification ever re-prompt if ignoring it with "x"?

Would appreciate any feedback in order to verify this. Thanks.
Comment 15 Lawrence Mandel [:lmandel] (use needinfo) 2011-12-07 07:10:11 PST
The tri-state should function as follows:

Click yes
- the user opts in to Telemetry
- toolkit.telemetry.enabled=true
- no further prompting
Click no
- the user opts out of Telemetry
- toolkit.telemetry.enabled=false
- toolkit.telemetry.rejected=true (?)
- no further prompting
Click x
- the user has not made a Telemetry choice (neither yes nor no)
- toolkit.telemetry.prompted= (?)
- further prompting (at some interval) until the user makes a choice

Taras - Can you help me fill in the (?)?
Comment 16 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-12-08 10:36:05 PST
I'm re-summarizing this bug to cover what was actually done. The patch here disables the close button on the notification completely.

The expected behavior is:

The notification having been displayed sets toolkit.telemetry.prompted to true.
- User clicking "yes" sets toolkit.telemetry.enabled to true
- User clicking "no" sets toolkit.telemetry.rejected to true
Comment 17 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-12-08 10:37:51 PST
(In reply to Gavin Sharp from comment #16)
> The notification having been displayed sets toolkit.telemetry.prompted to
> true.

This isn't quite accurate: it sets .prompted to the current "prompt rev" (currently 2). If the .prompted pref matches the current prompt rev, we do not prompt. This allows us to force reprompting in the future by changing the prompt rev constant.
Comment 18 Lawrence Mandel [:lmandel] (use needinfo) 2011-12-08 12:12:25 PST
Can the notification be dismissed without clicking yes or no?
Comment 19 Virgil Dicu [:virgil] [QA] 2011-12-09 08:38:24 PST
Thank you for the feedback.

The Close button (x) is still displayed in the notification if selecting "Learn more" (privacy policy). Is this expected?

As of the rest, I can confirm the behavior specified in comment 15 with the additions from comment 16, 17:

Clicking yes: toolkit.telemetry.enabled=true
Clicking no: toolkit.telemetry.enabled=false and toolkit.telemetry.rejected=true
Clicking x: toolkit.telemetry.enabled=false 

toolkit.telemetry.prompted is set to "2" regardless of the option chosen in the notification.

(In reply to Lawrence Mandel [:lmandel] from comment #18)
> Can the notification be dismissed without clicking yes or no?

From my experience, apart from closing the notification within the privacy policy, the only way to close it is to close the tab.
Comment 20 :Gavin Sharp [email: gavin@gavinsharp.com] 2011-12-12 15:47:48 PST
(In reply to Virgil Dicu [QA] from comment #19)
> the only way to close it is to close the tab.

That's correct.
Comment 21 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-13 13:50:49 PST
Virgil can you please verify if this is fixed on Beta given your testing so far?
Comment 22 Virgil Dicu [:virgil] [QA] 2011-12-14 05:30:38 PST
I specified and asked this in previous comments: the Close button (x) is still displayed in the notification if selecting "Learn more" (privacy policy).

I filed a bug for this: bug 710589. If it's intended that can be set to Invalid.
Comment 23 Virgil Dicu [:virgil] [QA] 2011-12-14 05:31:09 PST
Mozilla/5.0 (Windows NT 6.1; rv:9.0) Gecko/20100101 Firefox/9.0

The rest works as expected, as specified in comment 19:
Clicking yes: toolkit.telemetry.enabled=true
Clicking no: toolkit.telemetry.enabled=false and toolkit.telemetry.rejected=true
Clicking x: toolkit.telemetry.enabled=false 

I've only let this go for the answer to the problem which I now reported in bug 710589. Setting thus to verified for beta.
Comment 24 Mike Kaply [:mkaply] 2011-12-22 12:33:31 PST
This was a bad change. The pref should have been renamed.

It's causing an error for enterprises that were setting it to to true in their config files to prevent it from being shown to users.
Comment 25 Mike Kaply [:mkaply] 2011-12-22 12:39:15 PST
For people that don't know about this feature:

https://developer.mozilla.org/en/Automatic_Mozilla_Configurator/Locked_config_settings
Comment 26 Saul 2011-12-22 13:34:20 PST
While upgrading from Firefox 8 to 9, upon first launch an error appears with the following text,

AutoConfig Alert
Netscape.cfg/AutoConfig failed. Please contact your system administrator.
Error: defaultPref failed: [Exception... "Component returned failure code 0x8000ffff
(NS_ERROR_UNEXPECTED) [nsIPrefBranch.SetIntPref]" nsresult: "0x8000ffff
(NS_ERROR_UNEXPECTED)" location: "JS fram :: prefcalls.js :: defaultPref "" line 88" data: no]

This error is self correcting and does not re-occur. 

This error is related to the type change between Firefox version 8 and 9 as follows:
Prior to Firefox 9:
user_pref("toolkit.telemetry.prompted", true);
 
Firefox 9:
user_pref("toolkit.telemetry.prompted", 2);

The above setting is located under the user's profile in the prefs.js file. Following the error, the prefs.js file is updated to use the integer type. (hence self-correction)

To reproduce the error, the installation needs to be configured for the Locked Config Settings (mozilla.cfg) with the following parameter specified. 
                       lockPref("toolkit.telemetry.prompted", 2)
Comment 27 (dormant account) 2011-12-22 13:38:07 PST
(In reply to Michael Kaply (mkaply) from comment #25)
> For people that don't know about this feature:
> 
> https://developer.mozilla.org/en/Automatic_Mozilla_Configurator/
> Locked_config_settings

I didn't know about this. I can't access the page atm, but i guess the fix is to update the pref in the configurator.
Comment 28 Mike Kaply [:mkaply] 2011-12-22 13:53:45 PST
> I didn't know about this. I can't access the page atm, but i guess the fix is to update the pref in the configurator.

No, the correct fix is to revert the change and not turn a boolean pref into an int.

The problem is that a situation is created now where configuration file that worked on FF 8 and below don't work at 9.

In addition, this change would break any add-on that was setting or querying that preference.

Changing a preference from one type to another is generally a bad idea.
Comment 29 (dormant account) 2011-12-22 14:17:21 PST
Michael, I can see your point, but this didn't come up during beta testing and now that this is released it's not possible to change it back without reprompting every firefox user again.
Comment 30 Mike Kaply [:mkaply] 2011-12-22 14:25:50 PST
Well I hope someone makes a note somewhere that changing a preference from one type to another is never a good idea. And it certainly should have been documented somewhere for add-on authors which would have caused someone to notice it.

Unfortunately there's no good way to fix this problem so that it works on Firefox 8 and 9. We could add a try catch in autoconfig to prevent the popup, but we're still left with the fact that users will be prompted for telemetry when companies do not want them to be prompted.
Comment 31 Virgil Dicu [:virgil] [QA] 2011-12-28 04:39:02 PST
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0) Gecko/20100101 Firefox/10.0

This works as specified in comment 23 for Firefox 10 beta 1 too. Setting resolution to Verified Fixed.

Clicking yes: toolkit.telemetry.enabled=true
Clicking no: toolkit.telemetry.enabled=false and toolkit.telemetry.rejected=true
Clicking x: toolkit.telemetry.enabled=false

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