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

details element parsed as string not HTML #2408

Closed
miketaylr opened this issue Apr 18, 2018 · 11 comments
Closed

details element parsed as string not HTML #2408

miketaylr opened this issue Apr 18, 2018 · 11 comments
Assignees

Comments

@miketaylr
Copy link
Member

https://staging.webcompat.com/issues/1489

screen shot 2018-04-18 at 10 42 22 am

@miketaylr
Copy link
Member Author

I'll investigate, but if this is semi-complicated, we should just remove details and use a markdown header.

@miketaylr
Copy link
Member Author

On GitHub
screen shot 2018-04-18 at 10 45 54 am

@miketaylr
Copy link
Member Author

Hm.

screen shot 2018-04-18 at 10 49 37 am

@miketaylr
Copy link
Member Author

Ah, I think it's just being sanitized to a string. Let's see if we can hack it.

@miketaylr miketaylr reopened this Apr 18, 2018
@miketaylr miketaylr self-assigned this Apr 18, 2018
@Regaddi
Copy link
Member

Regaddi commented Apr 18, 2018

Phew. It's basically mixed Github flavoured Markdown with HTML in it. This might be tricky.

@Regaddi
Copy link
Member

Regaddi commented Apr 18, 2018

Seems like https://github.com/svbergerem/markdown-it-sanitizer is not maintained anymore. I'm currently reaching out to @svbergerem to clarify.

Basically <details> and <summary> is not on the list of accepted tags.
Suggested this in svbergerem/markdown-it-sanitizer#8.
But also a PR adding flexibility to this is failing.
Let's see what happens.

@svbergerem
Copy link
Contributor

@Regaddi No, the project is not dead. I'm not aware of any bugs and I didn't have enough time to implement new features. I already planned a bigger refactoring in 2016 that would allow adding and removing accepted tags. I just started working on the project again and I'll let you know when there is anything new.

@karlcow
Copy link
Member

karlcow commented Apr 18, 2018

Just to note that markdown allows inline HTML. It's a feature. :)

@miketaylr
Copy link
Member Author

Just to note that markdown allows inline HTML. It's a feature. :)

Yes, but we use the sanitizer to restrict what's allowed. Lots of scary things can sneak into valid HTML.

@miketaylr
Copy link
Member Author

@Regaddi showed me a workaround that we could ship until the sanitizer lib is updated, I might have some time to work on that tonight (before he wakes up), since I'm just hanging out at an AirBNB away from my family.

@miketaylr
Copy link
Member Author

I might have some time to work on that tonight (before he wakes up)

(probably not happening tonight!)

my idea was to essentially create a new file in the /static/js/lib/mixin directory, something like allowlist.js, then include it into the comment and issue model files similar to

_.extend({}, issuesPagination, {
.

ff9d7l

I think that's probably better than forking the sanitizer lib, short-term.

miketaylr pushed a commit that referenced this issue Apr 19, 2018
Fixes #2408 - Allow additional safe HTML tags in sanitized markdown
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants