element-x-android icon indicating copy to clipboard operation
element-x-android copied to clipboard

Notifications for rooms that should not be notified

Open kongo09 opened this issue 8 months ago • 1 comments

Steps to reproduce

I've set global notifications to

  • group chats: mentions and keywords only
  • direct chats: all messages

I've also set

  • notify on @room
  • no keywords

Outcome

What did you expect?

I would expect room notifications to follow that.

What happened instead?

And it does in EW. But it doesn't in EXA, leading to inconsistent display of unread messages.

Maybe that's the root cause for: https://github.com/element-hq/element-web/issues/29815

Your phone model

Pixel 8a

Operating system version

Android 14

Application version and app store

25.04.2-nightly

Homeserver

element.io

Will you send logs?

Yes

Are you willing to provide a PR?

No

kongo09 avatar Apr 25 '25 11:04 kongo09

I'm having a issue as well. I'm just not getting notified in room notifications while direct ones work. Even though I have my global set to all messages.

32bitx64bit avatar Apr 25 '25 20:04 32bitx64bit

@kongo09 I think I have a theory about this one: could it be related to threads? Element Web won't display new events in threads as unread events, while Element X does.

After some testing it seems like on the SDK we're currently ignoring read receipts for threads, so even if you open them in EW they won't be marked as read in EX. The only way to make them go aways is to either:

  • Open the room in EXA, so it sends a new read receipt with the 'fully read' behaviour.
  • Manually mark the room as read - I usually open the 'threads' view in EW and tap on the 'mark all as read' icon, which removes the unread room from EXA.
  • Receive a new non-thread event and send a read receipt for it in the EW client.

It would be quite useful to double check if this is what's causing the issue for you. If it is, I believe it'll be fixed once we finish the implementation of threads in EX.

jmartinesp avatar Jun 17 '25 15:06 jmartinesp

I don't think this is it, at least not quite.

  • So I have rooms where I just need to visit the room in EW and then they're marked as read in EXA.
  • Other rooms are not visited as read right away, but these then typically have an unread thread and I need to go into that thread, read that and then EXA reacts and removes the notification bubble

What irritates me is that it used to work every well about 2 months ago or so but then stopped as a regression. So did the read receipts handling in the SDK change?

kongo09 avatar Jun 17 '25 16:06 kongo09

There is one identified scenario where some people with old matrix accounts can get unexpected unreads for DMs because of a new state change or a reaction, basically because of any new matrix event. This is due to a legacy push rule, im.vector.rule.room_one_to_one:

{
    "conditions": [
      {
        "kind": "room_member_count",
        "is": "2"
      }
    ],
    "actions": [
      "notify"
    ],
    "rule_id": "im.vector.rule.room_one_to_one",
    "default": false,
    "enabled": true
}

EW does not have this issue because the JS SDK builds its own view of push rules in rewriteDefaultRules() with a list of push rules it supports. Other rule ids like this faulty im.vector.rule.room_one_to_one are ignored. This is the client fixing incorrect backend data. We need a better solution on mobile because it uses sygnal, APNS or Firebase, background service on mobile for nothing.

manuroe avatar Jun 18 '25 12:06 manuroe

Adding some info here to track the issue publicly, although this was discussed privately:

What caused this issue was UTDs. There is a push rule .org.matrix.msc4028.encrypted_event that is used to make sure m.room.encrypted events in encrypted rooms are always sent as push notifications, and then the client can receive them, fetch the event, decrypt it and decide if an actual UI notification should be displayed for it:

{
  "conditions": [
    {
      "kind": "event_match",
      "key": "type",
      "pattern": "m.room.encrypted"
    }
  ],
  "actions": [
    "notify"
  ],
  "rule_id": ".org.matrix.msc4028.encrypted_event",
  "default": true,
  "enabled": true
}

The problem with this push rule is:

  1. An UTD is a m.room.encrypted event that could not be decrypted, so it has the notify push action.
  2. The Rust SDK follows a simple rule for computing unread notification counts: checking if newly received events have a notify push action.

So when we mix these 2 facts we have the side effect that UTDs count as unread notifications and mark the room as unread in the room list, even if on other clients we can see they're events that shouldn't be notified - since they can't be decrypted we have no idea what they actually are.

Why are we receiving so many UTDs though? That's the real question, and it seems like this started happening when we performed a series of structural changes in the Rust SDK, one of them being changing how the UTD hook works with the timelines and how timelines can be instantiated. With that in mind, one of the main suspects of why this could happen is the sync room list on push feature, which will create a new timeline for the room so it can check we've actually synced it correctly in background.

With the work in https://github.com/element-hq/element-x-android/pull/4915 we're avoiding the creation of this timeline, and we hope this will help with the issue.


After another look, that PR probably won't help with the issue: the UTDs that were incorrectly counted as unread notifications were added in a sync when the app was just started and on foreground, not by a push notification.

jmartinesp avatar Jun 25 '25 08:06 jmartinesp