commit 63cefc91aaeaaa27a1ca443c5a7ee9b4da24131d
parent 5b13ebca0cddb9b98a46e6e938fdfbc83aecb5d5
Author: pstanciu <pstanciu@mozilla.com>
Date: Sat, 11 Oct 2025 05:10:42 +0300
Revert "Bug 1993295 - Allow legacy telemetry upload for users who predate the new user TOU experience r=toolkit-telemetry-reviewers,dmose,TravisLong" for causing xpc failure @test_event_files.js
This reverts commit 5b13ebca0cddb9b98a46e6e938fdfbc83aecb5d5.
Diffstat:
2 files changed, 19 insertions(+), 227 deletions(-)
diff --git a/toolkit/components/telemetry/app/TelemetryReportingPolicy.sys.mjs b/toolkit/components/telemetry/app/TelemetryReportingPolicy.sys.mjs
@@ -761,12 +761,12 @@ var TelemetryReportingPolicyImpl = {
* Prerequisite: data submission is enabled (this.dataSubmissionEnabled).
*
* In order to submit data, at least ONE of these conditions should be true:
- * 1. Terms of Use flow is bypassed
- * OR
- * Legacy policy has been bypassed AND user does not qualify to see the
- * TOU flow
- * 2. Legacy policy has been accepted AND user does not qualify to see the
- * TOU flow
+ * 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).
@@ -787,52 +787,25 @@ var TelemetryReportingPolicyImpl = {
// (disabled by default for Linux via the fallback
// browser.preonboarding.enabled pref) or if the explicit bypass pref is
// set.
- const bypassTOUFlowViaPref = Services.prefs.getBoolPref(
- TOU_BYPASS_NOTIFICATION_PREF,
- false
- );
- // In some cases, canUpload 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 (
- // Only bother configuring from nimbus if not already bypassing via pref
- !bypassTOUFlowViaPref &&
- (!this._nimbusVariables ||
- (typeof this._nimbusVariables === "object" &&
- Object.keys(this._nimbusVariables).length === 0))
- ) {
- this._configureFromNimbus();
- }
-
- const bypassTOUFlowViaNimbusVariables =
+ 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 bypassTOUFlow =
- bypassTOUFlowViaPref || bypassTOUFlowViaNimbusVariables;
-
const allowInteractionData = Services.prefs.getBoolPref(
"datareporting.healthreport.uploadEnabled",
false
);
// Condition 1
- const bypassFlow =
- bypassTOUFlow || (!this._shouldShowTOU() && bypassLegacyFlow);
+ const canUploadBypassLegacyAndTOU = bypassLegacyFlow && bypassTOUFlow;
// Condition 2
- // Note that isUserNotifiedOfCurrentPolicy should really be named
- // hasUserAcceptedLegacyPolicy as it returns true if the use has accepted
- // (been notified of) the legacy policy
const canUploadLegacy =
- !this._shouldShowTOU() && this.isUserNotifiedOfCurrentPolicy;
+ bypassTOUFlow && !bypassLegacyFlow && this.isUserNotifiedOfCurrentPolicy;
// Condition 3
const canUploadTOU = this.hasUserAcceptedCurrentTOU && allowInteractionData;
- return bypassFlow || canUploadLegacy || canUploadTOU;
+ return canUploadBypassLegacyAndTOU || canUploadLegacy || canUploadTOU;
},
isFirstRun() {
@@ -1123,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.
@@ -1260,12 +1233,13 @@ 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;
}
this._nimbusVariables = lazy.NimbusFeatures.preonboarding.getAllVariables();
+
if (this._nimbusVariables.enabled === null) {
const preonboardingMessage =
lazy.OnboardingMessageProvider.getPreonboardingMessages().find(
diff --git a/toolkit/components/telemetry/tests/unit/test_TOUNotificationFlow.js b/toolkit/components/telemetry/tests/unit/test_TOUNotificationFlow.js
@@ -775,8 +775,8 @@ add_task(async function test_canUpload_allowed_when_both_bypass_prefs_true() {
Services.prefs.setBoolPref("browser.preonboarding.enabled", true);
Assert.ok(
- TelemetryReportingPolicy.canUpload(),
- "Upload allowed when only legacy bypass is true"
+ !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(
@@ -786,7 +786,7 @@ add_task(async function test_canUpload_allowed_when_both_bypass_prefs_true() {
});
add_task(
- async function test_canUpload_allowed_under_various_bypass_conditions() {
+ async function test_canUpload_allowed_when_tou_bypass_and_legacy_notified() {
TelemetryReportingPolicy.reset();
registerCleanupFunction(() => {
@@ -814,8 +814,8 @@ add_task(
);
Assert.ok(
- TelemetryReportingPolicy.canUpload(),
- "Upload allowed when TOU bypass is true even if legacy bypass is false and legacy policy has NOT been notified"
+ !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
@@ -832,185 +832,3 @@ add_task(
);
}
);
-
-add_task(
- async function test_canUpload_does_not_reconfigure_when_nimbus_ready() {
- if (AppConstants.platform === "linux") {
- info("Skipping on Linux where TOU flow is disabled by default");
- return;
- }
-
- TelemetryReportingPolicy.reset();
-
- registerCleanupFunction(() => {
- Services.prefs.clearUserPref("browser.preonboarding.enabled");
- Services.prefs.clearUserPref("datareporting.healthreport.uploadEnabled");
- 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);
- TelemetryReportingPolicy.reset();
- });
-
- Services.prefs.setBoolPref(
- TelemetryUtils.Preferences.BypassNotification,
- false
- );
- Services.prefs.setBoolPref(TOU_BYPASS_NOTIFICATION_PREF, false);
-
- Services.prefs.setBoolPref(
- "datareporting.healthreport.uploadEnabled",
- true
- );
-
- // 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 }
- );
-
- // Simulate startup, this triggers the one-time _configureFromNimbus().
- await Policy.fakeSessionRestoreNotification();
-
- const getVarsStub = sinon
- .stub(NimbusFeatures.preonboarding, "getAllVariables")
- .callsFake(() => {
- throw new Error(
- "getAllVariables must not be called by canUpload() when already configured"
- );
- });
-
- TelemetryReportingPolicy.canUpload();
-
- Assert.equal(
- getVarsStub.callCount,
- 0,
- "canUpload() should not re-read Nimbus variables when already configured"
- );
-
- await unenroll();
- sinon.restore();
- }
-);
-
-add_task(async function test_canUpload_reconfigures_when_nimbus_not_ready() {
- TelemetryReportingPolicy.reset();
-
- registerCleanupFunction(() => {
- Services.prefs.clearUserPref("browser.preonboarding.enabled");
- Services.prefs.clearUserPref("datareporting.healthreport.uploadEnabled");
- 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();
- TelemetryReportingPolicy.reset();
- });
-
- Services.prefs.setBoolPref(
- TelemetryUtils.Preferences.BypassNotification,
- false
- );
- Services.prefs.setBoolPref(TOU_BYPASS_NOTIFICATION_PREF, false);
- Services.prefs.setBoolPref("datareporting.healthreport.uploadEnabled", true);
-
- // Don’t trigger the startup config path.
-
- // Fallback pref disables TOU flow and Nimbus returns enabled as false
- Services.prefs.setBoolPref("browser.preonboarding.enabled", false);
-
- const getVarsSpy = sinon
- .stub(NimbusFeatures.preonboarding, "getAllVariables")
- .returns({ enabled: false });
-
- const result = TelemetryReportingPolicy.canUpload();
-
- Assert.equal(
- getVarsSpy.callCount,
- 1,
- "canUpload() should trigger _configureFromNimbus() when variables are not yet set"
- );
-
- Assert.ok(
- result,
- "With Nimbus enabled set as false we are bypassing TOU and should allow upload"
- );
-
- sinon.restore();
-});
-
-add_task(
- skipIfNotBrowser(),
- async function test_legacy_bypass_blocked_when_TOU_should_show_and_not_accepted() {
- TelemetryReportingPolicy.reset();
-
- registerCleanupFunction(async () => {
- Services.prefs.clearUserPref("browser.preonboarding.enabled");
- 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.policy.dataSubmissionPolicyNotifiedDate"
- );
- Services.prefs.clearUserPref(
- "datareporting.policy.dataSubmissionPolicyAcceptedVersion"
- );
- TelemetryReportingPolicy.reset();
- await unenroll();
- });
-
- // Legacy bypass ON, TOU bypass OFF
- 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.dataSubmissionPolicyNotifiedDate"
- );
- 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 }
- );
-
- 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."
- );
- }
-);
