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

add security level antiscript option, use rich html format but dont permit script element. #1471

Merged
merged 1 commit into from
Jul 12, 2020

Conversation

vantoan8x
Copy link

… remove all script element.

πŸ“‘ Summary

Brief description about the content of your PR.

Resolves #

πŸ“ Design Decisions

Describe the way your implementation works or what design decisions you made if applicable.

πŸ“‹ Tasks

Make sure you

  • πŸ“– have read the contribution guidelines
  • πŸ’» have added unit/e2e tests (if appropriate)
  • πŸ”– targeted develop branch

@GDFaber
Copy link
Member

GDFaber commented Jun 14, 2020

Hi @vantoan8x, I like where you are going with this PR.

But I wonder if we should cover the use case
"allow only links for click events"
as well while we're at it.

Adding tests for the new settings would be awesome, you can find the tests here:

  • src/utils.spec.js
  • cypress/integration/configuration.spec.js

@vantoan8x
Copy link
Author

vantoan8x commented Jun 14, 2020

Dear @GDFaber,

I will cover test case and add another option "allow only links for click events" for this tommorow in vietname time GMT+7, this now late in my side.

Regards.

@GDFaber
Copy link
Member

GDFaber commented Jun 14, 2020

Sure, take your time for it, there's no need to rush :-)

@knsv
Copy link
Collaborator

knsv commented Jun 30, 2020

@vantoan8x Hi, any updates on this one?

@vantoan8x
Copy link
Author

vantoan8x commented Jul 1, 2020 via email

@vantoan8x vantoan8x force-pushed the securityLevel_antiscript_option branch from 308498f to fce2a16 Compare July 5, 2020 16:07
@vantoan8x
Copy link
Author

Dear @knsv,

I added test case for this function.

Regards.

Copy link
Collaborator

@knsv knsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is a good middle ground that will be appreciated by some sites integrating mermaid. Also I appreciate you adding the test.

@knsv knsv merged commit e93a055 into mermaid-js:develop Jul 12, 2020
@jgreywolf
Copy link
Contributor

Resolves #1376

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants