-
Notifications
You must be signed in to change notification settings - Fork 386
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
MSC1767: Extensible event types & fallback in Matrix (v2) #1767
Conversation
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.
In general, I like this much better than previously. Still strongly believe we should have server-side permission control on event types in non-e2ee rooms.
@non-Jedi many thanks for cogitating on this and reviewing :) |
I feel like this is much clearer and simpler than the previous Google doc version. At least, as a client dev, this doesn't scare me much. :) Thanks. |
I wonder what should happen if a custom state event is extended, e.g. with a text fallback? It could make sense to store the temperature example as state event, keyed by sensors / sensor location, and then the home automation system can directly query the current state and does not have to bother with syncing & filtering until the correct one is found. Still, as custom state events are invisible for normal clients, extending them with a non-state-events to associate a fallback or other one-off / change-related information could help, though that should probably not end up in the actual room state… I guess “the whole thing is now a state event with all content as value” would also work out, though I think the semantics thould be specified in the MSC. Example:
|
The final comment period, with a disposition to merge, as per the review above, is now complete. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
(Someone ticked their box post-fcp, and the bot is confused: ignore it) |
* first cut of MSC1767 for extensible events (replaces MSC1225) * tabs->spaces * fix markdown * fix markdown * GFM needs two spaces after ##? * delta from msc1225 * 2021 refresh * Refactor to be just text messaging + schema * Update MSC numbers * Touchups for FCP * *ahem* * Rewrite MSC to be understandable? * Update proposals/1767-extensible-events.md * Rewrite MSC, again * Clarify Andy's blog state * Relax m.markup's requirements * Misc clarifications * Revert "Relax m.markup's requirements" This reverts commit 5f15b9f. * Clarify push rules handling * Fix example * Update extensible events MSCs list * Update Andy's blog post reference * Update per review feedback * Move emotes, notices, and encryption out to dedicated MSCs * Clarify text throughout * Cover all the bases * Clarify mixins are exactly what they're assumed to be * Clarify fallback and number of datums per event * Fix description of plain text baseline * Clarify SCT's role in feature scope & cut "mixed" events (not mixins) * Fix description of mixins * Update proposals/1767-extensible-events.md Co-authored-by: David Baker <dbkr@users.noreply.github.com> * Expand on how unknown events are handled * Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Trim line length post-suggestions * Mention that extensible events might appear in other room versions early * Remove duplicated unknown parse order being an implementation detail * Note difference between optional content blocks and mixins * Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Update proposals/1767-extensible-events.md Co-authored-by: Alexey Rusakov <Kitsune-Ral@users.sf.net> * Clarify that clients are still strongly encouraged to validate HTML * Fix mention of mixing legacy event types * Update proposals/1767-extensible-events.md Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> * Link to MSC3765 * Rename `m.markup` to `m.text` --------- Co-authored-by: Travis Ralston <travpc@gmail.com> Co-authored-by: Travis Ralston <travisr@matrix.org> Co-authored-by: David Baker <dbkr@users.noreply.github.com> Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Co-authored-by: Alexey Rusakov <Kitsune-Ral@users.sf.net> Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Another possible mixin would be `m.relates_to` (not described by this MSC). Currently, | ||
some features like the [key verification framework](https://spec.matrix.org/v1.5/client-server-api/#key-verification-framework) | ||
rely on relationships as part of making the feature work. The expectation is that | ||
these features would be adapted to meet the "purely additive" condition (assuming | ||
`m.relates_to` does actually end up being a mixin). |
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.
What about reactions? Surely those can't function without a relation being present 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.
yea... reactions are awkward. The relationship should be additive (without aggregation key), but representing the aggregation key as a content block is difficult. The expectation is that a dedicated MSC would solve extev reactions.
blocks and legacy fields, however that approach did not give sufficient reason for clients | ||
to fully adopt the extensible events changes. | ||
|
||
In room versions supporting extensible events, clients MUST only send extensible events. |
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.
What happens when an existing room with lots of "old" events is upgraded from a version not supporting extensible events to one that does?
Since events are immutable, does it mean that right after the upgrade almost all the events in the room will not be rendered by the clients anymore effectively making the room look empty?
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.
Upgrading a room creates an entirely new room, "old" events will not be present in there.
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.
noob alert!
Oh I wasn't aware of this, where does the spec mention it?
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.
Here for example: https://spec.matrix.org/v1.8/client-server-api/#server-behaviour-16
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.
Super thanks! I naively thought such an important thing would be mentioned in https://spec.matrix.org/v1.8/rooms/ . I still have to get used at how the spec chapters are organized.
showing up in the plaintext or in tertiary formats on the events. Historically, room | ||
moderators have been pretty good about removing these malicious senders from their rooms | ||
when other users point out (quite quickly) that the event is appearing funky to them. | ||
|
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.
m.room.power_levels
provide no way of restricting what media (via mixins) you can embed into events.
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.
@Gnuxie could you write this up as a new matrix-spec issue to make sure it doesn't get lost?
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.
Perhaps an extremely naive question, but here goes: |
@Gnuxie Thanks for that link. I'm at a loss. It circularly points back to this issue. Power Levels is a State parameter. I am curious why it shows up here. I'm looking for an example of how powerlevels affects media type. |
@KnowledgeGarden sorry the link is unrelated, you just bumped the PR and reminded me to open an issue that i forgot about. |
Rendered.
Obsoletes #1225
Shepherd: @turt2live
Implementations:
Proven by other MSC implementations as well (may be an older version of this MSC, but still compatible on a technical level):
Early conceptual work for extensible events usage:
Articles:
FCP tickyboxes