Consider allowing class on code tag in basic_html

[michalmuskala]

Scrubbing with basic_html works great as a sanitisation pass after converting markdown. There's one issue.

Markdown parsers, that allow for github flavoured markdown, will add class to the code tag with the language name. Currently that class is removed (as are all class attributes). I understand the intent of removing all attributes, but it makes the scrubber unwieldy for working with markdown and requires implementing custom module.

[rrrene]

Agree. 👍

My first gut reaction would be to implement a Markdown sanitizer rather than changing the BasicHTML sanitizer's behaviour.

WDYT?

[michalmuskala]

Oh, a special markdown sanitizer sounds like a great idea!

[rrrene]

@michalmuskala Sorry for not responding here. In my defense, there was this "ElixirConf" going on that occupied my mind 😁

I created a possible iteration of this here. One thing I noticed: Should we allow target="..." on <a>? In practise we might want to set it to _blank in our Markdown parser ...

@ericmj We talked about this briefly in Florida. Any input from you is highly appreciated! 👍

[ericmj]

Only allowing target="_blank" makes sense.

[aphillipo]

If allowing target="_blank" you should also allow rel="noopener" and/or rel="noreferrer" to prevent information leakage and window.opener freakery...

https://mathiasbynens.github.io/rel-noopener/

[rrrene]

@aphillipo That's a valid point!

[rrrene]

I just released v1.1.0 of html_sanitize_ex, which includes the new scrubber. Use HtmlSanitizeEx.markdown_html/1 to sanitize HTML generated by a Markdown parser.

This was tested on elixirstatus.com for the last two weeks by depending on v1.1.0-rc1. Please report any bugs I overlooked!