From a96823cad452f972f1409b3119010fe771b09fb0 Mon Sep 17 00:00:00 2001 From: Nicholas Rishel Date: Tue, 2 Aug 2022 19:40:40 +0000 Subject: [PATCH] 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 --- .../NotificationCallback.cpp | 4 + widget/windows/ToastNotificationHandler.cpp | 51 +++++++++--- .../tests/unit/test_windows_alert_service.js | 80 ++++++++++++++++--- 3 files changed, 115 insertions(+), 20 deletions(-) diff --git a/toolkit/mozapps/notificationserver/NotificationCallback.cpp b/toolkit/mozapps/notificationserver/NotificationCallback.cpp index ff3c00882c41..2494d32f2374 100644 --- 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; } } diff --git a/widget/windows/ToastNotificationHandler.cpp b/widget/windows/ToastNotificationHandler.cpp index 14e1477ff1fd..537f5cb2e936 100644 --- 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 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 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 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 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 diff --git a/widget/windows/tests/unit/test_windows_alert_service.js b/widget/windows/tests/unit/test_windows_alert_service.js index f04ea67fa2fe..22702a3f45f0 100644 --- 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 => { + // is "\n". + let s = ``; + if (serverEnabled) { + s += `program firefox profile ${gProfD.path}`; + } else { + s += `invalid key invalid value`; + } + if (argument) { + s += ` action ${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 = - 'titletext'; + let expected = `titletext`; Assert.equal(expected, alertsService.getXmlStringForWindowsAlert(alert)); alert = makeAlert({ name, title, text, imageURL }); - expected = - 'titletext'; + expected = `titletext`; Assert.equal(expected, alertsService.getXmlStringForWindowsAlert(alert)); alert = makeAlert({ name, title, text, imageURL, requireInteraction: true }); - expected = - 'titletext'; + expected = `titletext`; Assert.equal(expected, alertsService.getXmlStringForWindowsAlert(alert)); alert = makeAlert({ name, title, text, imageURL, actions }); - expected = - 'titletext'; + expected = `titletext`; 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); });