Support increasing "unread count" for rooms without sending push notifications

[neilisfragile]

https://github.com/vector-im/riot-web/issues/8312
(actually, it's more https://github.com/vector-im/riot-web/issues/3268)

[erikjohnston]

We tested this for mozilla, but found that people got confused about lack of notification counts. We probably want to figure out how to do that, c.f. element-hq/element-web#6061 and https://github.com/vector-im/riot-web/issues/3374

[richvdh]

we're not actually sure we want to do this

[dud1337]

My 2c. Would love to get more feedback from others, though.

I use synapse and riot personally and for work.

In both cases, the amount of notifications, how "noisey" riot is, has been the biggest complaint, other than key-signing, and essentially causes users to stop using it via email rants about how distracting it is. No one likes getting a desktop/phone notification for every message of every work room they've been invited to. Especially if you are trying to work and it's the watercooler chat.

For one-to-one rooms (initiated by "Start Chat"), sure.

[neilisfragile]

Desired behaviour is:-

• For DM defaults is that all messages notify.
• For all other rooms, defaults to Mentions and Keywords

Retain existing notification settings for pre existing rooms.

[turt2live]

worth noting that the spec defines default override push rules which are incompatible with that decision. Options include fixing the spec (either by adding a line saying the defaults are suggestions or fixing the defaults), or having clients deal with the new behaviour so as to represent the user's preference rather than the server's.

[neilisfragile]

My immediate reaction would be to fix the spec to suggest the new defaults.

[babolivier]

I've started braindumping about this issue here: https://docs.google.com/document/d/1KgYLJRZzLiPEno-xr1PDkE21ZFftth6FsiRPXekz7DM/edit. I'll try to get this doc in a better shape and do more digging when I'm back on Monday.

[richvdh]

This is horribly complicated due to the multiple moving parts of Riot/Synapse/Spec, each of which has its own behaviour, but to focus initially on the desired behaviour:

Desired behaviour is:-

* For DM defaults is that all messages notify.

The current default here is "All mentions (noisy)". Changing this to "All messages" will mean that a message in a DM room will be pushed to mobile devices, but there will be no audible notification. Are you sure you want to change this behaviour?

* For all other rooms,  defaults to Mentions and Keywords

As previously mentioned, we tried this before, but it means that there is no longer an "unread messages" count for these rooms (unless we do additional work to change that, cf #6061). Again: are you sure?

[richvdh]

worth noting that the spec defines default override push rules which are incompatible with that decision.

@turt2live : I had a brief skim through https://matrix.org/docs/spec/client_server/r0.6.1#default-override-rules and couldn't see any that are incompatible. Can you clarify what you're referring to here?

[turt2live]

sorry, underride rules.

https://matrix.org/docs/spec/client_server/r0.6.1#m-rule-message defines that the default for a message event be a badge count.

[neilisfragile]

@richvdh
For DMs, I wasn't thinking of the distinction between noisy and otherwise. My guess is that it should be noisy for all messages in a DM.
For all other rooms, let me double check with @nadonomy on the assumptions for room message counts.

[nadonomy]

@richvdh
For DMs, I wasn't thinking of the distinction between noisy and otherwise. My guess is that it should be noisy for all messages in a DM.
For all other rooms, let me double check with @nadonomy on the assumptions for room message counts.

You're correct. However, it's more nuanced than that I think. Reading through this issue, I hadn't realised this would be the case:

The current default here is "All mentions (noisy)". Changing this to "All messages" will mean that a message in a DM room will be pushed to mobile devices, but there will be no audible notification. Are you sure you want to change this behaviour?

So principally what we want is the following separation of concerns:

Room settings

Per room, let users manage the following, which they would expect to sync to all of their devices:

• Let users set one of: 'All messages', 'Mentions & Keywords' or 'None'
• Rooms to default to 'Mentions & Keywords'
• DMs to default to 'All messages'

Account settings

Triggers

On triggers, which users would expect to sync to all their devices, and cause push etc:

• Default to 'All messages in DMs. Mentions & Keywords in rooms.' as above
• Let users alternatively set one of: 'All messages (in all rooms)', 'Mentions & keywords only (in both rooms & DMs) or 'None'

These would change the default behaviour for their account, where they can still modify the per room settings above.

Appearance

On appearance, which would be advantageous to sync across devices:

• Default to showing badges for notifications in room lists
• Let users opt in to display numbers in those badges

Audio

On audio, the behaviour is more nuanced, and we would expect users to manage these locally:

• For web/desktop, default audio to off, and let users opt in
• For mobile, while using the app default audio to on— letting users (a) use their hardware mute switch & (b) disable in Riot or OS settings
• For mobile push, default to on with audio— letting users (a) mute via hardware mute switch, and OS features like Do Not Disturb & (b) disable in Riot or OS settings

---

Hopefully all of the above is clearer for all parties involved, but from scanning this issue I get the sense that there may be more dragons than we'd hoped. Let me know if you think it would be more productive to debug further async or on a call.

[ara4n]

We should probably acknowledge that the scope of this bug is changing into a general 'fix notif behaviour' one - but rather than polishing a turd, i'd rather that we did consider the bigger picture, so I think it's defensible for the scope to creep here.

@nadonomy, I think your proposal above may be missing some edge cases:

• Do we not want to be able to control audio on a per-room basis as well as the per-room one? (albeit as a "sound on/off toggle" rather than the confusing "noisy" thing?)
• The feedback from mozilla when I tried to fix this last time (back at the first comment) was that they wanted badges without notifications - whereas currently (and in your proposal above) you only get badges if you also get notifications. In other words, they want to have a badge count of the number of messages they've missed in a given room without getting toasts & push notifs for each and every message... which doesn't seem unreasonable. Add experimental "dont_push" push action to suppress push for notifications #6061 provided a solution to this, but it didn't pass review so would need to be reopened & fixed.

So we may want to consider these.

[nadonomy]

Do we not want to be able to control audio on a per-room basis as well as the per-room one? (albeit as a "sound on/off toggle" rather than the confusing "noisy" thing?)

This was very deliberately scoped out to align better with a simpler set of concerns and user interactions since the first proposals. During ideation & research we were confident that we didn't want or need this, and the fact it hasn't come up until now also gives me more confidence in that decision. I misspoke— you can do this from Room Settings, but we deffered it from the main room list interactions.

Relatedly we'll get ephemeral audio management as a part of Do Not Disturb, which I think is much closer aligned with users goals.

The feedback from mozilla when I tried to fix this last time (back at the first comment) was that they wanted badges without notifications - whereas currently (and in your proposal above) you only get badges if you also get notifications. In other words, they want to have a badge count of the number of messages they've missed in a given room without getting toasts & push notifs for each and every message... which doesn't seem unreasonable. element-hq/element-web#6061 provided a solution to this, but it didn't pass review so would need to be reopened & fixed. https://github.com/vector-im/riot-web/issues/3374 is the original bug for that.

Ok, adding this to the product issue to consider the implications of this further.

[richvdh]

ok, sounds like this is not ready for development yet. I'm going to take it off the board for now.

[neilisfragile]

I talked with @erikjohnston and @jryans our conclusions:-

• There is still more thought necessary on how Riot stays in sync with the server.
• We think this would require a spec change, or at the very least some Synapse specific behaviour.

In any case working on getting the point where the defaults could be changed without changing existing behaviour is not blocked on the above, and as a start this issue should concentrate on getting to that point.

[babolivier]

So here's a plan of action for implementing these changes on Synapse:

1. Publish two MSCs:
   1. Defining a mark_unread action in the push rules. Rules with this action will not cause a push/email notification to be sent, but will count towards an unread count exposed in sync responses as unread_count (located alongside highlight_count and notification_count), as we currently do with messages that trigger a notification. The notify action will also increment this counter.
   2. Defining new defaults for global push rules, setting mark_unread as the new default for rooms, and notify as the new default for DMs. Changing the definition of a DM in the push rules defaults (which isn't always relevant) is not included in the scope of this MSC.
2. Implement the mark_unread action
3. Implement unread_count in sync response
4. Implement an update that saves the current default in the push_rules table for existing users
5. Change the default push rules to implement the second MSC

Part of this work is also bundling a server-side solution to allow fixing https://github.com/vector-im/riot-web/issues/3374, and will probably involve reviving #6061.

Step 2 and 3 should happen at the same time (but this is more a nice thing to have than a requirement), however 4 and 5 need to happen at the same time (albeit one right after the other) to avoid having to deal with a hybrid state where we're not sure of what the default value for a new user is.

We have decided not to block implementing and merging steps 2 and 5 on the MSCs in step 1 being fully merged.

Regarding step 4, a background database update was initially considered, however it's not ideal as it might conflict with actions the user might be doing while the update is being applied. A foreground schema update should be better in that regard thought it might block big servers for some time when restarting after upgrading. Maybe shouting loudly about that in the upgrade notes/changelog is good enough?

[richvdh]

First MSC is at matrix-org/matrix-spec-proposals#2625

[richvdh]

We've been doing a lot of thinking about this. It turns out that it's more complicated than taking things we currently send notifications for and giving them a different action of mark_unread. There are a couple of reasons for this:

• We are keen to avoid a "ghost events" situation, where it looks like a room has unread messages, but when you open it, there aren't any new messages. Currently, we push for some things we would not want to bump the unread count, including:
  • edits
  • anything in an encrypted message, which could include all sorts of weird undisplayable events
• There are also a few things that we don't currently have push rules for (and therefore wouldn't increase the unread count), though we could fix that by adding a few extra default rules:
  • power levels
  • topic changes
  • room name changes

Of these, the biggest problem is encrypted messages. The plan here is:

• m.room.encrypted events will increment the unread_count (which will happen anyway, because they will have notify actions)
• This means that things like edits will contribute incorrectly to the unread_count, making it oversteer.
• The client therefore looks at messages received in the /sync, and subtracts any that should not count to the unread_count according to the user's push rules, and wouldn't display an unread count for that room until this rollback has happened
• In future this could be made more sophisticated, for example by spidering further back in the room.

[richvdh]

since element-hq/element-web#7667 has landed, I'm going to close this in favour of element-hq/element-web#7723 which can cover the wider piece of work here.