Do not show membership/profile events in public rooms

[bxdxnn]

Content

Do not display membership and profile is in public rooms.

Motivation and context

Displaying join or profile events in public rooms leads to a significant amount of message flooding, which greatly negatively affects the readability of the room. This issue is especially noticeable in read-only rooms within Spaces, where, over time, you may need to scroll extensively to see actual messages, as they are often buried under numerous join and profile events.

Fixes #5528

Screenshots / GIFs

Tests

• Join a public room
• These events shouldn't be shown

You might encounter empty days if a day only includes filtered events, fix: #6359

Tested devices

• Physical
• Emulator
• OS version(s):

Checklist

• Changes have been tested on an Android device or Android emulator with API 24
• UI change has been tested on both light and dark themes
• Accessibility has been taken into account. See https://github.com/element-hq/element-x-android/blob/develop/CONTRIBUTING.md#accessibility
• Pull request is based on the develop branch
• Pull request title will be used in the release note, it clearly define what will change for the user
• Pull request includes screenshots or videos if containing UI changes
• You've made a self review of your PR

[jmartinesp]

I think this one also needs product supervision @mxandreas . Also, if we were to include this, it should probably have an associated toggle in settings (maybe in advanced settings?).

I was also going to say it would make sense for this to be an SDK feature, but since it's not a change that follows the spec, it's probably if we kept if in the clients.

[bxdxnn]

I suggest disabling this filtering if the "View source" option in the "Advanced settings" is enabled, but I'll wait for the product decision.

Or, can be done by implementing #3760.

[bxdxnn]

@jmartinesp @mxandreas

What's the status of the product decision? Do I need to open a spec proposal instead of doing this change in clients?

This change is very important for any public space, as messages in low-traffic rooms drown quickly from state events in the timeline.

[mxandreas]

This is a bit tricky because there a number of ways how to reduce this "noise". It could be based on the room access, but it could also be a personal preference - depending on what type of event it is. We need a comprehensive "strategy" first which will take a bit more time.

[bxdxnn]

@mxandreas

Some thoughts:

• Current PR's behavior
  • It makes sense that we still show it for private rooms, because:
    • Private rooms commonly don't have too many people — fewer profile changes
    • You might (or need) to know who are the people in the room, so this info is more useful
  • But:
    • We can have a use-case where the room is simply an invite-only and a lot of random people are being invited/in the room (similar to a public room), which will contradict the pros above.
• Move it to an option (enabled by default, in "Advanced Settings", toggles this behavior in all rooms regardless of private/public.)
  • Should we also filter it in DMs?
    • We shouldn't. You might wonder why there's a random DM in your room list when a person changes his profile, so this info should be useful.
  • Separate public/private?
    • Shouldn't be necessary, if someone needs to control/know who are the people in the room he can disable this temporary.
  • Advanced Settings or developer options?
    • I'd say Advanced Settings in the "Moderation and Safety" section for best discoverability, though the option is a bit too technical IMO.
• Filter both public/private (except DMs) without an option
  • Simpler, but hurts the "know the people in the room" use-case

IMO, we should go with "Move it to an option". What do you think? How to proceed with this PR?

[mxandreas]

I am looking into this, but I need feedback from also security, to make conclusions. Also, when we put something out there, it needs to "scale" for all other type of events, like what is our approach for if somebody is pinning a message or changing room topic.

A couple of comments (for join events):

Filter both public/private (except DMs) without an option

This very likely a no go. You do want to know who has joined your private conversation.

Move it to an option

This is not great for two reasons. Adding an option is very often the "path of least resistance" and if we lightly take that route each time we will have millions of options eventually the users are downing into. Secondly, for private rooms, you do want to know proactively/immediately if somebody joined, not go and reactively turn the toggle of to see if anybody has joined.

Show in private but not in public

This likely still the best option. If you do have a room which is invite-only and there are a lot of invitations, it must be an exception. It is hard to be believe you keep it invite-only if a lot of people are joining every day. However, the issue here is that currently room state like E2EE or join rule is not cryptographically protected (can be spoofed by a malicious HS), hence if we hide new members based on that info, it may open up new attack vectors. This is why we need check with security.

[TemoCrevlis]

Additionally, topic changes are also filtered because lengthy descriptions would take up too much space.

Room topic changes are significant, infrequent and impersonal enough (public/private) to warrant keeping it in the timeline.
For lengthy topic: It can appear as "Room topic has been changed ->" (or something to that effect).
And clicking/tapping it takes you to the Room info screen/panel which appears from the right.

[bxdxnn]

Additionally, topic changes are also filtered because lengthy descriptions would take up too much space.

Room topic changes are significant, infrequent and impersonal enough (public/private) to warrant keeping it in the timeline. For lengthy topic: It can appear as "Room topic has been changed ->" (or something to that effect). And clicking/tapping it takes you to the Room info screen/panel which appears from the right.

I agree, but "lengthy" needs to be defined.

[TemoCrevlis]

Additionally, topic changes are also filtered because lengthy descriptions would take up too much space.

