Backed out 2 changesets (bug 1501877, bug 1498940) for failing browser_about_studies.js CLOSED TREE
authorCiure Andrei <aciure@mozilla.com>
Thu, 25 Oct 2018 21:24:45 +0300
changeset 491364 d08c28ed42f1
parent 491363 821f982805ab
child 491365 9397caac8090
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
bugs1501877, 1498940
milestone65.0a1
backs outf0ed99b29aaf
ae8cdf156f9a
Backed out 2 changesets (bug 1501877, bug 1498940) for failing browser_about_studies.js CLOSED TREE Backed out changeset f0ed99b29aaf (bug 1498940) Backed out changeset ae8cdf156f9a (bug 1501877)
toolkit/components/normandy/content/AboutPages.jsm
toolkit/components/normandy/content/about-studies/about-studies.js
toolkit/components/normandy/lib/RecipeRunner.jsm
toolkit/components/normandy/test/browser/browser_about_studies.js
--- a/toolkit/components/normandy/content/AboutPages.jsm
+++ b/toolkit/components/normandy/content/AboutPages.jsm
@@ -10,17 +10,16 @@ ChromeUtils.defineModuleGetter(this, "Ad
 ChromeUtils.defineModuleGetter(this, "AddonStudyAction", "resource://normandy/actions/AddonStudyAction.jsm");
 ChromeUtils.defineModuleGetter(this, "CleanupManager", "resource://normandy/lib/CleanupManager.jsm");
 ChromeUtils.defineModuleGetter(this, "PreferenceExperiments", "resource://normandy/lib/PreferenceExperiments.jsm");
 ChromeUtils.defineModuleGetter(this, "RecipeRunner", "resource://normandy/lib/RecipeRunner.jsm");
 
 var EXPORTED_SYMBOLS = ["AboutPages"];
 
 const SHIELD_LEARN_MORE_URL_PREF = "app.normandy.shieldLearnMoreUrl";
-XPCOMUtils.defineLazyPreferenceGetter(this, "gOptOutStudiesEnabled", "app.shield.optoutstudies.enabled");
 
 
 /**
  * Class for managing an about: page that Normandy provides. Adapted from
  * browser/components/pocket/content/AboutPocket.jsm.
  *
  * @implements nsIFactory
  * @implements nsIAboutModule
@@ -188,19 +187,20 @@ XPCOMUtils.defineLazyGetter(this.AboutPa
      * content processes safely.
      *
      * @param {<browser>} target
      *   XUL <browser> element for the tab containing the about:studies page
      *   that requested a study list.
      */
     sendStudiesEnabled(target) {
       RecipeRunner.checkPrefs();
-      const studiesEnabled = RecipeRunner.enabled && gOptOutStudiesEnabled;
       try {
-        target.messageManager.sendAsyncMessage("Shield:ReceiveStudiesEnabled", { studiesEnabled });
+        target.messageManager.sendAsyncMessage("Shield:ReceiveStudiesEnabled", {
+          studiesEnabled: RecipeRunner.enabled,
+        });
       } catch (err) {
         // The child process might be gone, so no need to throw here.
         Cu.reportError(err);
       }
     },
 
     /**
      * Disable an active add-on study and remove its add-on.
@@ -232,12 +232,17 @@ XPCOMUtils.defineLazyGetter(this.AboutPa
     openDataPreferences() {
       const browserWindow = Services.wm.getMostRecentWindow("navigator:browser");
       browserWindow.openPreferences("privacy-reports", {origin: "aboutStudies"});
     },
 
     getShieldLearnMoreHref() {
       return Services.urlFormatter.formatURLPref(SHIELD_LEARN_MORE_URL_PREF);
     },
+
+    getStudiesEnabled() {
+      RecipeRunner.checkPrefs();
+      return RecipeRunner.enabled;
+    },
   });
 
   return aboutStudies;
 });
--- a/toolkit/components/normandy/content/about-studies/about-studies.js
+++ b/toolkit/components/normandy/content/about-studies/about-studies.js
@@ -145,25 +145,25 @@ class StudyList extends React.Component 
     }
 
     activeStudies.sort((a, b) => b.sortDate - a.sortDate);
     inactiveStudies.sort((a, b) => b.sortDate - a.sortDate);
 
     return (
       r("div", {},
         r("h2", {}, translations.activeStudiesList),
-        r("ul", { className: "study-list active-study-list" },
+        r("ul", { className: "study-list" },
           activeStudies.map(study => (
             study.type === "addon"
             ? r(AddonStudyListItem, { key: study.name, study, translations })
             : r(PreferenceStudyListItem, { key: study.name, study, translations })
           )),
         ),
         r("h2", {}, translations.completedStudiesList),
-        r("ul", { className: "study-list inactive-study-list" },
+        r("ul", { className: "study-list" },
           inactiveStudies.map(study => (
             study.type === "addon"
             ? r(AddonStudyListItem, { key: study.name, study, translations })
             : r(PreferenceStudyListItem, { key: study.name, study, translations })
           )),
         ),
       )
     );
--- a/toolkit/components/normandy/lib/RecipeRunner.jsm
+++ b/toolkit/components/normandy/lib/RecipeRunner.jsm
@@ -153,22 +153,17 @@ var RecipeRunner = {
 
     if (!Services.policies.isAllowed("Shield")) {
       log.debug("Disabling Shield because it's blocked by policy.");
       this.disable();
       return;
     }
 
     const apiUrl = Services.prefs.getCharPref(API_URL_PREF);
-    if (!apiUrl) {
-      log.warn(`Disabling Shield because ${API_URL_PREF} is not set.`);
-      this.disable();
-      return;
-    }
-    if (!apiUrl.startsWith("https://")) {
+    if (!apiUrl || !apiUrl.startsWith("https://")) {
       log.warn(`Disabling Shield because ${API_URL_PREF} is not an HTTPS url: ${apiUrl}.`);
       this.disable();
       return;
     }
 
     log.debug(`Enabling Shield`);
     this.enable();
   },
--- a/toolkit/components/normandy/test/browser/browser_about_studies.js
+++ b/toolkit/components/normandy/test/browser/browser_about_studies.js
@@ -66,17 +66,16 @@ decorate_task(
       "about:preferences#privacy",
       "Clicking Update Preferences opens the privacy section of the new about:preferences.",
     );
 
     BrowserTestUtils.removeTab(tab);
   }
 );
 
-// Test that the study listing shows studies in the proper order and grouping
 decorate_task(
   AddonStudies.withStudies([
     addonStudyFactory({
       name: "A Fake Add-on Study",
       active: true,
       description: "A fake description",
       studyStartDate: new Date(2018, 0, 4),
     }),
@@ -195,29 +194,29 @@ decorate_task(
       );
       ok(
         !inactivePrefStudy.querySelector(".remove-button"),
         "Inactive studies do not show a remove button"
       );
 
       activeAddonStudy.querySelector(".remove-button").click();
       await ContentTaskUtils.waitForCondition(() => (
-        getStudyRow(doc, addonStudies[0].name).matches(".study.disabled")
+        getStudyRow(doc, addonStudies[0].name).matches(".study--disabled")
       ));
       ok(
-        getStudyRow(doc, addonStudies[0].name).matches(".study.disabled"),
+        getStudyRow(doc, addonStudies[0].name).matches(".study--disabled"),
         "Clicking the remove button updates the UI to show that the study has been disabled."
       );
 
       activePrefStudy.querySelector(".remove-button").click();
       await ContentTaskUtils.waitForCondition(() => (
-        getStudyRow(doc, prefStudies[0].name).matches(".study.disabled")
+        getStudyRow(doc, prefStudies[0].name).matches(".study--disabled")
       ));
       ok(
-        getStudyRow(doc, prefStudies[0].name).matches(".study.disabled"),
+        getStudyRow(doc, prefStudies[0].name).matches(".study--disabled"),
         "Clicking the remove button updates the UI to show that the study has been disabled."
       );
     });
 
     const updatedAddonStudy = await AddonStudies.get(addonStudies[0].recipeId);
     ok(
       !updatedAddonStudy.active,
       "Clicking the remove button marks addon studies as inactive in storage."
@@ -226,54 +225,37 @@ decorate_task(
     const updatedPrefStudy = await PreferenceExperiments.get(prefStudies[0].name);
     ok(
       updatedPrefStudy.expired,
       "Clicking the remove button marks preference studies as expired in storage."
     );
   }
 );
 
-// Test that a message is shown when no studies have been run
 decorate_task(
   AddonStudies.withStudies([]),
   withAboutStudies,
-  async function testStudyListingNoStudies(studies, browser) {
+  async function testStudyListing(studies, browser) {
     await ContentTask.spawn(browser, null, async () => {
       const doc = content.document;
       await ContentTaskUtils.waitForCondition(() => doc.querySelectorAll(".study-list-info").length);
       const studyRows = doc.querySelectorAll(".study-list .study");
       is(studyRows.length, 0, "There should be no studies");
       is(
         doc.querySelector(".study-list-info").textContent,
         "You have not participated in any studies.",
         "A message is shown when no studies exist",
       );
     });
   }
 );
 
-// Test that the message shown when studies are disabled and studies exist
 decorate_task(
   withAboutStudies,
-  AddonStudies.withStudies([
-    addonStudyFactory({
-      name: "A Fake Add-on Study",
-      active: false,
-      description: "A fake description",
-      studyStartDate: new Date(2018, 0, 4),
-    }),
-  ]),
-  PreferenceExperiments.withMockExperiments([
-    preferenceStudyFactory({
-      name: "B Fake Preference Study",
-      lastSeen: new Date(2018, 0, 5),
-      expired: true,
-    }),
-  ]),
-  async function testStudyListingDisabled(browser, addonStudies, preferenceStudies) {
+  async function testStudyListing(browser) {
     try {
       RecipeRunner.disable();
 
       await ContentTask.spawn(browser, null, async () => {
         const doc = content.document;
         await ContentTaskUtils.waitForCondition(() => doc.querySelector(".info-box-content > span"));
 
         is(
@@ -282,41 +264,9 @@ decorate_task(
           "A message is shown when studies are disabled",
         );
       });
     } finally {
       // reset RecipeRunner.enabled
       RecipeRunner.checkPrefs();
     }
   }
-);
-
-// Test for bug 1498940 - detects studies disabled when only study opt-out is set
-decorate_task(
-  withPrefEnv({
-    set: [
-      ["datareporting.healthreport.uploadEnabled", true],
-      ["app.normandy.api_url", "https://example.com"],
-      ["app.shield.optoutstudies.enabled", false],
-    ],
-  }),
-  withAboutStudies,
-  AddonStudies.withStudies([]),
-  PreferenceExperiments.withMockExperiments([]),
-  async function testStudyListingStudiesOptOut(browser) {
-    RecipeRunner.checkPrefs();
-    ok(
-      RecipeRunner.enabled,
-      "RecipeRunner should be enabled as a Precondition",
-    );
-
-    await ContentTask.spawn(browser, null, async () => {
-      const doc = content.document;
-      await ContentTaskUtils.waitForCondition(() => doc.querySelector(".info-box-content > span").textContent);
-
-      is(
-        doc.querySelector(".info-box-content > span").textContent,
-        "This is a list of studies that you have participated in. No new studies will run.",
-        "A message is shown when studies are disabled",
-      );
-    });
-  }
-);
+).only();