commit 51bb09d070987c87690fb57de9f171b36e17789b
parent 84504d1bb6af26fbc84d8eff7aa33c4d7cf3e921
Author: Meg Viar <lmegviar@gmail.com>
Date: Thu, 16 Oct 2025 21:32:43 +0000
Bug 1994735 - Don't allow legacy telemetry upload if a notification if currently in progress r=toolkit-telemetry-reviewers,dmose,janerik
Differential Revision: https://phabricator.services.mozilla.com/D268883
Diffstat:
2 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/toolkit/components/telemetry/app/TelemetryReportingPolicy.sys.mjs b/toolkit/components/telemetry/app/TelemetryReportingPolicy.sys.mjs
@@ -221,6 +221,13 @@ export var TelemetryReportingPolicy = {
},
/**
+ * Test only method, used simulate a notification being in-progress.
+ */
+ testNotificationInProgress(inProgress) {
+ TelemetryReportingPolicyImpl._notificationInProgress = inProgress;
+ },
+
+ /**
* Test only method, used to get TOS on-train release dates by channel.
*/
get fullOnTrainReleaseDates() {
@@ -760,9 +767,12 @@ 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.
+ * If a notification is currently in progress, the user should not be allowed
+ * to upload data.
+ *
+ * Otherwise, 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.
* @return {Boolean} True if we are allowed to upload data, false otherwise.
*/
canUpload() {
@@ -772,6 +782,15 @@ var TelemetryReportingPolicyImpl = {
return false;
}
+ // If a notification is in progress, such as when the TOU modal is currently
+ // showing and user has not yet accepted, do not allow upload. The legacy
+ // flow doesn't require interaction, so the notification is only considered
+ // to be in progress for the brief period between when the infobar or
+ // privacy notice tab is shown and when the notification is recorded.
+ if (this._notificationInProgress) {
+ return false;
+ }
+
return !this._shouldNotifyDataReportingPolicy() && !this._shouldShowTOU();
},
diff --git a/toolkit/components/telemetry/tests/unit/test_TOUNotificationFlow.js b/toolkit/components/telemetry/tests/unit/test_TOUNotificationFlow.js
@@ -43,6 +43,8 @@ function fakeResetAcceptedPolicy() {
// Fake dismissing a modal dialog.
function fakeInteractWithModal() {
Services.obs.notifyObservers(null, "termsofuse:interacted");
+ // Mark that notification is no longer in progress
+ TelemetryReportingPolicy.testNotificationInProgress(false);
}
function unsetMinimumPolicyVersion() {
@@ -123,7 +125,7 @@ add_setup(() => {
"datareporting.policy.dataSubmissionPolicyAcceptedVersion"
);
Services.prefs.clearUserPref(TelemetryUtils.Preferences.BypassNotification);
-
+ TelemetryReportingPolicy.testNotificationInProgress(false);
TelemetryReportingPolicy.reset();
});
});
@@ -575,13 +577,21 @@ add_task(
let p = Policy.delayedSetup();
Policy.fakeSessionRestoreNotification();
+ // Mark that notification is in progress
+ TelemetryReportingPolicy.testNotificationInProgress(true);
// Spin the event loop once – the notification should *not* have fired yet.
await TestUtils.waitForTick();
+
Assert.ok(
!notificationSeen,
"Notification should not be dispatched before the user interacts"
);
+ Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "canUpload() is false while TOU modal is showing (notification in progress)"
+ );
+
Assert.equal(
modalStub.callCount,
1,
@@ -593,12 +603,18 @@ add_task(
await p;
Assert.ok(
+ TelemetryReportingPolicy.canUpload(),
+ "canUpload() is true after the user accepts the TOU via the modal"
+ );
+
+ Assert.ok(
notificationSeen,
"Notification fires after the user accepts the ToU in this session"
);
// Clean-up.
fakeResetAcceptedPolicy();
+ TelemetryReportingPolicy.testNotificationInProgress(false);
await doCleanup();
sinon.restore();
}
@@ -622,6 +638,8 @@ add_task(
let p = Policy.delayedSetup();
Policy.fakeSessionRestoreNotification();
+ // Mark that notification is in progress
+ TelemetryReportingPolicy.testNotificationInProgress(true);
// Spin the event loop once – the notification should *not* have fired yet.
await TestUtils.waitForTick();
Assert.ok(
@@ -635,10 +653,20 @@ add_task(
"showModal should be invoked exactly once when prompting the user"
);
+ Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "canUpload() is false while TOU modal is showing (notification in progress)"
+ );
+
await TestUtils.waitForTick();
await p;
Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "canUpload() remains false if the user never accepts and no bypass applies"
+ );
+
+ Assert.ok(
!notificationSeen,
"Notification should still not be dispatched if never interacted with"
);
@@ -646,6 +674,7 @@ add_task(
// Clean-up.
fakeResetAcceptedPolicy();
await doCleanup();
+ TelemetryReportingPolicy.testNotificationInProgress(false);
sinon.restore();
}
);
@@ -668,6 +697,8 @@ add_task(
let p = Policy.delayedSetup();
Policy.fakeSessionRestoreNotification();
+ // Mark that notification is in progress
+ TelemetryReportingPolicy.testNotificationInProgress(true);
// Spin the event loop once – the notification should *not* have fired yet.
await TestUtils.waitForTick();
Assert.ok(
@@ -681,6 +712,11 @@ add_task(
"showModal should be invoked exactly once when prompting the user"
);
+ Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "canUpload() is false while modal is displayed"
+ );
+
await TestUtils.waitForTick();
await p;
@@ -688,16 +724,28 @@ add_task(
!notificationSeen,
"Notification should still not be dispatched if never interacted with"
);
+
+ Assert.ok(
+ !TelemetryReportingPolicy.canUpload(),
+ "canUpload() remains false"
+ );
+
fakeInteractWithModal();
await TestUtils.waitForTick();
Assert.ok(
+ TelemetryReportingPolicy.canUpload(),
+ "canUpload() becomes true after later acceptance"
+ );
+
+ Assert.ok(
notificationSeen,
"Notification fires after the user accepts the ToU in this session"
);
// Clean-up.
fakeResetAcceptedPolicy();
+ TelemetryReportingPolicy.testNotificationInProgress(false);
await doCleanup();
sinon.restore();
}