Room topic changes are significant, infrequent and impersonal enough (public/private) to warrant keeping it in the timeline. For lengthy topic: It can appear as "Room topic has been changed ->" (or something to that effect). And clicking/tapping it takes you to the Room info screen/panel which appears from the right.

I agree, but "lengthy" needs to be defined.

Setting a 'number of lines' limit can be arbitrary so here's what I'd consider and recommend:
The dropdown message should not interrupt conversation flow and human messages should remain visible on the same screen.

Personally, I can tolerate up to 1/3 of my screen being system messages in an active Room, which is something like 5-8 lines of text. Messages exceeding this may be considered "Lengthy".

This limit also makes closing the dropdown more comfortable as the chevron stays well in view when expanded.

[bxdxnn]

Personally, I can tolerate up to 1/3 of my screen being system messages in an active Room, which is something like 5-8 lines of text. Messages exceeding this may be considered "Lengthy".

Topics are mostly one-lined, and checking the number of UI line wraps is technically challenging. It's better to define a character count instead.

[TemoCrevlis]

Personally, I can tolerate up to 1/3 of my screen being system messages in an active Room, which is something like 5-8 lines of text. Messages exceeding this may be considered "Lengthy".

Topics are mostly one-lined, and checking the number of UI line wraps is technically challenging. It's better to define a character count instead.

You're right, using a character count would be more accurate and simpler.

5 lines of random text on my phone: 173 characters.
8 lines looked like a little too much.
7 lines of random text: 223 characters.

~1/3 of my screen.
~5-7 lines: 173-223 characters.

[bxdxnn]

I've removed topic handling.
I'll address the "topic overflow" in another PR as it looks out of scope.

[bxdxnn]

FWIW, Telegram handles "membership events" by having a separate admin log screen with these kind of events.

[bxdxnn]

@mxandreas it've been 2 months and it's still not clear what's the status on this, is there anything from my side that can/should be done for this to proceed, or I just need to wait?

[mxandreas]

@mxandreas it've been 2 months and it's still not clear what's the status on this, is there anything from my side that can/should be done for this to proceed, or I just need to wait?

I've tried to move this forward internally and we've got some alignment but as people have several ongoing topics, not everything can be moved forward at great velocity. I hope you understand. cc @amshakal

[mxandreas]

After exploring the topic more broadly internally, I can confirm that it is fine to do the following:

• Hide join and leave events in the rooms which are not encrypted and which do not require an invite to join (e.g. join rule is public or restricted).

  • Keep showing all other membership events besides join and leave, e.g. ban, invite, etc.
  • Note: Element Web has currently a setting Show join/leave messages (invites/removes/bans unaffected) - which then applies to all rooms. We do not remove that setting immediately but first test on mobile if the approach here, works.

• Hide profile picture and display name update events in the rooms which are not encrypted and which do not require an invite to join (e.g. join rule is public or restricted).

  • Note: Element Web has currently the settings Show display name changes and Show profile picture changes - which then apply to all rooms. We do not remove that setting immediately but first test on mobile if the approach here, works.

Any comments or questions?

[bxdxnn]

No, everything is clear. That should be the current behavior, but I'll double-check test this PR about ban/kick later as IIRC that's the same event in Matrix.

[bxdxnn]

Updated according to the product decision and tested (with #6359 applied on top). Ready for review/merge

[jmartinesp]

I would have added some kind of option to enable/disable this behaviour, but let's see how it works with real usage and decide if there's any need to change it later.

That said, I have a couple of nits 🙏 .

[bxdxnn]

I would have added some kind of option to enable/disable this behaviour

Yes, but that best to be as a part of #3760

FYI, please note that #6359 should be merged along this PR (at least before the release), otherwise you might get empty day dividers where the membership events were supposed to be.

[codecov]

Codecov Report

❌ Patch coverage is 90.32258% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.25%. Comparing base (00efd9a) to head (1d5604c).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
...meline/postprocessor/RoomBeginningPostProcessor.kt	71.42%	0 Missing and 2 partials ⚠️
...es/impl/timeline/factories/TimelineItemsFactory.kt	93.33%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #6360      +/-   ##
===========================================
- Coverage    81.25%   81.25%   -0.01%     
===========================================
  Files         2652     2652              
  Lines        74373    74402      +29     
  Branches      9641     9649       +8     
===========================================
+ Hits         60432    60455      +23     
- Misses       10381    10385       +4     
- Partials      3560     3562       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bxdxnn]

One thing that isn't clear: will this behavior be ported to iOS by the time of the next release, or it's required to create an iOS PR myself?

[mxandreas]

One thing that isn't clear: will this behavior be ported to iOS by the time of the next release, or it's required to create an iOS PR myself?

I think for this one having it on iOS and Web is not a must have, but it is highly desired as it is going to cause confusion later. Thus, it would be appreciated to the corresponding PRs on Web and iOS.

[jmartinesp]

This will be replaced by matrix-org/matrix-rust-sdk#6644, with some changes in the Element X app.