Bug 1962454 - Pass origin instead of host/port to Windows notification r=nalexander,win-reviewers,firefox-desktop-core-reviewers ,gstoll

This is to match NotificationDB that uses origin.

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

browser/components/BrowserContentHandler.sys.mjs

@@ -1394,11 +1394,16 @@ nsDefaultCommandLineHandler.prototype = {
           // window to perform the action in.
           let winForAction;
 
-          if (
+          // Fall back to launchUrl to not break notifications opened from
-            !tagWasHandled &&
+          // previous builds after browser updates, as such notification would
-            notificationData?.launchUrl &&
+          // still have the old field.
-            !opaqueRelaunchData
+          let origin = notificationData?.origin ?? notificationData?.launchUrl;
-          ) {
+
+          if (!tagWasHandled && origin && !opaqueRelaunchData) {
+            let originPrincipal =
+              Services.scriptSecurityManager.createContentPrincipalFromOrigin(
+                origin
+              );
             // Unprivileged Web Notifications contain a launch URL and are
             // handled slightly differently than privileged notifications with
             // actions. If the tag was not handled, then the notification was
@@ -1406,7 +1411,9 @@ nsDefaultCommandLineHandler.prototype = {
             // fallback behavior.
             let { uri, principal } = resolveURIInternal(
               cmdLine,
-              notificationData.launchUrl
+              // TODO(krosylight): We should handle origin suffix to open the
+              // relevant container. See bug 1945501.
+              originPrincipal.originNoSuffix
             );
             if (cmdLine.state != Ci.nsICommandLine.STATE_INITIAL_LAUNCH) {
               // Try to find an existing window and load our URI into the current

browser/components/tests/browser/browser.toml

@@ -36,6 +36,9 @@ run-if = ["os == 'win'"]
 
 ["browser_forced_colors.js"]
 
+["browser_handle_notification.js"]
+run-if = ["os == 'win'"]
+
 ["browser_initial_tab_remoteType.js"]
 https_first_disabled = true
 

browser/components/tests/browser/browser_handle_notification.js

@@ -0,0 +1,127 @@
+/* Any copyright is dedicated to the Public Domain.
+http://creativecommons.org/publicdomain/zero/1.0/ */
+"use strict";
+
+function createCmdLine(tag, action, state) {
+  return Cu.createCommandLine(
+    [
+      "--notification-windowsTag",
+      tag,
+      "--notification-windowsAction",
+      JSON.stringify(action),
+    ],
+    null,
+    state
+  );
+}
+
+function runCmdLine(cmdLine) {
+  let cmdLineHandler = Cc["@mozilla.org/browser/final-clh;1"].getService(
+    Ci.nsICommandLineHandler
+  );
+  cmdLineHandler.handle(cmdLine);
+}
+
+function simulateNotificationClickWithExistingWindow(action) {
+  let cmdLine = createCmdLine(
+    "dummyTag",
+    action,
+    Ci.nsICommandLine.STATE_REMOTE_AUTO
+  );
+  runCmdLine(cmdLine);
+}
+
+function simulateNotificationClickWithNewWindow(action) {
+  let cmdLine = createCmdLine(
+    "dummyTag",
+    action,
+    Ci.nsICommandLine.STATE_INITIAL_LAUNCH
+  );
+  runCmdLine(cmdLine);
+}
+
+add_task(async function test_basic() {
+  let newTabPromise = BrowserTestUtils.waitForNewTab(
+    gBrowser,
+    "https://example.com/"
+  );
+
+  simulateNotificationClickWithExistingWindow({
+    action: "",
+    origin: "https://example.com",
+  });
+
+  let newTab = await newTabPromise;
+  ok(newTab, "New tab should be opened.");
+  BrowserTestUtils.removeTab(newTab);
+});
+
+// launchUrl was used pre-140, we can remove it when we are confident enough
+// that there's no old notification with launchUrl lying around anymore
+add_task(async function test_legacy_launchUrl() {
+  let newTabPromise = BrowserTestUtils.waitForNewTab(
+    gBrowser,
+    "https://example.com/"
+  );
+
+  simulateNotificationClickWithExistingWindow({
+    action: "",
+    launchUrl: "https://example.com",
+  });
+
+  let newTab = await newTabPromise;
+  ok(newTab, "New tab should be opened.");
+  BrowserTestUtils.removeTab(newTab);
+});
+
+add_task(async function test_invalid_origin_with_path() {
+  let newTabPromise = BrowserTestUtils.waitForNewTab(
+    gBrowser,
+    "https://example.com/"
+  );
+
+  simulateNotificationClickWithExistingWindow({
+    action: "",
+    origin: "https://example.com/example/",
+  });
+
+  let newTab = await newTabPromise;
+  ok(newTab, "New tab should be opened.");
+  BrowserTestUtils.removeTab(newTab);
+});
+
+add_task(async function test_user_context() {
+  let newTabPromise = BrowserTestUtils.waitForNewTab(
+    gBrowser,
+    "https://example.com/"
+  );
+
+  simulateNotificationClickWithExistingWindow({
+    action: "",
+    origin: "https://example.com^userContextId=1",
+  });
+
+  let newTab = await newTabPromise;
+  registerCleanupFunction(() => BrowserTestUtils.removeTab(newTab));
+  ok(newTab, "New tab should be opened.");
+
+  // TODO(krosylight): We want to make sure this opens on the right container.
+  // See bug 1945501.
+  is(newTab.userContextId, 0, "The default user context ID is used (for now).");
+});
+
+add_task(async function test_basic_initial_load() {
+  let newWinPromise = BrowserTestUtils.waitForNewWindow({
+    url: "https://example.com/",
+    anyWindow: true,
+  });
+
+  simulateNotificationClickWithNewWindow({
+    action: "",
+    origin: "https://example.com",
+  });
+
+  let newWin = await newWinPromise;
+  ok(newWin, "New window should be opened.");
+  BrowserTestUtils.closeWindow(newWin);
+});

toolkit/components/alerts/AlertNotification.cpp

@@ -238,6 +238,11 @@ AlertNotification::GetSource(nsAString& aSource) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+AlertNotification::GetOrigin(nsACString& aOrigin) {
+  return nsAlertsUtils::GetOrigin(mPrincipal, aOrigin);
+}
+
 NS_IMETHODIMP
 AlertNotification::GetOpaqueRelaunchData(nsAString& aOpaqueRelaunchData) {
   aOpaqueRelaunchData = mOpaqueRelaunchData;

toolkit/components/alerts/nsAlertsUtils.cpp

@@ -19,6 +19,7 @@ bool nsAlertsUtils::IsActionablePrincipal(nsIPrincipal* aPrincipal) {
 /* static */
 void nsAlertsUtils::GetSourceHostPort(nsIPrincipal* aPrincipal,
                                       nsAString& aHostPort) {
+  aHostPort.Truncate();
   if (!IsActionablePrincipal(aPrincipal)) {
     return;
   }
@@ -28,3 +29,13 @@ void nsAlertsUtils::GetSourceHostPort(nsIPrincipal* aPrincipal,
   }
   CopyUTF8toUTF16(hostPort, aHostPort);
 }
+
+/* static */
+nsresult nsAlertsUtils::GetOrigin(nsIPrincipal* aPrincipal,
+                                  nsACString& aOrigin) {
+  aOrigin.SetIsVoid(true);
+  if (!IsActionablePrincipal(aPrincipal)) {
+    return NS_OK;
+  }
+  return aPrincipal->GetOrigin(aOrigin);
+}

toolkit/components/alerts/nsAlertsUtils.h

@@ -25,5 +25,11 @@ class nsAlertsUtils final {
    * empty string if |aPrincipal| is not actionable.
    */
   static void GetSourceHostPort(nsIPrincipal* aPrincipal, nsAString& aHostPort);
+
+  /**
+   * Sets |aOrigin| to the origin from |aPrincipal|, or an error if |aPrincipal|
+   * is not actionable.
+   */
+  static nsresult GetOrigin(nsIPrincipal* aPrincipal, nsACString& aOrigin);
 };
 #endif /* nsAlertsUtils_h */

toolkit/components/alerts/nsIAlertsService.idl

@@ -219,6 +219,12 @@ interface nsIAlertNotification : nsISupports
    */
   readonly attribute AString source;
 
+  /**
+   * The origin of the originating page, or an empty string if the alert is not
+   * actionable. This corresponds to `nsIPrincipal.origin`.
+   */
+  readonly attribute ACString origin;
+
   /**
    * On Windows, chrome-privileged notifications -- i.e., those with a
    * non-actionable principal -- can have `opaqueRelaunchData`.  This data will

widget/windows/ToastNotificationHandler.cpp

@@ -342,8 +342,10 @@ nsString ToastNotificationHandler::ActionArgsJSONString(
       w.StringProperty("privilegedName", NS_ConvertUTF16toUTF8(mName));
     }
   } else {
-    if (!mHostPort.IsEmpty()) {
+    nsAutoCString origin;
-      w.StringProperty("launchUrl", NS_ConvertUTF16toUTF8(mHostPort));
+    nsresult rv = mAlertNotification->GetOrigin(origin);
+    if (NS_SUCCEEDED(rv) && !origin.IsVoid()) {
+      w.StringProperty("origin", origin);
     }
   }
 

widget/windows/tests/unit/test_windows_alert_service.js

@@ -195,7 +195,7 @@ function testAlert(when, { serverEnabled, profD, isBackgroundTaskMode } = {}) {
     ? ""
     : `<action content="Notification settings"/>`;
 
-  let parsedSettingsAction = hostport => {
+  let parsedSettingsAction = origin => {
     if (isBackgroundTaskMode) {
       return [];
     }
@@ -209,8 +209,8 @@ function testAlert(when, { serverEnabled, profD, isBackgroundTaskMode } = {}) {
             {
               action: "settings",
             },
-            hostport && {
+            origin && {
-              launchUrl: hostport,
+              origin,
             }
           )
         ),
@@ -219,7 +219,7 @@ function testAlert(when, { serverEnabled, profD, isBackgroundTaskMode } = {}) {
     ];
   };
 
-  let parsedSnoozeAction = hostport => {
+  let parsedSnoozeAction = (hostport, origin) => {
     let content = `Disable notifications from ${hostport}`;
     return [
       content,
@@ -230,8 +230,8 @@ function testAlert(when, { serverEnabled, profD, isBackgroundTaskMode } = {}) {
             {
               action: "snooze",
             },
-            hostport && {
+            origin && {
-              launchUrl: hostport,
+              origin,
             }
           )
         ),
@@ -414,8 +414,8 @@ function testAlert(when, { serverEnabled, profD, isBackgroundTaskMode } = {}) {
   );
 
   // But content unprivileged alerts can't use `windowsSystemActivationType`.
-  let launchUrl = "https://example.com/foo/bar.html";
+  let path = "https://example.com/foo/bar.html";
-  const principaluri = Services.io.newURI(launchUrl);
+  const principaluri = Services.io.newURI(path);
   const principal = Services.scriptSecurityManager.createContentPrincipal(
     principaluri,
     {}
@@ -436,19 +436,19 @@ function testAlert(when, { serverEnabled, profD, isBackgroundTaskMode } = {}) {
       {
         launch: parsedArgumentString({
           action: "",
-          launchUrl: principaluri.hostPort,
+          origin: principal.origin,
         }),
         actions: Object.fromEntries(
           [
-            parsedSnoozeAction(principaluri.hostPort),
+            parsedSnoozeAction(principal.hostPort, principal.origin),
-            parsedSettingsAction(principaluri.hostPort),
+            parsedSettingsAction(principal.origin),
             [
               "dismissTitle",
               {
                 content: "dismissTitle",
                 arguments: parsedArgumentString({
                   action: "dismiss",
-                  launchUrl: principaluri.hostPort,
+                  origin: principal.origin,
                 }),
               },
             ],
@@ -458,7 +458,7 @@ function testAlert(when, { serverEnabled, profD, isBackgroundTaskMode } = {}) {
                 content: "snoozeTitle",
                 arguments: parsedArgumentString({
                   action: "snooze",
-                  launchUrl: principaluri.hostPort,
+                  origin: principal.origin,
                 }),
               },
             ],
@@ -514,12 +514,12 @@ function testAlert(when, { serverEnabled, profD, isBackgroundTaskMode } = {}) {
       {
         launch: parsedArgumentString({
           action: "",
-          launchUrl: principaluri.hostPort,
+          origin: principal.origin,
         }),
         actions: Object.fromEntries(
           [
-            parsedSnoozeAction(principaluri.hostPort),
+            parsedSnoozeAction(principal.hostPort, principal.origin),
-            parsedSettingsAction(principaluri.hostPort),
+            parsedSettingsAction(principal.origin),
           ].filter(x => x.length)
         ),
       },