Closed Bug 697860 Opened 13 years ago Closed 13 years ago

Hide close button in the telemetry notification, and track explicit clicks on "no"

Categories

(Toolkit :: Telemetry, defect)

x86
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla10
Tracking Status
firefox9 --- verified

People

(Reporter: taras.mozilla, Assigned: taras.mozilla)

References

Details

(Keywords: verified-beta, Whiteboard: [qa!])

Attachments

(1 file, 1 obsolete file)

We should track the difference between clicking "no" and "x" in telemetry prompt.
Attached patch store prompt response (obsolete) — Splinter Review
Assignee: nobody → tglek
Attachment #570397 - Flags: review?
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).
Attachment #570397 - Flags: review? → review?(gavin.sharp)
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.
ping gavin?
Depends on: 691951
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.
Attachment #570397 - Flags: review?(gavin.sharp) → review-
Attached patch v2Splinter Review
Attachment #570397 - Attachment is obsolete: true
Attachment #571698 - Flags: review?(gavin.sharp)
Attachment #571698 - Flags: review?(gavin.sharp) → review+
https://hg.mozilla.org/mozilla-central/rev/1592d3bec85b
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla10
Attachment #571698 - Flags: approval-mozilla-aurora?
Attachment #571698 - Flags: approval-mozilla-aurora? → approval-mozilla-beta+
(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?
(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.
QA+ for verification using comment 3.
Whiteboard: [qa+]
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.
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 (?)?
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
Summary: Telemetry prompt should be tri-state → Hide close button in the telemetry notification, and track explicit clicks on "no"
(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.
Can the notification be dismissed without clicking yes or no?
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.
(In reply to Virgil Dicu [QA] from comment #19)
> the only way to close it is to close the tab.

That's correct.
Virgil can you please verify if this is fixed on Beta given your testing so far?
Blocks: 710589
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.
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.
Keywords: verified-beta
Whiteboard: [qa+] → [qa+], [qa!:9]
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.
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)
(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.
> 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.
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.
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.
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
Status: RESOLVED → VERIFIED
Whiteboard: [qa+], [qa!:9] → [qa!], [qa!:9], [qa!:10]
Whiteboard: [qa!], [qa!:9], [qa!:10] → [qa!]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: