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

vdom innerHTML implementation should append a text node for script tags #2156

Closed
DesignByOnyx opened this Issue Jan 1, 2016 · 3 comments

Comments

Projects
None yet
2 participants
@DesignByOnyx
Contributor

DesignByOnyx commented Jan 1, 2016

The innards of the innerHTML implementation in vdom does this:

...
var frag = parser.parse(""+html);
this.appendChild(frag);

However, script tags contents are CDATA and should not be parsed as html. Instead, a text node should just be appended with whatever content is given to it.

@DesignByOnyx

This comment has been minimized.

Show comment
Hide comment
@DesignByOnyx

DesignByOnyx Jan 2, 2016

Contributor

The same should probably also be applied to STYLE tags. Please advise.

Contributor

DesignByOnyx commented Jan 2, 2016

The same should probably also be applied to STYLE tags. Please advise.

@justinbmeyer justinbmeyer added the bug label Jan 3, 2016

@justinbmeyer justinbmeyer added this to the 2.3.8 milestone Jan 3, 2016

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jan 3, 2016

Contributor

@DesignByOnyx would this also do the wrong thing if I did:

var div = document.createElement('div');
div.innerHTML = "<script>var foo='bar';</script>";

Is parser.parse(""+html) aware of this rule?

Contributor

justinbmeyer commented Jan 3, 2016

@DesignByOnyx would this also do the wrong thing if I did:

var div = document.createElement('div');
div.innerHTML = "<script>var foo='bar';</script>";

Is parser.parse(""+html) aware of this rule?

@DesignByOnyx

This comment has been minimized.

Show comment
Hide comment
@DesignByOnyx

DesignByOnyx Jan 4, 2016

Contributor

@justinbmeyer - I think the parser handles that properly already. I haven't tested it, but this comment in the tokenizer makes me think that it has been accounted for.

The reason this bug surfaced for me is because the react integration I am building uses a <script> tag, and internally react is using innerHTML to set the contents for the tag. This was not a problem until the contents contained some JSON data that contained a string like this: "Foo<Bar". Adding spaces around the < fixed it, but unfortunately this is a trademarked name used by our client and the spaces cannot be there.

Contributor

DesignByOnyx commented Jan 4, 2016

@justinbmeyer - I think the parser handles that properly already. I haven't tested it, but this comment in the tokenizer makes me think that it has been accounted for.

The reason this bug surfaced for me is because the react integration I am building uses a <script> tag, and internally react is using innerHTML to set the contents for the tag. This was not a problem until the contents contained some JSON data that contained a string like this: "Foo<Bar". Adding spaces around the < fixed it, but unfortunately this is a trademarked name used by our client and the spaces cannot be there.

@daffl daffl closed this in #2157 Jan 4, 2016

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