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

Remove unintentional pings #886

Closed
1 of 2 tasks
daniellekirkwood opened this issue Dec 23, 2022 · 13 comments
Closed
1 of 2 tasks

Remove unintentional pings #886

daniellekirkwood opened this issue Dec 23, 2022 · 13 comments

Comments

@daniellekirkwood
Copy link
Contributor

daniellekirkwood commented Dec 23, 2022

Your use case

This is a meta issue that contains and concerns the work being undertaken to decide and implement the correct activity when a user's display name is used in message content.

A users display name should not cause a ping (and today is does). Pings should always be intentional and obvious (maybe through pills).


Here are related bugs:


In the Delight team roadmap, "Remove unintentional pings" has 3 line items:

  • Remove the ping when a users display name is written in the timeline. Pings should only occur when a user is ""@"
  • Reactions, poll messages, room events should not ping a user or leave a notification unread in their room list
@daniellekirkwood
Copy link
Contributor Author

@clokep assigning you as you're currently the driver for this. Let me know if you need any product input.

@clokep
Copy link
Contributor

clokep commented Dec 23, 2022

There's a bunch of other cases document in the spec repo (or as part of MSCs):

If folks know of other cases I would like to hear about them!

@clokep
Copy link
Contributor

clokep commented Jan 6, 2023

See MSC3952.

@clokep
Copy link
Contributor

clokep commented Mar 23, 2023

I removed #901 from the description of this issue -- it is only vaguely related.

@clokep
Copy link
Contributor

clokep commented May 10, 2023

MSC3952 has been accepted into the spec, but there's some remaining work to get this in front of users:

The bulk of that work is switching from unstable identifiers (org.matrix.msc3952.* to m.*) and enabling the features by default. As mentioned in the MSC, backwards compatibility between clients and/or servers that are using the legacy rules vs. the new rules should not be an issue most of the time.

Note that Element Web manually pulls push rules before each sync so it should immediately find out about these push rules when they're enabled on a homeserver. Additionally it Element Web uses a server flag to decide whether to enable the feature or not, although I'm not really sure this is necessary.

I think this essentially means we can enable the backend and frontend completely separately.

@kerryarchibald
Copy link
Contributor

@clokep / @daniellekirkwood do we care about backwards compatible support for the unstable identifiers? Ie do we need to support both org.matrix.msc3952.* and m.* on EW? AFAIK intentional mentions has not been released/available on mainline so the impact of abandoning the unstable identifier would be minimal.

@clokep
Copy link
Contributor

clokep commented May 18, 2023

@clokep / @daniellekirkwood do we care about backwards compatible support for the unstable identifiers? Ie do we need to support both org.matrix.msc3952.* and m.* on EW? AFAIK intentional mentions has not been released/available on mainline so the impact of abandoning the unstable identifier would be minimal.

I was not planning to maintain any backwards compatibility on the server. I think impact is minimal and not worth it.

@daniellekirkwood
Copy link
Contributor Author

Happy with that ☝️

@clokep
Copy link
Contributor

clokep commented May 30, 2023

@giomfo suggested that we likely want to do something to handle migrating of disabled push rules so that users don't just have mentions enabled by this work if they had previously disabled them. this sounds reasonable (although I have not yet investigated how hard it would be to do). From discussion we came up with:

  • If .m.rule.roomnotif is disabled, then disable .m.rule.is_room_mention
  • If both .m.rule.contains_display_name and .m.rule.contains_user_name are disabled, then disable .m.rule.is_user_mention

For the second bullet, the thought was that if a single one of them is disabled the user is likely trying to get around the "unintentional" behavior we're fixing, but if they're both disabled then they really don't want mentions. This still might be overly aggressive, but a user can always re-enable it manually in the future.

@daniellekirkwood does this sound good to you? I can look at implementing before stabilizing.

@daniellekirkwood
Copy link
Contributor Author

Yes, sounds good to me.

Good catch both! Thank you

@clokep
Copy link
Contributor

clokep commented May 31, 2023

@clokep
Copy link
Contributor

clokep commented Oct 16, 2023

@daniellekirkwood Should we consider this done?

@daniellekirkwood
Copy link
Contributor Author

@daniellekirkwood Should we consider this done?

YES 🕺

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants