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

Add proper support for Mentions (through matching MXID on both formatted_body and body) #19833

Closed
ShadowJonathan opened this issue Nov 21, 2021 · 3 comments
Labels
A-Notifications A-Pills O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience T-Enhancement X-Spec-Changes

Comments

@ShadowJonathan
Copy link
Contributor

ShadowJonathan commented Nov 21, 2021

Your use case

This ticket is a few issues, and one recommendation, rolled into one.

First, i've always been annoyed by the inconsistent mention system, which currently supports 3 methods of pinging;

  • Display name
  • Username
  • @room
  • Keywords

None of these do what i otherwise expect, coming from discord; Explicit mentions.

Element has these, but none of above notification options explicitly target that.

The illusion of explicit mentions is created by the fact that two of the above options overlap separately with a mention's content.

A message with a mention looks like the following;

{
	  "body": "Jonathan",
	  "format": "org.matrix.custom.html",
	  "formatted_body": "<a href=\"https://matrix.to/#/@jboi:jboi.nl\">Jonathan</a>",
	  "msgtype": "m.text"
}

Here, I will get pinged on the Displayname category, as Element's pushers will match on body, check my displayname, and ping me when it contains that displayname.

This is alright, if it werent for the fact that the following can also happen;

{
	  "body": "> <@jonathan:selea.se> It is used as a cache since 1.27.0\n\nThen the documentation needs to be updated.\n\nIt currently lists redis as only needed if you use workers.",
	  "format": "org.matrix.custom.html",
	  "formatted_body": "<mx-reply><blockquote><a href=\"https://matrix.to/#/!ehXvUhWNASUkSLvAGP:matrix.org/$gF9M-GRijYTtl15rL_EdcZg1xT7vrSmnxSIj4cdovqg?via=fariszr.com&via=matrix.org&via=matrix.breakpointingbad.com\">In reply to</a> <a href=\"https://matrix.to/#/@jonathan:selea.se\">@jonathan:selea.se</a><br />It is used as a cache since 1.27.0</blockquote></mx-reply>Then the documentation needs to be updated.\n\nIt currently lists redis as only needed if you use workers.",
	  "m.relates_to": {
	    "m.in_reply_to": {
	      "event_id": "$gF9M-GRijYTtl15rL_EdcZg1xT7vrSmnxSIj4cdovqg"
	    }
	  },
	  "msgtype": "m.text"
}

image

This is a reply, notice how the body of it contains "jonathan" as well, although embedded inside a few characters. This pinged me as well, because of my display name, this makes choosing that option annoying to me.

Now, consider the next case, my username is jboi on my instance, and this problem popped up the first few moments i turned on a bridge;

{
	  "msgtype": "m.text",
	  "body": "> <@whatsapp_[REDACTED]:jboi.nl> Waar ga je anders je cod coins aan uitgeven?\n\nIdk",
	  "format": "org.matrix.custom.html",
	  "formatted_body": "<mx-reply><blockquote><a href=\"https://matrix.to/#/[REDACTED]\">In reply to</a> <a href=\"https://matrix.to/#/@whatsapp_[REDACTED]:jboi.nl\">@whatsapp_[REDACTED]:jboi.nl</a><br>Waar ga je anders je cod coins aan uitgeven?</blockquote></mx-reply>Idk",
	  "m.relates_to": {
	    "m.in_reply_to": {
	      "event_id": "$PqFsOb15D8CAsgDHh1W3zC8-seNFKMEWtjIxksPQXJQ"
	    },
	    "rel_type": "net.maunium.reply",
	    "event_id": "$PqFsOb15D8CAsgDHh1W3zC8-seNFKMEWtjIxksPQXJQ"
	  }
}

image

The jboi.nl part of my bridge's domain name caused the pusher to match on these messages. Admittedly, this is a bridge problem as well, but it made me unable to enable that particular notification option.


Second, I sometimes don't get pinged.

This is a rare enough occurrence that i don't have data on it (sorry), but what I do know is that every time there were pills involved.

In the meantime, i have display name notifications enabled, and i tried enabling a "keyword" with my username (@jboi:jboi.nl), but that one didn't seem to work.

However, i've figured out why;

Pushers only match on body.

Look at this following event format, for this experiment, i had disabled display name notifications;

{
	  "body": "Jonathan",
	  "format": "org.matrix.custom.html",
	  "formatted_body": "<a href=\"https://matrix.to/#/@jboi:jboi.nl\">Jonathan</a>",
	  "msgtype": "m.text"
}

This event should in theory be pinging me, because i enabled the keyword. However, because of rich message fallbacks, the keywords only get matched on body, and the formatted_body is completely ignored.

The following event, however, does ping me;

{
	  "body": "@jboi:jboi.nl",
	  "msgtype": "m.text"
}

Third, here is my suggestion on how to fix this, and fulfil a feature request;

Add an extra notification option, which creates a pusher which matches the user's MXID on both body and formatted_body.

The rationale for this is that, while keywords dont work on formatted_body (though that could be an alternative), users will expect mentions/pings/pills to "just work", this was my first expectation with element as well, and the above cases of Display Name and Username pinging, with more false positives than not, have soured my satisfaction on that front a lot.

So, add a "Mention" option that "Just Works", which will be a hack (as it is a glorified keyword match on body and formatted_body), but until proper mention support arrives (where a ping is not an <a> HTML tag, but a numerated list, for example), I think this is a shiny low-hanging fruit.

Have you considered any alternatives?

No response

Additional context

No response

@ShadowJonathan
Copy link
Contributor Author

ShadowJonathan commented Nov 21, 2021

For synchronisation across different clients, this should probably be a preset push rule, but i'm hesitant to do that, as i'm waiting for a proper fix from the spec before i'd request such a push rule. This is a hack, in my eyes, so I don't think that the spec would properly accept this, when there's other alternatives.

Edit: Actually, now that i think about it, this could be a preset which could be changed to include the mention-list later on, and employ this hack for now.

Edit 2: I've created an MSC for this, I think it's best if this got synchronised across clients, and appended with that proper behaviour later. (matrix-org/matrix-spec-proposals#3517)

@ShadowJonathan
Copy link
Contributor Author

#8431 is related (as an issue-side of this 'coin')

@turt2live
Copy link
Member

This requires spec changes - see matrix-org/matrix-spec#550 and similar for things to upvote/implement.

Notifications are handled server-side (or intended to be server-side), so the client doesn't have reasonable ability to match custom rules like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Notifications A-Pills O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience T-Enhancement X-Spec-Changes
Projects
None yet
Development

No branches or pull requests

4 participants