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

Don't execute <script> tags w/ createElement mode #5146

Merged
merged 1 commit into from
Oct 12, 2015

Conversation

sophiebits
Copy link
Contributor

Each script will execute at most once so we could also set .textContent to something like ;, then add it to the document, then change the .textContent, but this seems like the simplest approach. See http://dev.w3.org/html5/spec-preview/the-script-element.html for details.

var div = ownerDocument.createElement('div');
div.innerHTML =
'<' + this._currentElement.type +
'></' + this._currentElement.type + '>';
Copy link
Member

Choose a reason for hiding this comment

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

I guess we know here that this is "script" so we probably could just hard code that string. Otherwise, template strings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this for consistency with the other HTML generation (it could be capitalized differently). Also the token </script> would make browsers sad if someone tries to put the entire source of React within a script tag. I can do template strings except it doesn't fit on one line anyway…

Each script will execute at most once so we could also set `.textContent` to something like `;`, then add it to the document, then change the `.textContent`, but this seems like the simplest approach. See http://dev.w3.org/html5/spec-preview/the-script-element.html for details.
sophiebits added a commit that referenced this pull request Oct 12, 2015
Don't execute <script> tags w/ createElement mode
@sophiebits sophiebits merged commit b32835e into facebook:master Oct 12, 2015
@facebook-github-bot
Copy link

@spicyj updated the pull request.

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.

3 participants