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

Mention that clients should not be producing invalid HTML #1605

Merged
merged 4 commits into from
Aug 31, 2018

Conversation

turt2live
Copy link
Member

Rendered: see 'docs' status check


Fixes #1595

@turt2live turt2live requested a review from a team August 29, 2018 18:38
@turt2live
Copy link
Member Author

@KitsuneRal I'd appreciate your specific review on this one.

In addition to not rendering unsafe HTML, clients should not emit unsafe HTML in events.
Likewise, clients should not generate HTML that is not needed, such as extra paragraph tags
surrounding text due to Rich Text Editors. HTML included in events should otherwise be valid,
such as having appropriate closing tags, valid attributes, and generally valid structure.
Copy link
Member

Choose a reason for hiding this comment

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

Regarding HTML validity - that's not quite accurate. AIUI, clients should specifically use data-mx-bg-color and data-mx-color attributes instead of color/background-color attributes/CSS properties.

What I had in mind is to minimalistically add " and emit" to line 65, "to/from" to line 77 and slightly reword line 101 because that paragraph only pertains to rendering. But if we want to make a separate paragraph then the above translation of Matrix custom HTML attributes should be mentioned.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think a separate paragraph is better, just because I think we also wanted the whole "don't produce garbage" thing for RTEs. Will add a mention about the custom attributes.

Copy link
Member

Choose a reason for hiding this comment

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

We also need make sure that we allow for things such as the <mx-reply> element that's used in rich replies.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a branch for rich replies that makes reference to this as a special case.

@turt2live turt2live requested a review from a team August 30, 2018 18:21
@turt2live turt2live requested review from KitsuneRal and a team and removed request for a team August 30, 2018 18:22
@turt2live turt2live merged commit 136ba15 into matrix-org:master Aug 31, 2018
@turt2live turt2live deleted the travis/c2s/clarify-html-again branch August 31, 2018 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants