Bug 1774083 - Part 4: Prepend Windows toast launch arguments to the toast action arguments. r=nalexander

Windows toast actions (buttons) override the launch argument. The launch arguments are necessary for the notification server to reconstruct the source of the toast, therefore we prepend it to the action argument. Differential Revision: https://phabricator.services.mozilla.com/D152466
2022-08-02 19:40:40 +00:00
parent a10fe6ef6f
commit a96823cad4
3 changed files with 115 additions and 20 deletions
--- a/toolkit/mozapps/notificationserver/NotificationCallback.cpp
+++ b/toolkit/mozapps/notificationserver/NotificationCallback.cpp
@@ -44,6 +44,10 @@ HRESULT STDMETHODCALLTYPE NotificationCallback::Activate(
      }
    } else if (key == L"profile") {
      profile = value;
+    } else if (key == L"action") {
+      // Remainder of args are from the Web Notification action, don't parse.
+      // See https://bugzilla.mozilla.org/show_bug.cgi?id=1781929.
+      break;
    }
  }

--- a/widget/windows/ToastNotificationHandler.cpp
+++ b/widget/windows/ToastNotificationHandler.cpp
@@ -12,6 +12,7 @@
 #include "imgIRequest.h"
 #include "mozilla/gfx/2D.h"
 #include "mozilla/Result.h"
+#include "mozilla/Tokenizer.h"
 #include "mozilla/WindowsVersion.h"
 #include "nsAppDirectoryServiceDefs.h"
 #include "nsAppRunner.h"
