-
Notifications
You must be signed in to change notification settings - Fork 410
MSC3768: Push rule action for in-app notifications #3768
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,96 @@ | ||||||||||||
# MSC3768: Push rule action for in-app notifications | ||||||||||||
|
||||||||||||
The [push rule] system is used in two different ways. Home servers | ||||||||||||
evaluate the rules on messages (which may be encrypted) and send *push* | ||||||||||||
notifications. Clients re-evaluate the rules locally on (decrypted) | ||||||||||||
messages and display *in-app* notifications -- most commonly in the form | ||||||||||||
of notification-count badges. | ||||||||||||
|
||||||||||||
However, there is currently no way to stop sending push notifications | ||||||||||||
while still receiving in-app notifications. This is a critical feature | ||||||||||||
of "Do Not Disturb" modes where users want to stop being notified | ||||||||||||
*outside* their client but still see notifications *inside* their client | ||||||||||||
so that they can catch up on them after leaving "Do Not Disturb" mode. | ||||||||||||
|
||||||||||||
The present proposal attempts to resolve this situation by introducing a | ||||||||||||
dedicated push rule action for in-app notifications without accompanying | ||||||||||||
push notifications. | ||||||||||||
|
||||||||||||
## Proposal | ||||||||||||
|
||||||||||||
A new push rule action `notify_in_app` is introduced. | ||||||||||||
|
||||||||||||
- `notify_in_app` -- This causes each matching event to generate a | ||||||||||||
notification **without sending a push**. In particular, this means, | ||||||||||||
like `notify`, the server should consider the event when computing | ||||||||||||
`notification_count` and `highlight_count` in the `/sync` response. | ||||||||||||
Unlike `notify`, the server should not forward the notification to | ||||||||||||
any of its pushers. Clients should display in-app notifications just | ||||||||||||
like for `notify`. | ||||||||||||
|
||||||||||||
The existing `notify` action is changed to imply `notify_in_app`. | ||||||||||||
|
||||||||||||
- `notify` -- This causes each matching event to generate a | ||||||||||||
notification. Implies `notify_in_app`. | ||||||||||||
|
||||||||||||
No change to the existing default push rules is required. Servers can | ||||||||||||
treat `notifify_in_app` exactly like `notify`, merely omitting the push, | ||||||||||||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
while clients don't have to distinguish between the two actions at all. | ||||||||||||
This makes for a minimally invasive solution to the problem of | ||||||||||||
in-app-only notifications. | ||||||||||||
|
||||||||||||
## Potential issues | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I'm not sure if it's documented but I believe we're encouraged to only comment on the diff / file to facilitate threading.) @bnjbvr said:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The only related reference I am aware of is the one about ignoring I think ignoring works somewhat reasonably well here, too. A client that doesn't support this proposal would treat Alternatively, we could turn
If the server uses
I'm not sure I fully understand what you mean. Isn't this essentially the background
This is true. I had chosen this as a first step because it's very easy to implement (same as the existing mentions & keywords mode but with a different action). UX-wise this might not be overly useful, however. I think the end goal should be to set a room to notify-in-app entirely. This is more complicated to manage though because it also involves tweaking the various override rules. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Oops, sorry for not starting a thread on the diff, and thanks for reproducing it here 👍)
I don't think that In both cases: if the suppression of a notification is the responsibility of a device, then we have another way of achieving the same outcome without this MSC. I wonder if this was also the question behind #3768 (comment). So here's me thinking out loud about cost vs benefits, here:
(Of course this list doesn't pretend to be exhaustive, and maybe I'm missing the point of this MSC. Please, feel free to add other benefits/drawbacks here!) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ok, I think we're saying the same then actually.
It sounds like what you're suggesting is for a client to store some setting in an account data event and then use that setting when processing remote notifications to selectively suppress them. Such an event could look like this: {
"type": "some.client.in_app_notifications",
"content": {
"rooms": ["!asdf:matrix.org", ...]
}
} This seems viable but also somewhat equivalent to what this proposal already does. Push rules are stored in account data and clients use The one benefit I can see is not having to fiddle with the notorious push rule system? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is a huge benefit IMO 😁 |
||||||||||||
|
||||||||||||
None. | ||||||||||||
|
||||||||||||
## Alternatives | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realise these 2 are newer, but it would be interesting to add references to #3881 and #3890 here. I seem to remember we added support for these to Element Classic (element-hq/element-ios#6815 / element-hq/element-ios#6798 – I'm failing to actually find them in the UI though) and I'm not sure I see the advantage of this approach over those. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the difference is that the others are global while this one can work per room (though the push rule legwork to do so is not necessarily trivial). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(I cannot apply this suggestion because I lost ownership of the branch.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ahhhh, thanks for the clarification, I didn't see any mention of wanting to set per-room DnD in the intro so that didn't even cross my mind 🙃 |
||||||||||||
|
||||||||||||
Several attempts at fixing similar problems have been made in the past. | ||||||||||||
Most of these alternatives fell through because they separated unread | ||||||||||||
and notification counts. For the specific case of in-app-only | ||||||||||||
notifications, such a separation is not needed and only adds unnecessary | ||||||||||||
complexity. | ||||||||||||
|
||||||||||||
For the sake of completeness, what follows is the result of an exercise | ||||||||||||
in archaeology: | ||||||||||||
|
||||||||||||
### dont_push action | ||||||||||||
|
||||||||||||
An experimental [Synapse PR] defined a `dont_push` action. While the | ||||||||||||
latter exhibits the same semantics as `notify_in_app`, its naming | ||||||||||||
disguises the fact that notifications are still being displayed in-app. | ||||||||||||
The PR was abandoned in favor of [MSC2625]. | ||||||||||||
|
||||||||||||
### MSC2625:mark_unread action / unread_count field | ||||||||||||
bnjbvr marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
|
||||||||||||
[MSC2625] went a step further by introducing a `mark_unread` action | ||||||||||||
together with an explicit `unread_count` next to the existing | ||||||||||||
`notification_count` and `highlight_count` in the `/sync` response. As | ||||||||||||
explained above, this kind of separation is not actually needed for | ||||||||||||
in-app-only notifications. [MSC2625], too, got abandoned, this time in | ||||||||||||
favor of [MSC2654]. | ||||||||||||
|
||||||||||||
### MSC2654: Unread counts | ||||||||||||
|
||||||||||||
Finally, [MSC2654] went yet further and introduced a separate system for | ||||||||||||
computing unread counters without push rules. Again, the complexity | ||||||||||||
resulting from this separation is not actually required to support | ||||||||||||
in-app-only notifications. | ||||||||||||
|
||||||||||||
## Security considerations | ||||||||||||
|
||||||||||||
None. | ||||||||||||
|
||||||||||||
## Unstable prefix | ||||||||||||
|
||||||||||||
While this MSC is not considered stable, `notify_in_app` should be | ||||||||||||
referred to as `org.matrix.msc3768.notify_in_app`. | ||||||||||||
|
||||||||||||
## Dependencies | ||||||||||||
|
||||||||||||
None. | ||||||||||||
|
||||||||||||
[push rule]: https://spec.matrix.org/v1.2/client-server-api/#push-rules | ||||||||||||
[Synapse PR]: https://github.com/matrix-org/synapse/pull/6061 | ||||||||||||
[MSC2625]: https://github.com/matrix-org/matrix-spec-proposals/pull/2625 | ||||||||||||
[MSC2654]: https://github.com/matrix-org/matrix-spec-proposals/pull/2654 | ||||||||||||
bnjbvr marked this conversation as resolved.
Show resolved
Hide resolved
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole thing sounds like the responsibility of the client or the OS controls on notifications. We could
notify
and push all of the time and the client can decide what to do based on whatever preferences. Or people can adjust their OS notification preferences as they see fit.What benefit is there from not pushing?
(Heads-up: I'm naive on all things push-rules, etc)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this is a good point actually. Due to E2EE, clients need to filter and selectively hide remote pushes anyway. So we could indeed still push the notifications from the server. The only benefit of not pushing would reduced resource usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggestion would be to make skipping the push on the server side a MAY. Functionally it's not required but practically it could save some processing resources. It's also relatively easy to implement.
(I cannot update the proposal, sadly, because this branch was created when I still worked for Element.)