Add MarkdownString.supportsHtml api proposal #132182
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mjbvz
commented
Sep 3, 2021
| // If we want to allow markdown permitted tags, then we can delete sanitizer and sanitize. | ||
| // We always pass the output through insane after this so that we don't rely on | ||
| // marked for sanitization. | ||
| markedOptions.sanitizer = (html: string): string => { |
There was a problem hiding this comment.
@alexr00 Are you aware of any extensions besides GitHub PR that use spans in markdown? Would Github PR be able to adopt supportHtml so that we can remove our customize sanitizer here?
mjbvz
commented
Sep 3, 2021
| } | ||
| } | ||
|
|
||
| appendText(value: string, newlineStyle: MarkdownStringTextNewlineStyle = MarkdownStringTextNewlineStyle.Paragraph): MarkdownString { | ||
| this.value += escapeMarkdownSyntaxTokens(this.supportThemeIcons ? escapeIcons(value) : value) | ||
| .replace(/([ \t]+)/g, (_match, g1) => ' '.repeat(g1.length)) | ||
| .replace(/^>/gm, '\\>') | ||
| .replace(/\>/gm, '\\>') |
jrieken
approved these changes
Sep 3, 2021
| } | ||
| } | ||
|
|
||
| appendText(value: string, newlineStyle: MarkdownStringTextNewlineStyle = MarkdownStringTextNewlineStyle.Paragraph): MarkdownString { | ||
| this.value += escapeMarkdownSyntaxTokens(this.supportThemeIcons ? escapeIcons(value) : value) | ||
| .replace(/([ \t]+)/g, (_match, g1) => ' '.repeat(g1.length)) | ||
| .replace(/^>/gm, '\\>') | ||
| .replace(/\>/gm, '\\>') |
Fixes #40607 This change introduces a new `supportsHtml` property on `MarkdownString` that can be used to enable rendering of a safe subset of tags and attributes inside of markdown strings For backwards compatibility, `supportsHtml` will default to false and must be explicitly enabled by extensions This PR will need to go in after we adopt dompurify (#131950) which should provide better control over how we actually go about sanitizing rendered html
mjbvz
force-pushed
the
dev/mjbvz/support-html
branch
from
d007982
to
76dd95b
Compare
|
Would this change (or dompurify PR) make it easier to support more styling properties on Properties like |
|
Not really. If we want to support a subset of css styles, we'd need a good css sanitizer as well. Right now we only support one or two a very specific style strings |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For #40607
This change introduces a new
supportsHtmlproperty onMarkdownStringthat can be used to enable rendering of a safe subset of tags and attributes inside of markdown stringsFor backwards compatability,
supportsHtmlwill default to false and must be explicitly enabled by extensionsThis PR will need to go in after we adopt dompurify (#131950) which should provide better control over how we actually go about sanitizing rendered html