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 eval support for script tags #617

Merged
merged 1 commit into from
Feb 28, 2016

Conversation

amadeus
Copy link
Contributor

@amadeus amadeus commented Feb 20, 2016

This commit adds support for eval-ing script.text. This is a
requirement for Phaser when using their script preloading functionality.

Phaser loads scripts through XMLHttpRequest, then sets the responseText
to the script.text attribute.

More information can be seen here:

https://github.com/photonstorm/phaser/blob/47123c192df7f3a78ac25110a9a72b72f61417c2/src/loader/Loader.js#L2750-L2761

This commit adds support for eval-ing script.text.  This is a
requirement for Phaser when using their script preloading functionality.

Phaser loads scripts through XMLHttpRequest, then sets the responseText
to the script.text attribute.

More information can be seen here:

https://github.com/photonstorm/phaser/blob/47123c192df7f3a78ac25110a9a72b72f61417c2/src/loader/Loader.js#L2750-L2761
@phoboslab
Copy link
Owner

Do we need to support script.innerHTML as well? Or do browsers only eval the .text property and ignore innerHTML?

@amadeus
Copy link
Contributor Author

amadeus commented Feb 22, 2016

@phoboslab I can't say for sure. I did .text because that's what Phaser did.

The more I think about it, .text would be the proper one - since you would want to be interpreting escaped html entities properly.

@amadeus
Copy link
Contributor Author

amadeus commented Feb 22, 2016

The only real question I have though - should we put that eval in a setTimeout at 0? Would need to do some browser testing to see if reading that script happens async after it gets appended in a browser or immediately. Also it would help of there's a JS error in that file, to not interrupt the JS context.

@amadeus
Copy link
Contributor Author

amadeus commented Feb 22, 2016

Annnnd, it looks like it's interpreted immediately. So the code I have submitted is good. Demo here:

https://jsfiddle.net/Lsga8otx/

phoboslab added a commit that referenced this pull request Feb 28, 2016
@phoboslab phoboslab merged commit 67524aa into phoboslab:master Feb 28, 2016
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.

None yet

2 participants