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

"$foo mentioned you" notifications generally lie.

Open ara4n opened this issue 1 year ago • 9 comments

Steps to reproduce

  1. You receive a notification saying "$foo mentioned you: $msg"
  2. You wince because typically $foo didn't mention you. instead they either:
  • replied to a message you sent
  • used @room
  • or otherwise didn't actually intentionally mention you.

Outcome

What did you expect?

imo, notifications shouldn't waste space with "$foo mentioned you" at all. If we really want do to this, they certainly shouldn't include "$foo mentioned you" unless $foo really did intentionally mention you.

What happened instead?

All sorts of things seem to trigger this. I can't actually remember seeing this notification when someone actually intentionally mentioned me, in fact.

Your phone model

No response

Operating system version

No response

Application version

508

Homeserver

No response

Will you send logs?

No

ara4n avatar Jan 27 '24 23:01 ara4n

So the hasMention check coming from the SDK notification obeys to some push rules that recognise as a mention also non intentional ones (if they are not supported from the sender), replies and room mentions. Probably we want at least for the reply case to distinguish between them. However maybe is worth also discussing from a design or product point of view if we want to keep the additional copy or change it, or what should be intended behaviour for replies or for all room mentions. @VolkerJunginger

Velin92 avatar Feb 02 '24 09:02 Velin92

I see we never made a decision. Should we remove the different text or try to detect intentional mentions only?

jmartinesp avatar Jul 09 '24 10:07 jmartinesp

What will happen if we remove false positives for intentional mentions and fix the computation of hasMention in the SDK?

manuroe avatar Jul 09 '24 10:07 manuroe

Then it should be working as expected: replies won't say you've been mentioned, and neither will messages with @room. Non-intentional mentions however, won't say you've been mentioned, but I think that's expected.

jmartinesp avatar Jul 09 '24 10:07 jmartinesp

yes, this is right.

manuroe avatar Jul 09 '24 10:07 manuroe

So we discussed this with several devs in the Rust SDK Development room and it seems like there's no good way to distinguish between mentions and replies given the current state of the spec:

  1. We have intentional mentions, but the spec says for backwards compatibility clients should add the sender of the replied to message as an intentional mention when replying. There's no way to distinguish between a reply to you that mentions another user and a reply to other user that mentions you given the current data.
  2. There is this MSC, and either the proposed implementation or the alternative one would help us with this problem.
  3. If we don't follow the spec's suggestion (as it's just a suggestion) other clients may have unexpected behaviour and we'd still display 'mentioned you' for messages of users who replied to some message of yours in one of those clients.
  4. We could technically try to check the message contents for actual mentions, but that's a 🐰 hole I'd rather not jump into as it has many possible edge cases.

jmartinesp avatar Jul 10 '24 09:07 jmartinesp

To illustrate it, below is how a notification for a reply looks like on EW on EXI

EW notification

image

EXI / EXA notification

IMG_1484

EXI timeline

For reference, the actual message is displayed like this in the EX timeline: image

manuroe avatar Jul 10 '24 11:07 manuroe

@amshakal , I assigned this to you as it seems we are going to review the copy instead of doing a MSC.

manuroe avatar Jul 11 '24 12:07 manuroe

Makes sense to change the copy. Can we have different copy for different scenarios? One for replies and one for @room? Is there a third scenario?

amshakal avatar Jul 11 '24 13:07 amshakal

the current behaviour is now to say "Foo mentioned or replied to you" on these msgs, given we can't differentiate them otherwise. This is technically correct, but exposes the bug that we can't differentiate them, which is just really weird UX.

i suggest we remove the "mentioned or replied" entirely and just use the name until we have fixed the API to be able to distinguish between them.

cc @amshakal

ara4n avatar Aug 30 '24 08:08 ara4n

This is technically correct, but exposes the bug that we can't differentiate them, which is just really weird UX.

Agreed.

However, this was known to be workaround, not the desired behaviour (which is much more costly). The small upside of the current message is that it is clear (e.g. you know at least the source(s) for these notifications). I would say that if the desired behavior is critical (e.g. causing significant damage), then we should prioritize implementing that solution, instead of fine-tuning the workaround.

mxandreas avatar Sep 02 '24 14:09 mxandreas

I am closing this issue to faciliate the project management for the matrix conf as the current solution is good enough for the moment. The next iteration will happen in https://github.com/element-hq/element-meta/issues/2520.

manuroe avatar Sep 04 '24 14:09 manuroe