Bug 1457620 - Expose shield preference toggle to all locales and move code to not being injected. r=flod,Gijs,mythmon
authorJonathan Kingston <jkt@mozilla.com>
Sat, 05 May 2018 11:07:08 +0100
changeset 794469 5bbecfa4b58cc85710332fbdc12d20af150963b6
parent 794468 a264016b1b7f416d1594265612752b662f1d4c5b
child 794470 9248d8d844876ff8b83a0233ff1b985a371b15e4
push id109697
push userbmo:sledru@mozilla.com
push dateSat, 12 May 2018 10:04:34 +0000
reviewersflod, Gijs, mythmon
bugs1457620
milestone62.0a1
Bug 1457620 - Expose shield preference toggle to all locales and move code to not being injected. r=flod,Gijs,mythmon MozReview-Commit-ID: 4uZSdqqAlG8
browser/components/preferences/in-content/privacy.js
browser/components/preferences/in-content/privacy.xul
browser/locales/en-US/browser/preferences/preferences.ftl
toolkit/components/normandy/lib/ShieldPreferences.jsm
--- a/browser/components/preferences/in-content/privacy.js
+++ b/browser/components/preferences/in-content/privacy.js
@@ -24,16 +24,19 @@ XPCOMUtils.defineLazyPreferenceGetter(th
 ChromeUtils.import("resource://gre/modules/PrivateBrowsingUtils.jsm");
 
 const PREF_UPLOAD_ENABLED = "datareporting.healthreport.uploadEnabled";
 
 const TRACKING_PROTECTION_KEY = "websites.trackingProtectionMode";
 const TRACKING_PROTECTION_PREFS = ["privacy.trackingprotection.enabled",
                                    "privacy.trackingprotection.pbmode.enabled"];
 
+const PREF_OPT_OUT_STUDIES_ENABLED = "app.shield.optoutstudies.enabled";
+const PREF_NORMANDY_ENABLED = "app.normandy.enabled";
+
 XPCOMUtils.defineLazyGetter(this, "AlertsServiceDND", function() {
   try {
     let alertsService = Cc["@mozilla.org/alerts-service;1"]
       .getService(Ci.nsIAlertsService)
       .QueryInterface(Ci.nsIAlertsDoNotDisturb);
     // This will throw if manualDoNotDisturb isn't implemented.
     alertsService.manualDoNotDisturb;
     return alertsService;
@@ -106,18 +109,28 @@ Preferences.addAll([
   { id: "browser.safebrowsing.phishing.enabled", type: "bool" },
 
   { id: "browser.safebrowsing.downloads.enabled", type: "bool" },
 
   { id: "urlclassifier.malwareTable", type: "string" },
 
   { id: "browser.safebrowsing.downloads.remote.block_potentially_unwanted", type: "bool" },
   { id: "browser.safebrowsing.downloads.remote.block_uncommon", type: "bool" },
+
 ]);
 
+// Study opt out
+if (AppConstants.MOZ_DATA_REPORTING) {
+  Preferences.addAll([
+    // Preference instances for prefs that we need to monitor while the page is open.
+    { id: PREF_OPT_OUT_STUDIES_ENABLED, type: "bool" },
+    { id: PREF_UPLOAD_ENABLED, type: "bool" },
+  ]);
+}
+
 // Data Choices tab
 if (AppConstants.NIGHTLY_BUILD) {
   Preferences.add({ id: "browser.chrome.errorReporter.enabled", type: "bool" });
 }
 if (AppConstants.MOZ_CRASHREPORTER) {
   Preferences.add({ id: "browser.crashReports.unsubmittedCheck.autoSubmit2", type: "bool" });
 }
 
@@ -375,16 +388,17 @@ var gPrivacyPane = {
         this.initCollectBrowserErrors();
       }
       if (AppConstants.MOZ_CRASHREPORTER) {
         this.initSubmitCrashes();
       }
       this.initSubmitHealthReport();
       setEventListener("submitHealthReportBox", "command",
         gPrivacyPane.updateSubmitHealthReport);
+      this.initOptOutStudyCheckbox();
     }
     this._initA11yState();
     let signonBundle = document.getElementById("signonBundle");
     let pkiBundle = document.getElementById("pkiBundle");
     appendSearchKeywords("showPasswords", [
       signonBundle.getString("loginsDescriptionAll2"),
     ]);
     appendSearchKeywords("viewSecurityDevicesButton", [
@@ -1360,16 +1374,78 @@ var gPrivacyPane = {
   /**
    * Update the health report preference with state from checkbox.
    */
   updateSubmitHealthReport() {
     let checkbox = document.getElementById("submitHealthReportBox");
     Services.prefs.setBoolPref(PREF_UPLOAD_ENABLED, checkbox.checked);
   },
 
+
+  /**
+   * Initialize the opt-out-study preference checkbox into about:preferences and
+   * handles events coming from the UI for it.
+   */
+  initOptOutStudyCheckbox(doc) {
+    const allowedByPolicy = Services.policies.isAllowed("Shield");
+    const checkbox = document.getElementById("optOutStudiesEnabled");
+
+    function updateStudyCheckboxState() {
+      // The checkbox should be disabled if any of the below are true. This
+      // prevents the user from changing the value in the box.
+      //
+      // * the policy forbids shield
+      // * the Shield Study preference is locked
+      // * the FHR pref is false
+      //
+      // The checkbox should match the value of the preference only if all of
+      // these are true. Otherwise, the checkbox should remain unchecked. This
+      // is because in these situations, Shield studies are always disabled, and
+      // so showing a checkbox would be confusing.
+      //
+      // * the policy allows Shield
+      // * the FHR pref is true
+      // * Normandy is enabled
+
+      const checkboxMatchesPref = (
+        allowedByPolicy &&
+        Services.prefs.getBoolPref(PREF_UPLOAD_ENABLED, false) &&
+        Services.prefs.getBoolPref(PREF_NORMANDY_ENABLED, false)
+      );
+
+      if (checkboxMatchesPref) {
+        if (Services.prefs.getBoolPref(PREF_OPT_OUT_STUDIES_ENABLED, false)) {
+          checkbox.setAttribute("checked", "checked");
+        } else {
+          checkbox.removeAttribute("checked");
+        }
+        checkbox.setAttribute("preference", PREF_OPT_OUT_STUDIES_ENABLED);
+      } else {
+        checkbox.removeAttribute("preference");
+        checkbox.removeAttribute("checked");
+      }
+
+      const isDisabled = (
+        !allowedByPolicy ||
+        Services.prefs.prefIsLocked(PREF_OPT_OUT_STUDIES_ENABLED) ||
+        !Services.prefs.getBoolPref(PREF_UPLOAD_ENABLED, false)
+      );
+
+      // We can't use checkbox.disabled here because the XBL binding may not be present,
+      // in which case setting the property won't work properly.
+      if (isDisabled) {
+        checkbox.setAttribute("disabled", "true");
+      } else {
+        checkbox.removeAttribute("disabled");
+      }
+    }
+    Preferences.get(PREF_UPLOAD_ENABLED).on("change", updateStudyCheckboxState);
+    updateStudyCheckboxState();
+  },
+
   observe(aSubject, aTopic, aData) {
     switch (aTopic) {
       case "sitedatamanager:updating-sites":
         // While updating, we want to disable this section and display loading message until updated
         this.toggleSiteData(false);
         this.showSiteDataLoading();
         break;
 
--- a/browser/components/preferences/in-content/privacy.xul
+++ b/browser/components/preferences/in-content/privacy.xul
@@ -573,16 +573,28 @@
   <vbox data-subcategory="reports">
     <description flex="1">
       <checkbox id="submitHealthReportBox"
                 data-l10n-id="collection-health-report"
                 class="tail-with-learn-more"/>
       <label id="FHRLearnMore"
              class="learnMore text-link"
              data-l10n-id="collection-health-report-link"/>
+      <vbox class="indent">
+        <hbox align="center">
+          <checkbox id="optOutStudiesEnabled"
+                    class="tail-with-learn-more"
+                    data-l10n-id="collection-studies"/>
+          <label id="viewShieldStudies"
+                 href="about:studies"
+                 useoriginprincipal="true"
+                 class="learnMore text-link"
+                 data-l10n-id="collection-studies-link"/>
+        </hbox>
+      </vbox>
     </description>
 #ifndef MOZ_TELEMETRY_REPORTING
   <description id="TelemetryDisabledDesc"
     class="indent tip-caption" control="telemetryGroup"
     data-l10n-id="collection-health-report-disabled"/>
 #endif
 
 #ifdef NIGHTLY_BUILD
--- a/browser/locales/en-US/browser/preferences/preferences.ftl
+++ b/browser/locales/en-US/browser/preferences/preferences.ftl
@@ -874,16 +874,20 @@ collection-header = { -brand-short-name 
 collection-description = We strive to provide you with choices and collect only what we need to provide and improve { -brand-short-name } for everyone. We always ask permission before receiving personal information.
 collection-privacy-notice = Privacy Notice
 
 collection-health-report =
     .label = Allow { -brand-short-name } to send technical and interaction data to { -vendor-short-name }
     .accesskey = r
 collection-health-report-link = Learn more
 
+collection-studies =
+    .label = Allow { -brand-short-name } to install and run studies
+collection-studies-link = View { -brand-short-name } studies
+
 # This message is displayed above disabled data sharing options in developer builds
 # or builds with no Telemetry support available.
 collection-health-report-disabled = Data reporting is disabled for this build configuration
 
 collection-browser-errors =
     .label = Allow { -brand-short-name } to send browser error reports (including error messages) to { -vendor-short-name }
     .accesskey = b
 collection-browser-errors-link = Learn more
--- a/toolkit/components/normandy/lib/ShieldPreferences.jsm
+++ b/toolkit/components/normandy/lib/ShieldPreferences.jsm
@@ -2,176 +2,57 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 ChromeUtils.defineModuleGetter(
-  this, "AppConstants", "resource://gre/modules/AppConstants.jsm"
-);
-ChromeUtils.defineModuleGetter(
   this, "AddonStudies", "resource://normandy/lib/AddonStudies.jsm"
 );
 ChromeUtils.defineModuleGetter(
   this, "CleanupManager", "resource://normandy/lib/CleanupManager.jsm"
 );
 
 var EXPORTED_SYMBOLS = ["ShieldPreferences"];
 
-const XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 const NS_PREFBRANCH_PREFCHANGE_TOPIC_ID = "nsPref:changed"; // from modules/libpref/nsIPrefBranch.idl
-const FHR_UPLOAD_ENABLED_PREF = "datareporting.healthreport.uploadEnabled";
-const OPT_OUT_STUDIES_ENABLED_PREF = "app.shield.optoutstudies.enabled";
-const NORMANDY_ENABLED_PREF = "app.normandy.enabled";
+const PREF_OPT_OUT_STUDIES_ENABLED = "app.shield.optoutstudies.enabled";
 
 /**
  * Handles Shield-specific preferences, including their UI.
  */
 var ShieldPreferences = {
   init() {
-    // Watch for changes to the FHR pref
-    Services.prefs.addObserver(FHR_UPLOAD_ENABLED_PREF, this);
+    // Watch for changes to the Opt-out pref
+    Services.prefs.addObserver(PREF_OPT_OUT_STUDIES_ENABLED, this);
     CleanupManager.addCleanupHandler(() => {
-      Services.prefs.removeObserver(FHR_UPLOAD_ENABLED_PREF, this);
-    });
-
-    // Watch for changes to the Opt-out pref
-    Services.prefs.addObserver(OPT_OUT_STUDIES_ENABLED_PREF, this);
-    CleanupManager.addCleanupHandler(() => {
-      Services.prefs.removeObserver(OPT_OUT_STUDIES_ENABLED_PREF, this);
+      Services.prefs.removeObserver(PREF_OPT_OUT_STUDIES_ENABLED, this);
     });
-
-    // Disabled outside of en-* locales temporarily (bug 1377192).
-    // Disabled when MOZ_DATA_REPORTING is false since the FHR UI is also hidden
-    // when data reporting is false.
-    if (AppConstants.MOZ_DATA_REPORTING && Services.locale.getAppLocaleAsLangTag().startsWith("en")) {
-      Services.obs.addObserver(this, "privacy-pane-loaded");
-      CleanupManager.addCleanupHandler(() => {
-        Services.obs.removeObserver(this, "privacy-pane-loaded");
-      });
-    }
   },
 
   observe(subject, topic, data) {
     switch (topic) {
-      // Add the opt-out-study checkbox to the Privacy preferences when it is shown.
-      case "privacy-pane-loaded":
-        this.injectOptOutStudyCheckbox(subject.document);
-        break;
       case NS_PREFBRANCH_PREFCHANGE_TOPIC_ID:
         this.observePrefChange(data);
         break;
     }
   },
 
   async observePrefChange(prefName) {
     let prefValue;
     switch (prefName) {
       // If the opt-out pref changes to be false, disable all current studies.
-      case OPT_OUT_STUDIES_ENABLED_PREF: {
-        prefValue = Services.prefs.getBoolPref(OPT_OUT_STUDIES_ENABLED_PREF);
+      case PREF_OPT_OUT_STUDIES_ENABLED: {
+        prefValue = Services.prefs.getBoolPref(PREF_OPT_OUT_STUDIES_ENABLED);
         if (!prefValue) {
           for (const study of await AddonStudies.getAll()) {
             if (study.active) {
               await AddonStudies.stop(study.recipeId, "general-opt-out");
             }
           }
         }
         break;
       }
     }
   },
-
-  /**
-   * Injects the opt-out-study preference checkbox into about:preferences and
-   * handles events coming from the UI for it.
-   */
-  injectOptOutStudyCheckbox(doc) {
-    const allowedByPolicy = Services.policies.isAllowed("Shield");
-
-    const container = doc.createElementNS(XUL_NS, "vbox");
-    container.classList.add("indent");
-
-    const hContainer = doc.createElementNS(XUL_NS, "hbox");
-    hContainer.setAttribute("align", "center");
-    container.appendChild(hContainer);
-
-    const checkbox = doc.createElementNS(XUL_NS, "checkbox");
-    checkbox.setAttribute("id", "optOutStudiesEnabled");
-    checkbox.setAttribute("class", "tail-with-learn-more");
-    checkbox.setAttribute("label", "Allow Firefox to install and run studies");
-    hContainer.appendChild(checkbox);
-
-    const viewStudies = doc.createElementNS(XUL_NS, "label");
-    viewStudies.setAttribute("id", "viewShieldStudies");
-    viewStudies.setAttribute("href", "about:studies");
-    viewStudies.setAttribute("useoriginprincipal", true);
-    viewStudies.textContent = "View Firefox Studies";
-    viewStudies.classList.add("learnMore", "text-link");
-    hContainer.appendChild(viewStudies);
-
-    // Preference instances for prefs that we need to monitor while the page is open.
-    doc.defaultView.Preferences.add({ id: OPT_OUT_STUDIES_ENABLED_PREF, type: "bool" });
-
-    // Weirdly, FHR doesn't have a Preference instance on the page, so we create it.
-    const fhrPref = doc.defaultView.Preferences.add({ id: FHR_UPLOAD_ENABLED_PREF, type: "bool" });
-    function updateStudyCheckboxState() {
-      // The checkbox should be disabled if any of the below are true. This
-      // prevents the user from changing the value in the box.
-      //
-      // * the policy forbids shield
-      // * the Shield Study preference is locked
-      // * the FHR pref is false
-      //
-      // The checkbox should match the value of the preference only if all of
-      // these are true. Otherwise, the checkbox should remain unchecked. This
-      // is because in these situations, Shield studies are always disabled, and
-      // so showing a checkbox would be confusing.
-      //
-      // * MOZ_TELEMETRY_REPORTING is true
-      // * the policy allows Shield
-      // * the FHR pref is true
-      // * Normandy is enabled
-
-      const checkboxMatchesPref = (
-        AppConstants.MOZ_DATA_REPORTING &&
-        allowedByPolicy &&
-        Services.prefs.getBoolPref(FHR_UPLOAD_ENABLED_PREF, false) &&
-        Services.prefs.getBoolPref(NORMANDY_ENABLED_PREF, false)
-      );
-
-      if (checkboxMatchesPref) {
-        if (Services.prefs.getBoolPref(OPT_OUT_STUDIES_ENABLED_PREF)) {
-          checkbox.setAttribute("checked", "checked");
-        } else {
-          checkbox.removeAttribute("checked");
-        }
-        checkbox.setAttribute("preference", OPT_OUT_STUDIES_ENABLED_PREF);
-      } else {
-        checkbox.removeAttribute("preference");
-        checkbox.removeAttribute("checked");
-      }
-
-      const isDisabled = (
-        !allowedByPolicy ||
-        Services.prefs.prefIsLocked(OPT_OUT_STUDIES_ENABLED_PREF) ||
-        !Services.prefs.getBoolPref(FHR_UPLOAD_ENABLED_PREF)
-      );
-
-      // We can't use checkbox.disabled here because the XBL binding may not be present,
-      // in which case setting the property won't work properly.
-      if (isDisabled) {
-        checkbox.setAttribute("disabled", "true");
-      } else {
-        checkbox.removeAttribute("disabled");
-      }
-    }
-    fhrPref.on("change", updateStudyCheckboxState);
-    updateStudyCheckboxState();
-    doc.defaultView.addEventListener("unload", () => fhrPref.off("change", updateStudyCheckboxState), { once: true });
-
-    // Actually inject the elements we've created.
-    const parent = doc.getElementById("submitHealthReportBox").closest("description");
-    parent.appendChild(container);
-  },
 };