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.

Sign up for GitHub

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

Jump to bottom

Strikethrough (<s>) is removed on sanitize #3565

Closed
wvpm opened this issue Oct 29, 2020 · 1 comment · Fixed by #3646
Closed

Strikethrough (<s>) is removed on sanitize #3565

wvpm opened this issue Oct 29, 2020 · 1 comment · Fixed by #3646
Assignees
@corinagum
Labels
backlog Out of scope for the current iteration but it will be evaluated in a future release. Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Required for internal reporting. Do not delete. customer-reported Required for internal Azure reporting. Do not delete. front-burner p0 Must Fix. Release-blocker size-s 1 days or less
Milestone
R12

Comments

@wvpm
Copy link

wvpm commented Oct 29, 2020

Version

https://cdn.botframework.com/botframework-webchat/4.10.1/webchat.js

Describe the bug

When sending markdown text "~~test~~" to the customer from the bot without a textFormat property in the activity (so it defaults to markdown), the WebChat shows "test" instead of "test".

Additional context

Based on https://github.com/microsoft/BotFramework-WebChat/blob/bbbaeb9c2c46ec61ad109c2dbce46099f87efeba/packages/bundle/src/renderMarkdown.js, I concluded that markdown-it is used to render the markdown. Note that strike is allowed as an html tag whilst s is not. According to https://github.com/markdown-it/markdown-it/blob/master/test/fixtures/markdown-it/strikethrough.txt markdown-it converts strikethrough into s. This is subsequently sanitized, which explains the behaviour I experience.

I would recommend added s to the allowed html tag list.

[Bug]
@wvpm wvpm added Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-reported Required for internal Azure reporting. Do not delete. labels Oct 29, 2020
@wvpm wvpm changed the title Strikethrough is sanitized Strikethrough is removed Oct 29, 2020
@corinagum corinagum changed the title Strikethrough is removed Strikethrough (<s>) is removed on sanitize Oct 30, 2020
@corinagum
Copy link
Contributor

wow, interesting that we added <strike> but not <s>. Thanks for the catch! Putting this on our potential list for R12, for which planning will happen in the next few weeks.

@corinagum corinagum added the customer-replied-to Required for internal reporting. Do not delete. label Oct 30, 2020
@corinagum corinagum added this to the R12 milestone Oct 30, 2020
@corinagum corinagum added size-s 1 days or less backlog Out of scope for the current iteration but it will be evaluated in a future release. front-burner labels Oct 30, 2020
@corinagum corinagum added the p0 Must Fix. Release-blocker label Nov 9, 2020
corinagum added a commit to corinagum/BotFramework-WebChat that referenced this issue Dec 21, 2020
@corinagum
microsoft#3565 Allow <s> on sanitize markdown
76ad803
@corinagum corinagum mentioned this issue Dec 21, 2020
Merged
11 tasks
corinagum added a commit that referenced this issue Dec 23, 2020
@corinagum
Allow <s> on sanitize markdown (#3646)
6368e77 
* #3565 Allow <s> on sanitize markdown

* Update CHANGELOG.md

* Add <del> and <ins> to allowed sanitize tags
@compulim compulim mentioned this issue Mar 2, 2021
Closed
52 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@corinagum corinagum

Labels
backlog Out of scope for the current iteration but it will be evaluated in a future release. Bot Services Required for internal Azure reporting. Do not delete. Do not change color. bug Indicates an unexpected problem or an unintended behavior. customer-replied-to Required for internal reporting. Do not delete. customer-reported Required for internal Azure reporting. Do not delete. front-burner p0 Must Fix. Release-blocker size-s 1 days or less
Projects
None yet
Milestone
R12
Development

Successfully merging a pull request may close this issue.

Allow <s> on sanitize markdown corinagum/BotFramework-WebChat
2 participants
@corinagum @wvpm