Define the supported HTML subset for message events #1562
Conversation
Also clarify that `m.notice` messages can support HTML. Fixes matrix-org#1559 Fixes matrix-org#1560
| ``class`` (only classes which start with ``language-`` for syntax highlighting) | ||
|
|
||
|
|
||
| Additionally, clients should ensure that *all* ``a`` tags get a ``rel="noopener"`` |
There was a problem hiding this comment.
+1 to keeping tables, lots of bots n things use it. The discord bridge will be using it to display discords tables in clients.
Apparently FastHub (android client) has rendered all inline comments in one thread, will move this comment when at a PC
| injection, and similar attacks. The strongly suggested set of HTML tags to permit, | ||
| denying the use and rendering of anything else, is: ``font``, ``del``, ``h1``, | ||
| ``h2``, ``h3``, ``h4``, ``h5``, ``h6``, ``blockquote``, ``p``, ``a``, ``ul``, | ||
| ``ol``, ``sup``, ``sub``, ``nl``, ``li``, ``b``, ``i``, ``u``, ``strong``, ``em``, |
There was a problem hiding this comment.
i have no idea what nl is doing in there. i'd be inclined to remove it.
|
|
||
| :``img``: | ||
| ``width``, ``height``, ``alt``, ``title``, ``src`` (provided it is a Matrix Content | ||
| URI) |
There was a problem hiding this comment.
"provided it uses the mxc:// scheme" perhaps
There was a problem hiding this comment.
My concern is that people generally don't know how the media repository works, or what an "mxc://" scheme is. To be fair, a "Matrix Content URI" also doesn't click.
A link on mxc:// maybe?
There was a problem hiding this comment.
I think that making "Matrix Content URI" to the section where it's defined (#module-content) would be helpful. In that section, it's called "Matrix Content (MXC) URI", so I think we should go with that.
| plain text version of the HTML should be provided in the ``body``. | ||
|
|
||
| Clients should limit the HTML they render to avoid Cross-Site Scripting, HTML | ||
| injection, and similar attacks. The strongly suggested set of HTML tags to permit, |
There was a problem hiding this comment.
we should also specify a maximum nesting of the tags, to avoid thousands of nested b-tags crashing browsers.
There was a problem hiding this comment.
Does arbitrarily picking the number 42 seem reasonable?
There was a problem hiding this comment.
ftr went with 100 further down
There was a problem hiding this comment.
(i was worried with 42 being too small given scope for legitimate nesting from RTE copy-paste antics)
| to display if available. Currently ``m.text``, ``m.emote``, and ``m.notice`` | ||
| support an additional ``format`` parameter of ``org.matrix.custom.html``. When | ||
| this field is present, a ``formatted_body`` with the HTML must be provided. The | ||
| plain text version of the HTML should be provided in the ``body``. |
There was a problem hiding this comment.
might be worth a informational link through to #1225 as an FYI on how this might evolve in general, to reassure people we're not baking in the crappy formatted_body thing forever. (do we have a way of doing informational blocks in the spec atm, which aren't normative but just FYI?)
There was a problem hiding this comment.
We can do a "note" which ends up being a dark gray box of text.
There was a problem hiding this comment.
added a note further down
| ``h2``, ``h3``, ``h4``, ``h5``, ``h6``, ``blockquote``, ``p``, ``a``, ``ul``, | ||
| ``ol``, ``sup``, ``sub``, ``nl``, ``li``, ``b``, ``i``, ``u``, ``strong``, ``em``, | ||
| ``strike``, ``code``, ``hr``, ``br``, ``div``, ``table``, ``thead``, ``tbody``, | ||
| ``tr``, ``th``, ``td``, ``caption``, ``pre``, ``span``, ``img``. |
There was a problem hiding this comment.
I suggest we remove the table stuff (table, thead, tbody, tr, th, td, caption) from this list as nobody uses it, and it makes implementation much harder on non-web-based clients. We could include it as a "you could always whitelist this if your client can render it safely, but it's not currently recommended" perhaps.
There was a problem hiding this comment.
I use tables all the time in various alerts from things. It's the closest we have to arbitrary fields like slack.
|
lgtm other than comments |
This commit also includes minor clarifications to surrounding text.
Rendered: see 'docs' status check
Also clarify that
m.noticemessages can support HTML.Reference: https://github.com/matrix-org/matrix-react-sdk/blob/2dc94ac277bfaed6e7a8116ff08bba22ee8fb642/src/HtmlUtils.js#L179-L291
Fixes #1559
Fixes #1560