1805514 - Generalize Firefox Windows notification actions to `SpecialMessageActions` API

Status: RESOLVED FIXED

(Toolkit :: Alerts Service, enhancement)

Description

[Nick Alexander :nalexander [he/him]]

Right now, Windows notifications can include a launchURL so that clicking a notification button opens that tab (even upon relaunching Firefox if it is not already running).

This ticket tracks generalizing this parameter to the SpecialMessageActions API, which supports a bunch of different "deep link" actions (in a loosely structured JSON structure).

Comment 1

[Nick Alexander :nalexander [he/him]]

Barret: I'm looking at this in preparation for additional background update notification experiments. I see a schema for SMAs but I don't see any message channels consuming the schema. The most obvious consumer of SMAs that I see that could comply to a schema is an untyped JSON object in the InfoBar schema. That surprises me: is it possible to require the SMA schema for that JSON object? Can you suggest how I can add a onclick SMA to each actions element around here that uses the SMA schema?

Thanks!

Comment 2

[Beth Rennie [:beth] (she/her)]

You're correct, the SMA schema is not consumed at this time. The action entry in the InfoBar schema is an SMA and there are others in Spotlight (but its schema is incomplete)

To use the SMA schema json, you would have to edit make-schemas.py to bundle the definitions (like we do for FxMSCommon.schema.json here

I suggest adding a bundle_actions flag like we have for bundle_common, except it can just be bool.

Then patch the schema:

if schema_def.bundle_actions:
    sma_schema = patch_schema(
        read_schema(SMA_SCHEMA_PATH),
        bundled_id=schema_def.schema_id,
        schema_id=schema_def.schema_id,
    )

    # patch_schema mutates the given schema, so we read a new copy in for
    # each bundle operation.
    defs.update(
        {
            name: dfn
            for name, dfn in sma_schema["definitions"].items()
        }
    )

Then in your background task message, you can use a ref like:

"onclick": {
    "$ref": "file:///SpecialMessageActionsSchemas.json#/definitions/SpecialMessageActionSchemas",
    "description": "The action to trigger when the message is clicked"
}

This should be enough to get you started.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Pre: Fix typos. r?nrishel

Depends on D182303

Comment 4

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Pre: Remove unused listener declaration. r?nrishel

Depends on D182304

Comment 5

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Part 1: Support bundling actions in `make-schemas.py`. r?barret

Depends on D182305

Comment 6

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Pre: Fix typos. r?nrishel

Depends on D182308

Comment 7

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Pre: Remove unused listener declaration. r?nrishel

Depends on D182309

Comment 8

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Part 1: Support bundling actions in `make-schemas.py`. r?barret

Depends on D182310

Comment 9

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Part 1: Support `launch_action` in Windows native toast notifications. r?barret

launch_action is both top-level (when the toast is clicked) and
per-action (when a button is clicked).

Depends on D182311

Comment 10

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Part 2: Compare with structured comparison rather than string comparison. r?nrishel

We have gotten a great deal of mileage comparing Windows toast XML as
strings. However, we intend to start including JSON in the
arguments blocks round-tripping through the notification server, and
the encoding of JSON first into strings and then into XML is extremely
painful to read.

This ugly commit rewrites the XML to be simpler, parsing the launch
and action arguments into data structures. The simplified XML is
compared as a string while the arguments are compared as data
structures. In the future, we might parse entirely into a data
structure for comparison.

Depends on D182312

Comment 11

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Part 3: Allow JS opaque relaunch data and actions. r?nrishel

This commit replaces two existing launch argument keys, launchURL
and privilegedName, with an opaque string of data. Here opaque
means, "does not need to be inspected by the Windows notification
server DLL" (and in general, by the system backend components).

The existing action argument key was always intended for this
purpose but was not used in the first implementation. Here, we make
action a stringified JSON object, which is easy for API consumers to
manage and generalizes to (mostly) arbitrary relaunch data.

This JSON object is a compound notificationData object containing
both:

• the consumer-provided opaqueRelaunchData (generally, an action);
• and implementation-provided details (the alert's name, if
  privileged, etc).
  This compound object and the fact that everything transits as strings
  makes everything a little more confusing than it really is.

The API to this opaque relaunch data is based on strings for
convenience. It would be possible to manage JSON objects, perhaps by
using nsIStructuredCloneContainer to serialize "structured clone
encodable" JS objects across the process boundaries, but managing the
objects and container in that approach is much more effort than having
the API consumer stringify as desired.

Depends on D182313

Comment 12

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1805514 - Part 5: Add tests for notification command line handling. r?nrishel

This exposes the Windows notification tag to tests and uses it to test
the action extraction and processing part of the logic.

To expose the notification tag, the Windows system backend adds an
additional "alertshow2" observer notification that sends a JSON object
(stringified) including the tag. It would be possible to rework
"alertshow" to send the existing cookie string and this new data (this
is Bug 1835948), but that would touch a good deal more code and have
its own risks, so I'm not going to do it here.

To test the fallback, the Windows system backend adds a new
alerts.useSystemBackend.windows.notificationserver.forceFallback
boolean pref that defaults to false. When true, Windows 8-style
notification callbacks will not be invoked.

Depends on D182314

Comment 13

[Nick Alexander :nalexander [he/him]]

Attachment: WIP: Bug 1805514 - TO FOLD - Pass `-notification-windowsAction` on the command line.

This patch makes the notification server extract the Firefox action
data from the Windows toast arguments passed to the server callback.
Since this fallback data is now provided to Firefox at launch, there's
no need to fetch it from the Windows notification object; we simply
need to know whether to pass through to a Windows 8.1 callback
(tagWasHandled=true) or to act on the fallback data
(tagWasHandled=false).

Depends on D182314

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2f6028f1b946
Pre: Fix typos. r=nrishel
https://hg.mozilla.org/integration/autoland/rev/73140b0a5dc2
Pre: Remove unused listener declaration. r=nrishel
https://hg.mozilla.org/integration/autoland/rev/15a6ff7b676d
Part 1: Support `launch_action` in Windows native toast notifications. r=barret
https://hg.mozilla.org/integration/autoland/rev/6397f085a6b0
Part 2: Compare with structured comparison rather than string comparison. r=nrishel
https://hg.mozilla.org/integration/autoland/rev/1e09c6aabfcd
Part 3: Allow JS opaque relaunch data and actions. r=nrishel

Comment 15

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/2f6028f1b946
https://hg.mozilla.org/mozilla-central/rev/73140b0a5dc2
https://hg.mozilla.org/mozilla-central/rev/15a6ff7b676d
https://hg.mozilla.org/mozilla-central/rev/6397f085a6b0
https://hg.mozilla.org/mozilla-central/rev/1e09c6aabfcd

Comment 16

[Nick Alexander :nalexander [he/him]]

Comment on attachment 9341457 [details]
Bug 1805514 - Part 3: Allow JS opaque relaunch data and actions. r?nrishel

Beta/Release Uplift Approval Request

• User impact if declined: This functionality is desired (but not strictly required) for an additional background task messaging component of the Challenge the Default campaign, scheduled to launch August 9th. This functionality will allow Challenge the Default-specific notifications to set Firefox as the default browser as part of their Call to Action. Without this uplift, the notification will only be able to launch the browser to a landing page (and use existing functionality for prompting to set as default, etc).
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: QA have tested this functionality in numerous try builds and will also exercise Nightly shortly. (Indeed, they may already have done so.) This testing is tracked in https://mozilla-hub.atlassian.net/browse/QA-1952.
• List of other uplifts needed: None
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): The notification-specific functionality is not really risky: if it is broken in almost any way, it merely means that the messaging campaign will be unsuccessful, and almost all such possible failure modes are invisible to the targeted users.

The risk is that the background task messaging notifications use the same mechanisms as do Web Notifications. It is possible that the generalizations of this patch fail in some as yet unknown circumstances, impacting notification consumers. QA has exercised such scenarios at some length, and Nightly users tend to report these very visible issues quickly, but it is still a (moderate) risk.

• String changes made/needed:
• Is Android affected?: No

Comment 17

[Phabricator Automation]

Comment on attachment 9341453 [details]
Bug 1805514 - Part 1: Support bundling actions in make-schemas.py. r?barret

Revision D182311 was moved to bug 1843820. Setting attachment 9341453 [details] to obsolete.

Comment 18

[Dianna Smith [:diannaS]]

Comment on attachment 9341451 [details]
Bug 1805514 - Pre: Fix typos. r?nrishel

Approved for 116.0b7

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/774ad1c1df71
https://hg.mozilla.org/releases/mozilla-beta/rev/608877a6b345
https://hg.mozilla.org/releases/mozilla-beta/rev/bb7e2f12263a
https://hg.mozilla.org/releases/mozilla-beta/rev/b5d018d85247
https://hg.mozilla.org/releases/mozilla-beta/rev/23424f65527c