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

Sanitize Option Not Affecting Output #41

Closed
4 tasks done
ryanfitzer opened this issue Jan 24, 2024 · 8 comments
Closed
4 tasks done

Sanitize Option Not Affecting Output #41

ryanfitzer opened this issue Jan 24, 2024 · 8 comments
Labels
🙋 no/question This does not need any changes 👎 phase/no Post cannot or will not be acted on

Comments

@ryanfitzer
Copy link

ryanfitzer commented Jan 24, 2024

Initial checklist

Affected packages and versions

remark-html@16.0.1

Link to runnable example

codesandbox example

Steps to reproduce

Using:

  • node@20.9.0
  • npm@10.2.4
  • next@14.0.4

Expected behavior

The <video> element would be included in the output.

Actual behavior

The <video> is stripped from the output.

Runtime

Other (please specify in steps to reproduce)

Package manager

Other (please specify in steps to reproduce)

OS

macOS 13.6.1 (22G313)

Build and bundle tools

Next.js

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Jan 24, 2024
@wooorm
Copy link
Member

wooorm commented Jan 24, 2024

Please post code. Everything’s tested well. You need to prove things. Don’t waste folks time.

@ChristianMurphy
Copy link
Member

Welcome @ryanfitzer! 👋
Sorry you ran into a spot of trouble.

Please check the readme for the projects and spend some time articulating your question.
See https://github.com/remarkjs/.github/blob/main/support.md for recommendations.

You aren't using the sanitize option in your example your question doesn't really make sense.
My guess is you haven't setup the full HTML flow, please see https://unifiedjs.com/learn/recipe/remark-html/

@ChristianMurphy ChristianMurphy closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2024
@ChristianMurphy ChristianMurphy added the 🙋 no/question This does not need any changes label Jan 24, 2024

This comment has been minimized.

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Jan 24, 2024
@ryanfitzer
Copy link
Author

I updated the codesandbox link. Looks like the one I originally copied using the "share" link just gave me the starting example :(

@ChristianMurphy
Copy link
Member

ChristianMurphy commented Jan 24, 2024

@ryanfitzer thanks for updating the link, again please read https://unifiedjs.com/learn/recipe/remark-html/#how-to-properly-support-html-inside-markdown
You are missing support for embedded HTML in markdown.

@ryanfitzer
Copy link
Author

Thanks for the help! My confusion is due to the following factors:

  • The readme.md states that it "compiles markdown to HTML".
  • The readme.md doesn't mention support for embedded HTML is not included.
  • Embedded HTML is part of the Markdown spec.
  • When I used { sanitize: false } in the options, everything worked as expected, so I was thinking I just needed to provide a customized schema to the sanitize option.

I updated the codesandbox with a functioning example for anyone who comes across this issue with the same confusion as I did.

Again, thanks for the support and for providing the helpful codesandbox template!

@ChristianMurphy
Copy link
Member

ChristianMurphy commented Jan 24, 2024

The readme.md states that it "compiles markdown to HTML".
The readme.md doesn't mention support for embedded HTML is not included.

Which readme are you referring to?
If you have specific recommendations to concisely improve the readme, happy to discuss further.

Generally speaking, there are so many possible use cases, we can't document all of them, and especially not across to 300+ readme files. This is why https://unifiedjs.com/learn/ exists, to capture common use cases as recipes.

Embedded HTML is part of the Markdown spec.

This project implements the CommonMark spec.
It is also important to note the distinction between embedded in the language, and part of the language.
HTML is embedded, it is a separate document contained in the Markdown document. It is not part of the Markdown language.

@wooorm
Copy link
Member

wooorm commented Jan 24, 2024

the first misconception is that this project is not recommended, per the readme. There is a ton more power if you choose the recommended tools.
There used to be like 5 warnings in the readme, which is overkill. Then folks don’t read the other text. Now I have one, in the “When should I use this” section.

the second misconception here seems to be whether HTML is dangerous or not. HTML is dangerous. So it’s all removed by default, and you can configure and choose what is allowed or not afterwards, which is documented in the Options section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🙋 no/question This does not need any changes 👎 phase/no Post cannot or will not be acted on
Development

No branches or pull requests

3 participants