commit 77c24e7f8a88e3dc41fa60e9e494ccd553eeabda
parent a01bc22d1280f4051408355de749e93424fc3221
Author: Cristian Tuns <ctuns@mozilla.com>
Date: Mon, 13 Oct 2025 22:29:14 -0400
Revert "Bug 1993295 - Allow legacy telemetry upload for users who predate the new user TOU experience r=toolkit-telemetry-reviewers,dmose,TravisLong" for causing xpcshell failures in /test_TOUNotificationFlow.js
This reverts commit a01bc22d1280f4051408355de749e93424fc3221.
Diffstat:
5 files changed, 134 insertions(+), 291 deletions(-)
diff --git a/toolkit/components/telemetry/app/TelemetryReportingPolicy.sys.mjs b/toolkit/components/telemetry/app/TelemetryReportingPolicy.sys.mjs
@@ -760,9 +760,16 @@ var TelemetryReportingPolicyImpl = {
* Check if we are allowed to upload data.
* Prerequisite: data submission is enabled (this.dataSubmissionEnabled).
*
- * For upload to be allowed from a data reporting standpoint, the user should
- * not qualify to see the legacy policy notification flow and also not qualify
- * to see the Terms of Use acceptance flow.
+ * In order to submit data, at least ONE of these conditions should be true:
+ * 1. The TOU flow is bypassed via a pref or Nimbus variable AND the legacy
+ * notification flow bypass pref is set, so users bypass BOTH flows.
+ * 2. The TOU flow is bypassed via a pref or Nimbus variable and the legacy
+ * notification flow bypass pref is NOT set, so has been been shown the
+ * legacy flow (the data submission pref should be true and the
+ * datachoices infobar should have been displayed).
+ * 3. The user has accepted the Terms of Use AND the user has opted-in to
+ * sharing technical interaction data (the upload enabled pref should be
+ * true).
* @return {Boolean} True if we are allowed to upload data, false otherwise.
*/
canUpload() {
@@ -772,7 +779,33 @@ var TelemetryReportingPolicyImpl = {
return false;
}
- return !this._shouldNotifyDataReportingPolicy() && !this._shouldShowTOU();
+ const bypassLegacyFlow = Services.prefs.getBoolPref(
+ TelemetryUtils.Preferences.BypassNotification,
+ false
+ );
+ // TOU flow is bypassed if the Nimbus preonboarding feature is disabled
+ // (disabled by default for Linux via the fallback
+ // browser.preonboarding.enabled pref) or if the explicit bypass pref is
+ // set.
+ const bypassTOUFlow =
+ Services.prefs.getBoolPref(TOU_BYPASS_NOTIFICATION_PREF, false) ||
+ (!Services.prefs.getBoolPref("browser.preonboarding.enabled", false) &&
+ this._nimbusVariables?.enabled !== true) ||
+ this._nimbusVariables?.enabled === false;
+ const allowInteractionData = Services.prefs.getBoolPref(
+ "datareporting.healthreport.uploadEnabled",
+ false
+ );
+
+ // Condition 1
+ const canUploadBypassLegacyAndTOU = bypassLegacyFlow && bypassTOUFlow;
+ // Condition 2
+ const canUploadLegacy =
+ bypassTOUFlow && !bypassLegacyFlow && this.isUserNotifiedOfCurrentPolicy;
+ // Condition 3
+ const canUploadTOU = this.hasUserAcceptedCurrentTOU && allowInteractionData;
+
+ return canUploadBypassLegacyAndTOU || canUploadLegacy || canUploadTOU;
},
isFirstRun() {
@@ -839,20 +872,6 @@ var TelemetryReportingPolicyImpl = {
* Determine whether the user should be shown the terms of use.
*/
_shouldShowTOU() {
- // In some cases, _shouldShowTOU can be called before the Nimbus variables
- // are set. When this happens, we call _configureFromNimbus to initialize
- // these variables before evaluating them. This ensures we have accurate
- // data regarding whether preonboarding is enabled for the user. When
- // preonboarding is explicitly disabled, it should be treated the same the
- // bypassing the TOU flow via the bypass pref.
- if (
- !this._nimbusVariables ||
- (typeof this._nimbusVariables === "object" &&
- Object.keys(this._nimbusVariables).length === 0)
- ) {
- this._configureFromNimbus();
- }
-
if (!this._nimbusVariables.enabled || !this._nimbusVariables.screens) {
this._log.trace(
"_shouldShowTOU - TOU not enabled or no screens configured."
@@ -1077,7 +1096,7 @@ var TelemetryReportingPolicyImpl = {
// _during_ the Firefox process lifetime; right now, we only notify the user
// at Firefox startup.
this.updateTOUPrefsForLegacyUsers();
- this._configureFromNimbus();
+ await this._configureFromNimbus();
if (this.isFirstRun()) {
// We're performing the first run, flip firstRun preference for subsequent runs.
@@ -1214,7 +1233,7 @@ var TelemetryReportingPolicyImpl = {
* Capture Nimbus configuration: record feature variables for future use and
* set Gecko preferences based on values.
*/
- _configureFromNimbus() {
+ async _configureFromNimbus() {
if (AppConstants.MOZ_BUILD_APP != "browser") {
// OnboardingMessageProvider is browser/ only
return;
diff --git a/toolkit/components/telemetry/tests/unit/head.js b/toolkit/components/telemetry/tests/unit/head.js
@@ -594,17 +594,6 @@ if (runningInParent) {
);
}
- // Disable TOU pre-onboarding in xpcshell so Telemetry isn't gated on Browser
- // UI.
- const TOS_ENABLED_PREF = "browser.preonboarding.enabled";
- const previous = Services.prefs.getBoolPref(TOS_ENABLED_PREF, false);
-
- Services.prefs.setBoolPref(TOS_ENABLED_PREF, false);
-
- registerCleanupFunction(() => {
- Services.prefs.setBoolPref(TOS_ENABLED_PREF, previous);
- });
-
fakePingSendTimer(
callback => {
Services.tm.dispatchToMainThread(() => callback());
diff --git a/toolkit/components/telemetry/tests/unit/test_PrefMigrationForTOU.js b/toolkit/components/telemetry/tests/unit/test_PrefMigrationForTOU.js
@@ -38,13 +38,6 @@ const skipIfNotBrowser = () => ({
skip_if: () => AppConstants.MOZ_BUILD_APP != "browser",
});
-add_setup(() => {
- // In head.js, we force TOU pre-onboarding off in xpcshell so Telemetry isn't
- // gated on Browser UI. Revert for these tests.
- const TOS_ENABLED_PREF = "browser.preonboarding.enabled";
- Services.prefs.clearUserPref(TOS_ENABLED_PREF);
-});
-
function setupLegacyAndRolloutPrefs({
acceptedVersion,
notifiedTime,
diff --git a/toolkit/components/telemetry/tests/unit/test_TOUNotificationFlow.js b/toolkit/components/telemetry/tests/unit/test_TOUNotificationFlow.js
@@ -100,34 +100,6 @@ add_setup(skipIfNotBrowser(), async () => {
registerCleanupFunction(cleanup);
});
-add_setup(() => {
- // In head.js, we force TOU pre-onboarding off in xpcshell so Telemetry isn't
- // gated on Browser UI. Revert for these tests.
- const TOS_ENABLED_PREF = "browser.preonboarding.enabled";
- Services.prefs.clearUserPref(TOS_ENABLED_PREF);
-});
-
-add_setup(() => {
- // Clean up all potentially modified prefs and reset state after test tasks
- // have run.
- registerCleanupFunction(() => {
- Services.prefs.clearUserPref(TOU_ACCEPTED_DATE_PREF);
- Services.prefs.clearUserPref(TOU_ACCEPTED_VERSION_PREF);
- Services.prefs.clearUserPref(TOU_BYPASS_NOTIFICATION_PREF);
- Services.prefs.clearUserPref("browser.preonboarding.enabled");
- Services.prefs.clearUserPref("browser.preonboarding.screens");
- Services.prefs.clearUserPref(
- "datareporting.policy.dataSubmissionPolicyNotifiedTime"
- );
- Services.prefs.clearUserPref(
- "datareporting.policy.dataSubmissionPolicyAcceptedVersion"
- );
- Services.prefs.clearUserPref(TelemetryUtils.Preferences.BypassNotification);
-
- TelemetryReportingPolicy.reset();
- });
-});
-
add_task(skipIfNotBrowser(), async function test_feature_prefs() {
// Verify that feature values impact Gecko preferences at
// `sessionstore-windows-restored` time, but not afterward.
@@ -703,289 +675,160 @@ add_task(
}
);
-add_task(async function test_canUpload_unblocked_by_tou_accepted() {
- if (AppConstants.platform === "linux") {
- info("Skipping test for Linux where TOU flow is disabled by default");
- return;
- }
-
- const cleanup = () => {
- Services.prefs.clearUserPref(TelemetryUtils.Preferences.BypassNotification);
- Services.prefs.clearUserPref("termsofuse.acceptedDate");
- Services.prefs.clearUserPref("termsofuse.acceptedVersion");
- TelemetryReportingPolicy.reset();
- };
-
- Services.prefs.setBoolPref(
- TelemetryUtils.Preferences.BypassNotification,
- false
- );
-
- TelemetryReportingPolicy.reset();
-
- Assert.ok(
- !TelemetryReportingPolicy.canUpload(),
- "Before accepting TOU legacy upload is blocked"
- );
-
- Assert.ok(
- !Services.prefs.getIntPref(
- "datareporting.policy.dataSubmissionPolicyAcceptedVersion",
- 0
- ),
- "Legacy policy flow accepted version not set"
- );
-
- Assert.ok(
- !Services.prefs.getBoolPref(
- "datareporting.policy.dataSubmissionPolicyNotifiedTime",
- 0
- ),
- "Legacy policy flow accepted date not set"
- );
-
- Services.prefs.setStringPref("termsofuse.acceptedDate", String(Date.now()));
- Services.prefs.setIntPref(
- "termsofuse.acceptedVersion",
- Services.prefs.getIntPref(TOU_CURRENT_VERSION_PREF, 4)
- );
-
- TelemetryReportingPolicy.reset();
-
- Assert.ok(
- TelemetryReportingPolicy.userHasAcceptedTOU(),
- "TOU acceptance is recorded"
- );
-
- Assert.ok(TelemetryReportingPolicy.canUpload(), "Legacy upload allowed");
-
- cleanup();
-});
-
-add_task(async function test_canUpload_allowed_when_both_bypass_prefs_true() {
- const cleanup = () => {
- Services.prefs.clearUserPref(TelemetryUtils.Preferences.BypassNotification);
- Services.prefs.clearUserPref("termsofuse.bypassNotification");
- Services.prefs.clearUserPref("browser.preonboarding.enabled");
- TelemetryReportingPolicy.reset();
- };
-
- Services.prefs.setBoolPref(
- TelemetryUtils.Preferences.BypassNotification,
- true
- );
- Services.prefs.setBoolPref("termsofuse.bypassNotification", true);
- Services.prefs.setBoolPref("browser.preonboarding.enabled", true);
-
- TelemetryReportingPolicy.reset();
- Assert.ok(
- TelemetryReportingPolicy.canUpload(),
- "Upload allowed when both legacy and TOU notification flows are bypassed"
- );
-
- Services.prefs.setBoolPref(
- TelemetryUtils.Preferences.BypassNotification,
- true
- );
- Services.prefs.setBoolPref("termsofuse.bypassNotification", false);
- Services.prefs.setBoolPref("browser.preonboarding.enabled", true);
- Services.prefs.setBoolPref("browser.preonboarding.screens", []);
-
- TelemetryReportingPolicy.reset();
- Assert.ok(
- !TelemetryReportingPolicy.canUpload(),
- "Upload not allowed when only legacy bypass is true and TOU should show"
- );
-
- Services.prefs.setBoolPref(
- TelemetryUtils.Preferences.BypassNotification,
- false
- );
- Services.prefs.setBoolPref("termsofuse.bypassNotification", true);
- // Force TOU enabled so we’re not falling back to legacy notification (on
- // Linux, this is false by default).
- Services.prefs.setBoolPref("browser.preonboarding.enabled", true);
-
- TelemetryReportingPolicy.reset();
- Assert.ok(
- !TelemetryReportingPolicy.canUpload(),
- "Upload blocked when only TOU bypass is true and TOU should show"
- );
-
- cleanup();
-});
-
add_task(
- async function test_canUpload_does_not_reconfigure_when_nimbus_ready() {
+ async function test_canUpload_unblocked_by_tou_accepted_and_uploadEnabled() {
if (AppConstants.platform === "linux") {
- info("Skipping on Linux where TOU flow is disabled by default");
+ info("Skipping test for Linux where TOU flow is disabled by default");
return;
}
-
TelemetryReportingPolicy.reset();
- const cleanup = () => {
- Services.prefs.clearUserPref("browser.preonboarding.enabled");
+ registerCleanupFunction(() => {
Services.prefs.clearUserPref(
TelemetryUtils.Preferences.BypassNotification
);
- Services.prefs.clearUserPref(TOU_BYPASS_NOTIFICATION_PREF);
- Services.prefs.clearUserPref(TOU_ACCEPTED_VERSION_PREF);
- Services.prefs.clearUserPref(TOU_ACCEPTED_DATE_PREF);
+ Services.prefs.clearUserPref("datareporting.healthreport.uploadEnabled");
+ Services.prefs.clearUserPref("termsofuse.acceptedDate");
+ Services.prefs.clearUserPref("termsofuse.acceptedVersion");
TelemetryReportingPolicy.reset();
- sinon.restore();
- };
+ });
Services.prefs.setBoolPref(
TelemetryUtils.Preferences.BypassNotification,
false
);
- Services.prefs.setBoolPref(TOU_BYPASS_NOTIFICATION_PREF, false);
- // Enroll so that _configureFromNimbus has something to read during startup.
- const unenroll = await NimbusTestUtils.enrollWithFeatureConfig(
- {
- featureId: NimbusFeatures.preonboarding.featureId,
- value: {
- enabled: true,
- currentVersion: 4,
- minimumVersion: 4,
- screens: [{ id: "test" }],
- },
- },
- { isRollout: false }
+ // This is true by default for most users
+ Services.prefs.setBoolPref(
+ "datareporting.healthreport.uploadEnabled",
+ true
);
- // Simulate startup, this triggers the one-time _configureFromNimbus().
- await Policy.fakeSessionRestoreNotification();
+ Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "Before accepting TOU legacy upload is blocked"
+ );
- const getVarsStub = sinon
- .stub(NimbusFeatures.preonboarding, "getAllVariables")
- .callsFake(() => {
- throw new Error(
- "getAllVariables must not be called by canUpload() when already configured"
- );
- });
- TelemetryReportingPolicy.reset();
+ Assert.ok(
+ !Services.prefs.getIntPref(
+ "datareporting.policy.dataSubmissionPolicyAcceptedVersion",
+ 0
+ ),
+ "Legacy policy flow accepted version not set"
+ );
- TelemetryReportingPolicy.canUpload();
+ Assert.ok(
+ !Services.prefs.getBoolPref(
+ "datareporting.policy.dataSubmissionPolicyNotifiedDate",
+ 0
+ ),
+ "Legacy policy flow accepted date not set"
+ );
- Assert.equal(
- getVarsStub.callCount,
- 0,
- "canUpload() should not re-read Nimbus variables when already configured"
+ Services.prefs.setStringPref("termsofuse.acceptedDate", String(Date.now()));
+ Services.prefs.setIntPref(
+ "termsofuse.acceptedVersion",
+ Services.prefs.getIntPref(TOU_CURRENT_VERSION_PREF, 4)
+ );
+
+ Assert.ok(
+ TelemetryReportingPolicy.userHasAcceptedTOU(),
+ "TOU acceptance is recorded"
+ );
+
+ Assert.ok(TelemetryReportingPolicy.canUpload(), "Legacy upload allowed");
+
+ Services.prefs.setBoolPref(
+ "datareporting.healthreport.uploadEnabled",
+ false
);
- await unenroll();
- cleanup();
+ Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "Legacy upload blocked when user opts out of sharing interaction data"
+ );
}
);
-add_task(async function test_canUpload_reconfigures_when_nimbus_not_ready() {
- const cleanup = () => {
- Services.prefs.clearUserPref("browser.preonboarding.enabled");
+add_task(async function test_canUpload_allowed_when_both_bypass_prefs_true() {
+ TelemetryReportingPolicy.reset();
+
+ registerCleanupFunction(() => {
Services.prefs.clearUserPref(TelemetryUtils.Preferences.BypassNotification);
- Services.prefs.clearUserPref(TOU_BYPASS_NOTIFICATION_PREF);
- Services.prefs.clearUserPref(TOU_ACCEPTED_VERSION_PREF);
- Services.prefs.clearUserPref(TOU_ACCEPTED_DATE_PREF);
- sinon.restore();
+ Services.prefs.clearUserPref("termsofuse.bypassNotification");
+ Services.prefs.clearUserPref("browser.preonboarding.enabled");
TelemetryReportingPolicy.reset();
- };
+ });
- // Force a path where canUpload() must consult Nimbus
- Services.prefs.setBoolPref("browser.preonboarding.enabled", true);
- Services.prefs.setBoolPref(TOU_BYPASS_NOTIFICATION_PREF, false);
Services.prefs.setBoolPref(
TelemetryUtils.Preferences.BypassNotification,
true
);
+ Services.prefs.setBoolPref("termsofuse.bypassNotification", true);
- const getVarsSpy = sinon
- .stub(NimbusFeatures.preonboarding, "getAllVariables")
- .returns({ enabled: false });
-
- TelemetryReportingPolicy.reset();
+ Assert.ok(
+ TelemetryReportingPolicy.canUpload(),
+ "Upload allowed when both legacy and TOU notification flows are bypassed"
+ );
- const result = TelemetryReportingPolicy.canUpload();
+ Services.prefs.setBoolPref("termsofuse.bypassNotification", false);
- Assert.equal(
- getVarsSpy.callCount,
- 1,
- "canUpload() should trigger _configureFromNimbus() when variables are not yet set"
+ Services.prefs.setBoolPref("browser.preonboarding.enabled", true);
+ Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "Upload blocked when only one bypass is true and no other allow path conditions are met"
);
-
+ Services.prefs.setBoolPref("browser.preonboarding.enabled", false);
Assert.ok(
- result,
- "With Nimbus enabled=false, TOU is off; legacy bypass allows upload"
+ TelemetryReportingPolicy.canUpload(),
+ "Upload NOT blocked when only one bypass is true and no other allow path conditions are met"
);
-
- cleanup();
});
add_task(
- skipIfNotBrowser(),
- async function test_legacy_bypass_blocked_when_TOU_should_show_and_not_accepted() {
- const cleanup = async () => {
- Services.prefs.clearUserPref("browser.preonboarding.enabled");
+ async function test_canUpload_allowed_when_tou_bypass_and_legacy_notified() {
+ TelemetryReportingPolicy.reset();
+
+ registerCleanupFunction(() => {
Services.prefs.clearUserPref(
TelemetryUtils.Preferences.BypassNotification
);
- Services.prefs.clearUserPref(TOU_BYPASS_NOTIFICATION_PREF);
- Services.prefs.clearUserPref(TOU_ACCEPTED_VERSION_PREF);
- Services.prefs.clearUserPref(TOU_ACCEPTED_DATE_PREF);
+ Services.prefs.clearUserPref("termsofuse.bypassNotification");
Services.prefs.clearUserPref(
- "datareporting.policy.dataSubmissionPolicyNotifiedTime"
+ "datareporting.policy.dataSubmissionPolicyNotifiedDate"
);
Services.prefs.clearUserPref(
- "datareporting.policy.dataSubmissionPolicyAcceptedVersion"
+ "TelemetryUtils.Preferences.DataSubmissionEnabled"
);
TelemetryReportingPolicy.reset();
- await unenroll();
- };
+ });
- // Legacy bypass ON, TOU bypass OFF
+ Services.prefs.setBoolPref("termsofuse.bypassNotification", true);
Services.prefs.setBoolPref(
TelemetryUtils.Preferences.BypassNotification,
- true
- );
- Services.prefs.setBoolPref(TOU_BYPASS_NOTIFICATION_PREF, false);
-
- // User has NOT accepted TOU
- Services.prefs.clearUserPref(TOU_ACCEPTED_VERSION_PREF);
- Services.prefs.clearUserPref(TOU_ACCEPTED_DATE_PREF);
-
- // User has NOT accepted via Legacy flow
- Services.prefs.clearUserPref(
- "datareporting.policy.dataSubmissionPolicyNotifiedTime"
+ false
);
- Services.prefs.clearUserPref(
- "datareporting.policy.dataSubmissionPolicyAcceptedVersion"
- );
-
- // Make _shouldShowTOU() return true
- Services.prefs.setBoolPref("browser.preonboarding.enabled", true);
- const unenroll = await NimbusTestUtils.enrollWithFeatureConfig(
- {
- featureId: NimbusFeatures.preonboarding.featureId,
- value: {
- enabled: true,
- currentVersion: 4,
- minimumVersion: 4,
- screens: [{ id: "test" }],
- },
- },
- { isRollout: false }
+ Services.prefs.setBoolPref(
+ TelemetryUtils.Preferences.DataSubmissionEnabled,
+ true
);
+ Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "Upload not allowed when TOU bypass is true, legacy bypass is false, and legacy policy has NOT been notified"
+ );
+ // Mark legacy “notified” via the actual code path triggered via
+ // testInfobarShown. This calls _recordNotificationData, which updates the
+ // legacy notification accepted time and version prefs. It also calls
+ // _userNotified which in turn calls lazy.TelemetrySend.notifyCanUpload.
+ // This simulates the user being notified of the legacy infobar flow.
+ TelemetryReportingPolicy.testInfobarShown();
+ // Ensure the policy re-reads state after pref mutations.
TelemetryReportingPolicy.reset();
- const result = TelemetryReportingPolicy.canUpload();
-
Assert.ok(
- !result,
- "When TOU flow should be shown, but is not yet accepted with legacy bypass true, (!shouldShowTOU && bypassLegacy) is false, so upload is blocked."
+ TelemetryReportingPolicy.canUpload(),
+ "Upload allowed when TOU bypass is true, legacy bypass is false, and legacy policy has been notified"
);
-
- await cleanup();
}
);
diff --git a/toolkit/crashreporter/test/unit/test_crashreporter_crash.js b/toolkit/crashreporter/test/unit/test_crashreporter_crash.js
@@ -113,7 +113,6 @@ add_task(async function run_test() {
// Enable the FHR, official policy bypass (since we're in a test) and
// specify a telemetry server & client ID.
Services.prefs.setBoolPref("termsofuse.bypassNotification", true);
- Services.prefs.setBoolPref("browser.preonboarding.enabled", false);
Services.prefs.setBoolPref(
"datareporting.policy.dataSubmissionPolicyBypassNotification",
true
