Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSC3890: Remotely silence local notifications #3890

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
65 changes: 65 additions & 0 deletions proposals/3890-remote-local-notification-toggle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# MSC3890: Remotely silence local notifications
Some clients (eg Element Web) do not use http pushers for notifications, but rather generate their own notifications in
the client based on the `/sync` response. Users should be able to remotely toggle on and off these notifications (as
well as [push notifications (MSC3881)](https://github.com/matrix-org/matrix-spec-proposals/pull/3881)) to control
interruptions and reachability in private and professional contexts.
Comment on lines +2 to +5
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think clients which use push also process notifications locally when doing /sync -- are they also expected to implement this? It seems like this is meant to be the matrix-js-sdk version of MSC3881, but it isn't really clear the interaction of them.

If you remote toggle a notifications for a client which supports push notifications does that mean setting the account data from MSC3890 (this MSC) & disabling the pusher (see MSC3881) for that device?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I've added a section about how these settings should interact here


To allow these clients to silence notifications remotely we will use account data and client-side notification
filtering.

This proposal seeks to silence interrupting notifications that are generated in the client. This includes:
- system notifications such as system banners, popups, system notification center
- sounds, including noisy notifications and ringers

In-app notifications will not be affected, with in-app notification center, badges, unread markers and counts remaining
the same.

*This proposal only refers to notifications that are generated on the client. Silencing of push notifications is covered
by [MSC3881: Remotely toggle push notifications](https://github.com/matrix-org/matrix-spec-proposals/pull/3881)*

## Proposal
Add a new account data event `m.local_notification_settings.<device-id>` with content:
kerryarchibald marked this conversation as resolved.
Show resolved Hide resolved
```jsonp
{
is_silenced: boolean
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally we prefer to phrase things in the positive: noisy_notifications: true (for example)

}
```
During client-side notification generation:
- While no `local_notification_settings` event exists for the current device, or a `local_notification_settings` event
exists where `is_silenced` is falsy, events should be processed as normal and trigger system notifications and sounds
where necessary.
- While a `local_notification_settings` event exists for the current device where `is_silenced` is true, no event should
trigger a system notification or sound.

During server-side removal of devices:
- When devices are removed servers should delete any `m.local_notification_settings.<device-id>` account_data events for
the given device, and communicate these changes to clients as described in
[MSC3391](https://github.com/matrix-org/matrix-spec-proposals/pull/3391).

A server is to clean up deleted device account data when able to do so, such as with [MSC3391: Removing account
data.](https://github.com/matrix-org/matrix-spec-proposals/pull/3391)

#### Support
Clients that implement `m.local_notification_settings.<device-id>` notification filtering should ensure a
`local_notification_settings` event exists for the active device to indicate support for that device. Clients should
only expose local notification silencing controls for devices that have indicated their support.

## Alternatives
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A consideration might be to have per-device account data similar to how we already have per-room account data: though awkward at first, the intention behind it would be to allow other devices to change the device's account data values, specifically for when cases like this matter.

Another example might be to have per-device [client] settings that might be possible to change from another client, like breadcrumbs or theme. This would be part of the overal "granular settings" idea created years ago by element-web (then vector-web).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with the "granular settings" idea. I'm not sure how many client level settings it would be desirable/useful to have on the server and editable from other devices. Quick setup of new devices that mirror existing ones could be one use case.

If read/write is allowed from any device in the account (which it would have to be to fit this use case) then the added utility of per device account data over profile account data seems quite small.

I've added a small note in the alternatives section which I can explore more fully if needed.

#### Push rules with profile tags
The spec allows for pushers to be assigned a
[`profile_tag`](https://spec.matrix.org/v1.3/client-server-api/#post_matrixclientv3pushersset) which can be used to
define per-device push rule sets. In combination with the `notify_in_app` action proposed in
[MSC3768](https://github.com/matrix-org/matrix-spec-proposals/pull/3768) this would allow to toggle a pusher between the
`global` push rule set and a push rule set where all rules with `notify` actions were overridden to use `notify_in_app`.
Furthermore, the overrides could be simplified through cascading profile tags as proposed in
[MSC3837](https://github.com/matrix-org/matrix-spec-proposals/pull/3837). Keeping the two sets in sync would, however,
not be trivial. Additionally, profile tags are only partially spec'ed and there is active interest in
[removing](https://github.com/matrix-org/matrix-spec/issues/637) them entirely. If feasible, this would allow for remote
control of notifications for clients using http-pushers and local notification generation.
Comment on lines +87 to +88
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is feasible, I think speccing device-scoped push rules even maybe just allowing override rules to manage this would be better as otherwise you have a "local" silencing, but still causing the device to get awoken via its pusher.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^the proposal, like MSC3881, should mention why per-device push rules aren't suitable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] as otherwise you have a "local" silencing, but still causing the device to get awoken via its pusher

This doesn't apply here as the proposal specifically covers clients without pushers that generate notifications locally from /sync responses.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushers are not the same as per-device push rules, confusingly. Per-device push rules are a theory that push rules (which are what clients use to calculate counts locally) could be scoped to a particular device, allowing for locally-silenced notification [counts].

Pushers also use push rules to determine counts/highlights, which also means per-device push rules could also affect how devices with pushers could work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@turt2live sorry, I was replying to the comment above yours, specifically the snippet I quoted. In the context of this MSC, we're focusing on clients that use push rules but not pushers. As a result, without a pusher there is also no wake-up involved. The fact that the silencing is "local" is actually by design because the generation of notifications from /sync responses is also local.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, but the underlying argument is still that this MSC might not be neccessary if per-device push rules were to actually exist. Account data feels like the wrong mechanism for this at the moment, regardless of having a pusher or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I'm not disagreeing with that. My point was just that needlessly waking up clients is not the proper reason to favor per-device push rules because that happens neither here nor in MSC3881.

Anyway, I think we're aligned. What's missing in both MSCs is a more elaborate description of why we think these proposals are favorable to per-device push rules. We'll be working an that. 👍


## Security considerations
N/A

## Unstable prefix
While this MSC is not included in the spec `m.local_notification_settings.<device-id>` should use the unstable prefix
`org.matrix.msc3890.local_notification_settings.<device-id>`