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

MSC4238: Pinned events read marker #4238

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ajbura
Copy link

@ajbura ajbura commented Dec 10, 2024

@ajbura ajbura changed the title MSC: Pinned events read marker MSC4238: Pinned events read marker Dec 10, 2024
@ajbura ajbura marked this pull request as ready for review December 10, 2024 12:22
@turt2live turt2live added proposal A matrix spec change proposal client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Dec 10, 2024
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation requirements:

  • Client
  • Server


Currently, messages are pinned using the `m.room.pinned_events` state event (see [specification](https://spec.matrix.org/v1.12/client-server-api/#mroompinned_events)). However, when new messages are pinned by the super user, other users can not detect newly pinned messages.

It primarily happen because `m.room.pinned_events` does not have any information that can help users detect the change. Resulting in users having no visual clues about newly pinned message. To cope with it they either have to mentally remember the total number of pinned message or discovery of `m.room.pinned_events` state event in clients timeline reminds them to check for possibly new pinned messages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is prev_content not sufficient?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because prev_content doesn't have info regarding the read status of events. Also it contain content of immediate previous whereas new pinned events entry can span multiple events.

It primarily happen because `m.room.pinned_events` does not have any information that can help users detect the change.
Resulting in users having no visual clues about newly pinned message. To cope with it they either have to mentally
remember the total number of pinned message or discovery of `m.room.pinned_events` state event in clients timeline
reminds them to check for possibly new pinned messages.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally agree with the problem statement but not sure if a solution really needs one more read marker. The existing m.read marker applies to message and state events alike so in theory a client could (should?) check whether any (and which, in fact) m.room.pinned_events state events are newer than m.read. The same problem exists with m.room.topic, for example: without being in a room, you cannot tell whether the room topic has changed unless the client marks the room as unread when a new m.room.topic event arrives.

When it comes to the conditions to mark the room as unread, these are effectively defined by push rules. If you add a push rule that says to notify (not highlight) on new m.room.pinned_events the respective room will end up being marked as updated. I encourage the author of this MSC to explore this direction; adding a push rule is easier for the spec as well as for clients.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:feature MSC for not-core and not-maintenance stuff needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants