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

test(HTML): Apply whitelist to HTML in HTML accepting properties #4259

Closed
wants to merge 7 commits into from

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Jun 6, 2019

This applies a whitelist approach to HTML in HTML accepting properties (description and notes).


resolves #4033, resolves #3985
closes #4035

@ExE-Boss ExE-Boss requested a review from Elchi3 as a code owner June 6, 2019 21:27
@queengooborg queengooborg added the linter Issues or pull requests regarding the tests / linter of the JSON files. label Jun 7, 2019
@queengooborg
Copy link
Contributor

queengooborg commented Jun 10, 2019

This should also resolve #2799 after documentation has been merged.

Copy link
Contributor

@queengooborg queengooborg left a 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.

@queengooborg queengooborg dismissed their stale review July 2, 2019 22:36

Changes have been made. I'm not ready to give any approval for the PR just yet, but so far, it's looking great!

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Sep 6, 2019

I’m considering moving this to use posthtml‑parser, which is a very lightweight parser that produces a simple AST.

@ddbeck
Copy link
Collaborator

ddbeck commented Oct 28, 2019

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter Issues or pull requests regarding the tests / linter of the JSON files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lint for unnnecessary attributes in markup (e.g. inside notes) Lint for broken HTML
3 participants