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

Alternative to dangerouslySetInnerHTML in block save #421

Closed
aduth opened this issue Apr 13, 2017 · 2 comments
Closed

Alternative to dangerouslySetInnerHTML in block save #421

aduth opened this issue Apr 13, 2017 · 2 comments
Labels
[Feature] Blocks Overall functionality of blocks

Comments

@aduth
Copy link
Member

aduth commented Apr 13, 2017

Related: #419 (comment)

By default, React escapes string children, converting HTML tags to their encoded equivalents. On the web, this is considered a "good thing" in helping avoid unintentional cross-site scripting attacks. However, in the serialization step of converting a block to its string form, this complicates the block's save function where we need to be able to include raw HTML in the generated output to be saved for post_content. The block's save function is never truly rendered, only ever passed through ReactDOMServer.renderToStaticMarkup to generate the composed content of the post. In order to include HTML in the serialized content, a developer must use the special dangerouslySetInnerHTML prop.

What's the problem? It's not obvious that a developer would need to use dangerouslySetInnerHTML to include arbitrary markup in the saved post content. Additionally the property name is very intentional in its scariness, which is likely off-putting for our legitimate use-case.

Why do we need it? We may consider exploring alternatives, but for the moment particularly the Editable (TinyMCE) component is affected by this in its value being reflected as raw HTML.

@aduth aduth added the [Feature] Blocks Overall functionality of blocks label Apr 13, 2017
@aduth
Copy link
Member Author

aduth commented Apr 13, 2017

A few ideas:

  • Could we disable default string escaping in our ReactDOMServer.renderToString abstraction?
  • Could we traverse the tree of virtual DOM nodes to automatically convert string children to a dangerouslySetInnerHTML prop?
  • Do we find another solution than to accommodate the Editable component's value as raw HTML? Perhaps some compatible tree structure?

@youknowriad
Copy link
Contributor

Could we traverse the tree of virtual DOM nodes to automatically convert string children to a dangerouslySetInnerHTML prop?

This could confuse block authors. If they're familiar with React, maybe they explicitly expect their content to be escaped and it won't be the case.$

Do we find another solution than to accommodate the Editable component's value as raw HTML? Perhaps some compatible tree structure?

Seems like a good idea to explore, though, it won't be possible to reuse elements from the edit method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks
Projects
None yet
Development

No branches or pull requests

2 participants