-
Notifications
You must be signed in to change notification settings - Fork 388
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
MSC4252: Extensible Events modification: State event handling #4252
Open
turt2live
wants to merge
2
commits into
main
Choose a base branch
from
travis/msc/extev/00-base/mod/state
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implementation requirements:
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
# MSC4252: Extensible Events modification: State event handling | ||
|
||
[MSC1767 (accepted; not merged)](https://github.com/matrix-org/matrix-spec-proposals/pull/1767) introduced | ||
a concept of Extensible Events to Matrix, with a specific recommendation that state events *not* be | ||
treated as extensible except on a case-by-case basis. During review of [MSC3765](https://github.com/matrix-org/matrix-spec-proposals/pull/3765), | ||
it was thought that the recommendation may need to be changed - this MSC captures that thinking, which | ||
is similarly captured in [this comment](https://github.com/matrix-org/matrix-spec-proposals/pull/3765#discussion_r1915778656) | ||
on MSC3765. | ||
|
||
## Proposal | ||
|
||
MSC1767 is modified to permit clients to use content blocks to render unknown state event types in | ||
rooms. Clients SHOULD render such events in a way where the user is not confused by the event being | ||
rendered. An example approach may be to add a small decoration to say "this message may appear differently | ||
to other users". | ||
|
||
## Potential issues | ||
|
||
Clients may render state events in a confusing way for users, allowing senders to 'trick' the user | ||
into believing something was said. For example, sending `m.room.topic` state events with `m.text` of | ||
`Alice has been promoted to Admin`. Per the proposal, clients should find a way to de-confuse the | ||
user, or otherwise handle unknown (state) event types more safely. Or, render specified event types | ||
and avoid the issue entirely 😇. | ||
|
||
## Alternatives | ||
|
||
The obvious one is the current text of MSC1767, as of writing: | ||
|
||
> Unknown state event types generally should not be parsed by clients. This is to prevent situations | ||
> where the sender masks a state change as some other, non-state, event. For example, even if a state | ||
> event has an `m.text` content block, it should not be treated as a room message. | ||
|
||
## Security considerations | ||
|
||
Implied by potential issues. | ||
|
||
## Unstable prefix | ||
|
||
While this proposal is not considered stable, clients should follow MSC1767's unstable prefix guidance. | ||
|
||
Clients are encouraged to experiment with UI/UX which de-confuses users. | ||
|
||
## Dependencies | ||
|
||
This MSC has no dependencies which aren't already accepted. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we documented that modifying the existing MSC in-place was accepted practice, but I'm not seeing such text. #4253 brings it forward.