-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
test(HTML): Apply whitelist to HTML in HTML accepting properties #4259
Conversation
This should also resolve #2799 after documentation has been merged. |
This snuck in from mdn#4265.
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.
Overall, this PR is looking wonderful! Just got a couple of comments on some of the code whilst I was giving it a test whirl on my end.
Changes have been made. I'm not ready to give any approval for the PR just yet, but so far, it's looking great!
I’m considering moving this to use posthtml‑parser, which is a very lightweight parser that produces a simple AST. |
Since this PR has been idle for a while, I'm going to close this issue for now. If you wanted to revisit this with a library (instead of the regex approach), we'd be interested in discussing or reviewing that. As always, thank you, @ExE-Boss! Also of note, @Elchi3 and I discussed today that we'd like to have a set of quality goals for the HTML that's a part of BCD. Included in that would be making sure we only allow a subset of tags and properties and to make sure that our HTML is well-formed. You've been thinking about at least part of this already, so we'd be excited if you participated in that issue, when we've written it up (hoping to do so in the next sprint or two). |
This applies a whitelist approach to HTML in HTML accepting properties (
description
andnotes
).resolves #4033, resolves #3985
closes #4035