@@ -85,6 +86,7 @@ static bool SetAttribute(IXmlElement* element, const HStringReference& name,

 static bool AddActionNode(IXmlDocument* toastXml, IXmlNode* actionsNode,
                          const nsAString& actionTitle,
+                          const nsAString& launchArg,
                          const nsAString& actionArgs,
                          const nsAString& actionPlacement = u""_ns) {
  ComPtr<IXmlElement> action;
@@ -96,8 +98,15 @@ static bool AddActionNode(IXmlDocument* toastXml, IXmlNode* actionsNode,
      SetAttribute(action.Get(), HStringReference(L"content"), actionTitle);
  NS_ENSURE_TRUE(success, false);

-  success =
-      SetAttribute(action.Get(), HStringReference(L"arguments"), actionArgs);
+  // Action arguments overwrite the toast's launch arguments, so we need to
+  // prepend the launch arguments necessary for the Notification Server to
+  // reconstruct the toast's origin.
+  //
+  // Web Notification actions are arbitrary strings; to prevent breaking launch
+  // argument parsing the action argument must be last. All delimiters after
+  // `action` are part of the action arugment.
+  nsAutoString args = launchArg + u"\naction\n"_ns + actionArgs;
+  success = SetAttribute(action.Get(), HStringReference(L"arguments"), args);
  NS_ENSURE_TRUE(success, false);

  if (!actionPlacement.IsEmpty()) {
@@ -345,14 +354,14 @@ ComPtr<IXmlDocument> ToastNotificationHandler::CreateToastXmlDocument() {
    NS_ENSURE_SUCCESS(ns, nullptr);

    AddActionNode(toastXml.Get(), actionsNode.Get(), disableButtonTitle,
-                  u"snooze"_ns, u"contextmenu"_ns);
+                  launchArg, u"snooze"_ns, u"contextmenu"_ns);
  }

  nsAutoString settingsButtonTitle;
  bundle->GetStringFromName("webActions.settings.label", settingsButtonTitle);
  success =
      AddActionNode(toastXml.Get(), actionsNode.Get(), settingsButtonTitle,
-                    u"settings"_ns, u"contextmenu"_ns);
+                    launchArg, u"settings"_ns, u"contextmenu"_ns);
  NS_ENSURE_TRUE(success, nullptr);

  for (const auto& action : mActions) {
@@ -365,8 +374,8 @@ ComPtr<IXmlDocument> ToastNotificationHandler::CreateToastXmlDocument() {
    ns = action->GetAction(actionString);
    NS_ENSURE_SUCCESS(ns, nullptr);

-    success =
-        AddActionNode(toastXml.Get(), actionsNode.Get(), title, actionString);
+    success = AddActionNode(toastXml.Get(), actionsNode.Get(), title, launchArg,
+                            actionString);
    NS_ENSURE_TRUE(success, nullptr);
  }

@@ -514,7 +523,8 @@ HRESULT
 ToastNotificationHandler::OnActivate(IToastNotification* notification,
                                     IInspectable* inspectable) {
  if (mAlertListener) {
-    nsAutoString argString;
+    // Extract the `action` value from the argument string.
+    nsAutoString actionString;
    if (inspectable) {
      ComPtr<IToastActivatedEventArgs> eventArgs;
      HRESULT hr = inspectable->QueryInterface(
@@ -524,17 +534,36 @@ ToastNotificationHandler::OnActivate(IToastNotification* notification,
        hr = eventArgs->get_Arguments(arguments.GetAddressOf());
        if (SUCCEEDED(hr)) {
          uint32_t len = 0;
-          const wchar_t* buffer = arguments.GetRawBuffer(&len);
+          const char16_t* buffer = (char16_t*)arguments.GetRawBuffer(&len);
          if (buffer) {
-            argString.Assign(buffer, len);
+            // Toast arguments are a newline separated key/value combination of
+            // launch arguments and an optional action argument provided as an
+            // argument to the toast's constructor. After the `action` key is
+            // found, the remainder of toast argument (including newlines) is
+            // the `action` value.
+            Tokenizer16 parse(buffer);
+            nsDependentSubstring token;
+
+            while (parse.ReadUntil(Tokenizer16::Token::NewLine(), token)) {
+              if (token == u"action"_ns) {
+                Unused << parse.ReadUntil(Tokenizer16::Token::EndOfFile(),
+                                          actionString);
+              } else {
+                // Next line is a value in a key/value pair, skip.
+                parse.SkipUntil(Tokenizer16::Token::NewLine());
+              }
+              // Skip newline.
+              Tokenizer16::Token unused;
+              Unused << parse.Next(unused);
+            }
          }
        }
      }
    }

-    if (argString.EqualsLiteral("settings")) {
+    if (actionString.EqualsLiteral("settings")) {
      mAlertListener->Observe(nullptr, "alertsettingscallback", mCookie.get());
-    } else if (argString.EqualsLiteral("snooze")) {
+    } else if (actionString.EqualsLiteral("snooze")) {
      mAlertListener->Observe(nullptr, "alertdisablecallback", mCookie.get());
    } else if (mClickable) {
      // When clicking toast, focus moves to another process, but we want to set
--- a/widget/windows/tests/unit/test_windows_alert_service.js
+++ b/widget/windows/tests/unit/test_windows_alert_service.js
@@ -5,6 +5,27 @@
 * Test that Windows alert notifications generate expected XML.
 */

+let gProfD = do_get_profile();
+
+// Setup that allows to use the profile service in xpcshell tests,
+// lifted from `toolkit/profile/xpcshell/head.js`.
+function setupProfileService() {
+  let gDataHome = gProfD.clone();
+  gDataHome.append("data");
+  gDataHome.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
+  let gDataHomeLocal = gProfD.clone();
+  gDataHomeLocal.append("local");
+  gDataHomeLocal.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
+
+  let xreDirProvider = Cc["@mozilla.org/xre/directory-provider;1"].getService(
+    Ci.nsIXREDirProvider
+  );
+  xreDirProvider.setUserDataDirectory(gDataHome, false);
+  xreDirProvider.setUserDataDirectory(gDataHomeLocal, true);
+}
+
+add_setup(setupProfileService);
+
 function makeAlert(options) {
  var alert = Cc["@mozilla.org/alert-notification;1"].createInstance(
    Ci.nsIAlertNotification
@@ -29,7 +50,21 @@ function makeAlert(options) {
  return alert;
 }

-add_task(async () => {
+function testAlert(serverEnabled) {
+  let argumentString = argument => {
+    // &#xA; is "\n".
+    let s = ``;
+    if (serverEnabled) {
+      s += `program&#xA;firefox&#xA;profile&#xA;${gProfD.path}`;
+    } else {
+      s += `invalid key&#xA;invalid value`;
+    }
+    if (argument) {
+      s += `&#xA;action&#xA;${argument}`;
+    }
+    return s;
+  };
+
  let alertsService = Cc["@mozilla.org/system-alerts-service;1"]
    .getService(Ci.nsIAlertsService)
    .QueryInterface(Ci.nsIWindowsAlertsService);
@@ -44,22 +79,49 @@ add_task(async () => {
  ];

  let alert = makeAlert({ name, title, text });
-  let expected =
-    '<toast><visual><binding template="ToastText03"><text id="1">title</text><text id="2">text</text></binding></visual><actions><action content="Notification settings" arguments="settings" placement="contextmenu"/></actions></toast>';
+  let expected = `<toast launch="${argumentString()}"><visual><binding template="ToastText03"><text id="1">title</text><text id="2">text</text></binding></visual><actions><action content="Notification settings" arguments="${argumentString(
+    "settings"
+  )}" placement="contextmenu"/></actions></toast>`;
  Assert.equal(expected, alertsService.getXmlStringForWindowsAlert(alert));

  alert = makeAlert({ name, title, text, imageURL });
-  expected =
-    '<toast><visual><binding template="ToastImageAndText03"><image id="1" src="file:///image.png"/><text id="1">title</text><text id="2">text</text></binding></visual><actions><action content="Notification settings" arguments="settings" placement="contextmenu"/></actions></toast>';
+  expected = `<toast launch="${argumentString()}"><visual><binding template="ToastImageAndText03"><image id="1" src="file:///image.png"/><text id="1">title</text><text id="2">text</text></binding></visual><actions><action content="Notification settings" arguments="${argumentString(
+    "settings"
+  )}" placement="contextmenu"/></actions></toast>`;
  Assert.equal(expected, alertsService.getXmlStringForWindowsAlert(alert));

  alert = makeAlert({ name, title, text, imageURL, requireInteraction: true });
-  expected =
-    '<toast scenario="reminder"><visual><binding template="ToastImageAndText03"><image id="1" src="file:///image.png"/><text id="1">title</text><text id="2">text</text></binding></visual><actions><action content="Notification settings" arguments="settings" placement="contextmenu"/></actions></toast>';
+  expected = `<toast scenario="reminder" launch="${argumentString()}"><visual><binding template="ToastImageAndText03"><image id="1" src="file:///image.png"/><text id="1">title</text><text id="2">text</text></binding></visual><actions><action content="Notification settings" arguments="${argumentString(
+    "settings"
+  )}" placement="contextmenu"/></actions></toast>`;
  Assert.equal(expected, alertsService.getXmlStringForWindowsAlert(alert));

  alert = makeAlert({ name, title, text, imageURL, actions });
-  expected =
-    '<toast><visual><binding template="ToastImageAndText03"><image id="1" src="file:///image.png"/><text id="1">title</text><text id="2">text</text></binding></visual><actions><action content="Notification settings" arguments="settings" placement="contextmenu"/><action content="title1" arguments="action1"/><action content="title2" arguments="action2"/></actions></toast>';
+  expected = `<toast launch="${argumentString()}"><visual><binding template="ToastImageAndText03"><image id="1" src="file:///image.png"/><text id="1">title</text><text id="2">text</text></binding></visual><actions><action content="Notification settings" arguments="${argumentString(
+    "settings"
+  )}" placement="contextmenu"/><action content="title1" arguments="${argumentString(
+    "action1"
+  )}"/><action content="title2" arguments="${argumentString(
+    "action2"
+  )}"/></actions></toast>`;
  Assert.equal(expected, alertsService.getXmlStringForWindowsAlert(alert));
+}
+
+add_task(async () => {
+  Services.prefs.clearUserPref(
+    "alerts.useSystemBackend.windows.notificationserver.enabled"
+  );
+  testAlert(false);
+
+  Services.prefs.setBoolPref(
+    "alerts.useSystemBackend.windows.notificationserver.enabled",
+    false
+  );
+  testAlert(false);
+
+  Services.prefs.setBoolPref(
+    "alerts.useSystemBackend.windows.notificationserver.enabled",
+    true
+  );
+  testAlert(true);
 });