-
-
Notifications
You must be signed in to change notification settings - Fork 99
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
Remove reply fallbacks #1994
Remove reply fallbacks #1994
Conversation
As per MSC2781. Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
replies, this is no longer the case. Clients might still want to remove this | ||
fallback before rendering the event. |
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 think they must remove it, no?
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.
There is not must in the MSC. There is:
Clients […] should consider treating
<mx-reply>
parts as they treat other invalid html tags.
And
[…] clients may need to strip out such fallbacks.
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.
My understanding is that it should not be mandatory as it will allow clients to drop any special code regarding this feature when it is not sent anymore in new messages. It will only appear in historical events but it could be considered acceptable to display the fallback in that case.
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.
Fallbacks are still being sent by current clients, so receiving clients will have to remove the fallback for a while yet.
I think SHOULD captures the intent here.
Signed-off-by: Kévin Commaille <[email protected]>
replies, this is no longer the case. Clients might still want to remove this | ||
fallback before rendering the event. |
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.
Fallbacks are still being sent by current clients, so receiving clients will have to remove the fallback for a while yet.
I think SHOULD captures the intent here.
Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
Signed-off-by: Kévin Commaille <[email protected]>
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.
Thanks!
Due to invalid formatting, replies to replies became garbled, causing display issues in some clients. Hydrogen itself managed to display the replies correctly but other clients and bridges struggled because they were actually using the fallbacks. Current spec: https://spec.matrix.org/v1.12/client-server-api/#fallbacks-for-rich-replies Reply fallbacks are actively being removed in the upcoming spec but that doesn't mean that Hydrogen should keep the old bugged code in place. Upcoming MSCs: - matrix-org/matrix-spec-proposals#2781 - matrix-org/matrix-spec-proposals#3676 - spec: matrix-org/matrix-spec#1994 Signed-off-by: Mirian Margiani <[email protected]>
Due to invalid formatting, replies to replies became garbled, causing display issues in some clients. Hydrogen itself managed to display the replies correctly but other clients and bridges struggled because they were actually using the fallbacks. Current spec: https://spec.matrix.org/v1.12/client-server-api/#fallbacks-for-rich-replies Reply fallbacks are actively being removed in the upcoming spec but that doesn't mean that Hydrogen should keep the old bugged code in place. Upcoming MSCs: - matrix-org/matrix-spec-proposals#2781 - matrix-org/matrix-spec-proposals#3676 - spec: matrix-org/matrix-spec#1994 Signed-off-by: Mirian Margiani <[email protected]>
As per MSC2781.
Fixes: #350
Fixes: #368
Fixes: #372
Pull Request Checklist
Preview: https://pr1994--matrix-spec-previews.netlify.app