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

Bring back scripts evaluation in TextPanel #26412

Closed
wants to merge 1 commit into from
Closed

Conversation

dprokop
Copy link
Member

@dprokop dprokop commented Jul 17, 2020

Fixes #26406

When html sanitization is disabled we need to process html against any inline script tag occurrences and eval those scripts to have an effect. React's dangerouslySetInnerHTML uses innerHTML which does not evaluate scripts: https://developer.mozilla.org/en-US/docs/Web/API/Element/innerHTML

@ryantxu I couldn't get the test for new logic to pass, any ideas? Seems like the parsed text is undefined in the test env, no idea why. Maybe some jsdom issue?

@dprokop dprokop requested review from torkelo, ryantxu, a team and mckn and removed request for a team July 17, 2020 14:51
@dprokop dprokop added this to the 7.1.1 milestone Jul 17, 2020
@dprokop dprokop self-assigned this Jul 17, 2020
@dprokop
Copy link
Member Author

dprokop commented Jul 17, 2020

Alternatively we could use https://github.com/christo-pr/dangerously-set-html-content

@dprokop dprokop requested a review from daniellee July 17, 2020 14:59
@ryantxu
Copy link
Member

ryantxu commented Jul 17, 2020

ugg -- maybe we can take the opportunity to move script to an explicit script field?

@dprokop
Copy link
Member Author

dprokop commented Jul 17, 2020

ugg -- maybe we can take the opportunity to move script to an explicit script field?

Well, we could, but this will still be breaking change. And following this pattern we would have to provide style field too. I think with the Monaco editor having all the things in one editor is just fine.

@ryantxu
Copy link
Member

ryantxu commented Jul 17, 2020

for sure the approach in https://github.com/christo-pr/dangerously-set-html-content is better :)

const slotHtml = document.createRange().createContextualFragment(html)

@dprokop
Copy link
Member Author

dprokop commented Jul 17, 2020

yes, i'm testing this lib as we speak

@dprokop
Copy link
Member Author

dprokop commented Jul 17, 2020

Closing in favour of #26413

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

Successfully merging this pull request may close these issues.

Custom JS tabs menu not responding since move to 7.1.0 (from 6.6.2)
3 participants