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.
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
MSC2477: User-defined ephemeral events in rooms #2477
base: old_master
Are you sure you want to change the base?
MSC2477: User-defined ephemeral events in rooms #2477
Changes from 5 commits
aad3e26
4aa82b8
d4930c0
ba0b657
0b27339
5dc9372
dd488d3
9d73697
2f4dd47
047d63b
2cae98c
ff2ddbe
64e9460
d01e95d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
worth noting that this limits custom EDUs to rooms compared to something like presence which is user based instead of room based.
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.
The second line in my proposal block already notes this, as I didn't think the usefulness of custom non-room EDUs merited the work of designing some whole new kind of permission system for sending them. Though perhaps a reference to user EDUs built on the Profile-as-a-Room MSC1769 could be added.
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.
Following the new per-endpoint versioning spec.
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.
"optional but suggested" is not really a state we can support. If we're bumping room versions, we should make it required.
It's also unclear how this would apply to something like presence which is not bound to a room and already duplicates this information.
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.
This proposal was by design only made for room-specific ephemeral events, so presence - as a non-room-bound EDU - shouldn't be affected. I'll try to make this more clear.
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.
room_id
isn't part of the spec for ephemeral event bodies down /sync.Likewise below.
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.
The spec entry you linked includes an
m.typing
event in the example response for code 200, which does includeroom_id
in the data;I think that's where that came from, since I tried to base my examples directly on the spec examples.
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 see, thanks for pointing that out! I've made a PR to fix that, as I believe that's a spec bug: #3679.
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.
This feels suboptimal because it means the room admin needs to know the entire superset of features of all potential clients which may join the room. Consider:
To resolve this requires a fair bit of social work as either:
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.
Unstable prefixes will need to be defined in order for an implementation of the MSC to be carried out. I suggest implementation replace:
ephemeral
andephemeral_default
fields in them.room.power_levels
state event withorg.matrix.msc2477.ephemeral
andorg.matrix.msc2477.ephemeral_default
respectively.PUT /_matrix/client/v1/rooms/{roomId}/ephemeral/{eventType}/{txnId}
endpoint withPUT /_matrix/client/unstable/org.matrix.msc2477/rooms/{roomId}/ephemeral/{eventType}/{txnId}
.And an experimental room version with name
org.matrix.msc2477
should be used where this feature is enabled.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.
And a whole lot later than it should've been, but I've added a section on an unstable prefix.
Wasn't sure about adding blurbs on when the
m.room.power_levels
keys should be allowed, but I figured that it'd a bit superfluous since the only method that will act on them is limited to the experimental room version.