From c7766ffb33511ddb61098123c330b490905892b9 Mon Sep 17 00:00:00 2001
From: Martin Giger <martin@humanoids.be>
Date: Thu, 21 Nov 2024 15:45:14 +0000
Subject: [PATCH] Bug 1931782 - Format in-app notification CTA URLs with the
 URL formatter. r=arschmitz

Differential Revision: https://phabricator.services.mozilla.com/D229790

--HG--
extra : moz-landing-system : lando
---
 .../modules/NotificationManager.sys.mjs       |  5 ++-
 .../test/unit/test_notification_manager.js    | 42 ++++++++++++++++++-
 2 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/mail/components/inappnotifications/modules/NotificationManager.sys.mjs b/mail/components/inappnotifications/modules/NotificationManager.sys.mjs
index 806427bc50..3922da06cb 100644
--- a/mail/components/inappnotifications/modules/NotificationManager.sys.mjs
+++ b/mail/components/inappnotifications/modules/NotificationManager.sys.mjs
@@ -137,7 +137,10 @@ export class NotificationManager extends EventTarget {
         detail: notificationId,
       })
     );
-    lazy.openLinkExternally(this.#currentNotification.URL);
+    const formattedURL = Services.urlFormatter.formatURL(
+      this.#currentNotification.URL
+    );
+    lazy.openLinkExternally(formattedURL);
     Glean.inappnotifications.interaction.record({
       notification_id: notificationId,
       active_this_session: this.#getActiveNotificationDuration(),
diff --git a/mail/components/inappnotifications/test/unit/test_notification_manager.js b/mail/components/inappnotifications/test/unit/test_notification_manager.js
index b3f0f3b19a..e8c40a4ceb 100644
--- a/mail/components/inappnotifications/test/unit/test_notification_manager.js
+++ b/mail/components/inappnotifications/test/unit/test_notification_manager.js
@@ -46,6 +46,7 @@ function getMockNotifications() {
 }
 
 let didOpen = false;
+let expectedURI = "about:blank";
 
 add_setup(async function () {
   // PlacesUtils when executing the CTA needs the profile.
@@ -59,7 +60,7 @@ add_setup(async function () {
       didOpen = true;
       Assert.equal(
         uri.spec,
-        "about:blank",
+        expectedURI,
         "Should only receive about blank load request"
       );
     },
@@ -431,3 +432,42 @@ add_task(async function test_newNotificationReemit_handleEvent() {
   );
   notificationManager.updatedNotifications([]);
 });
+
+add_task(async function test_executeNotificationCTA_formatURL() {
+  didOpen = false;
+  const notificationManager = new NotificationManager();
+  const mockNotifications = getMockNotifications();
+  const url = "https://example.com/%LOCALE%/file.json";
+  mockNotifications[1].URL = url;
+  expectedURI = Services.urlFormatter.formatURL(url);
+  notificationManager.updatedNotifications(mockNotifications);
+  const { detail: notification } = await BrowserTestUtils.waitForEvent(
+    notificationManager,
+    NotificationManager.NEW_NOTIFICATION_EVENT
+  );
+  Assert.equal(notification.id, "bar", "Should pick the second notification");
+
+  const notificationInteractionEvent = BrowserTestUtils.waitForEvent(
+    notificationManager,
+    NotificationManager.NOTIFICATION_INTERACTION_EVENT
+  );
+  const clearNotificationEvent = BrowserTestUtils.waitForEvent(
+    notificationManager,
+    NotificationManager.CLEAR_NOTIFICATION_EVENT
+  );
+  notificationManager.executeNotificationCTA(notification.id);
+  const { detail: notificationId } = await notificationInteractionEvent;
+  Assert.equal(
+    notificationId,
+    notification.id,
+    "Should have interacted with the notification"
+  );
+  Assert.ok(didOpen, "Should open URL externally");
+  await clearNotificationEvent;
+  await BrowserTestUtils.waitForEvent(
+    notificationManager,
+    NotificationManager.REQUEST_NOTIFICATIONS_EVENT
+  );
+
+  notificationManager.updatedNotifications([]);
+});