0

I have a webpage that makes an AJAX request to a PHP script. That PHP script responds with a valid JSON object, and sets the Content-type header to application/json.

The JSON format is as follows (as reported by console.log(JSON.stringify(data)) where data is the JSON response):

{
    "content": "<script type=\"text/javascript\">console.log(\"test\");</script>"
}

I then create a div with the class of "content" (remember data is my AJAX responseText JSON object):

var content = document.createElement("div");
content.setAttribute("class", "content");
content.innerHTML = data.content;

Finally, I append the content div to an existing div on my site.

existingDiv.appendChild(content);

I examine the source in the elements tab of Google Chrome's developer tools because it does a good job of showing what is an actual DOM node, and what is just text. The script block shows up as a DOM node.

Unfortunately, the script is not executed - console.log("test"); does not output test to the developer tools console.

The only option I've seen so far is to use eval() which I'm trying to avoid like the plague. Also, my content might contain more than just a script block. It could contain other HTML markup as well.

What am I missing? How can I get this dynamically added block of Javascript to execute?

A jQuery solution is acceptable.

Pointy
  • 405,095
  • 59
  • 585
  • 614
crush
  • 16,713
  • 9
  • 59
  • 100
  • 1
    Injecting a script tag based on an arbitrary string is exactly the same as using `eval()`, at least regarding the reasons why `eval()` should be avoided. – MCL Feb 08 '13 at 14:07
  • @MCL could you expand more on that in an answer? – crush Feb 08 '13 at 14:17
  • 1
    I could answer that by asking you why you want to avoid `eval()` "like the plague" and then telling you that injecting arbitrary ` – MCL Feb 08 '13 at 14:26
  • The location of the script is on the same domain, and its content can be trusted. My main concerns were with `eval` being perceived as "evil" and with performance as I've heard it is very slow. Your point is a good one though, and well taken. – crush Feb 08 '13 at 14:29
  • 1
    If performance is your only concern, then script injection is the better choice. I wrote a quick fiddle and I must admit, I am a bit suprised. Although both approaches use some kind of "hackish afterwards" code evaluation, the script injection runs remarkably faster. Check it out [here](http://jsfiddle.net/mwEcr/). – MCL Feb 08 '13 at 14:49
  • So I guess it is true that `eval()` is horrendously slow. Thanks for the fiddle! Unfortunately, doing script injection like this would be more complex than I wish because more than just `` can exist in the `data.content` – crush Feb 08 '13 at 14:51

3 Answers3

1

Setting .innerHTML does not evaluate <script> tags; it's just not what browsers do.

If you need to return JavaScript to be executed, you're better off just sending it as a string (without <script> tags) so that you can easily pass it to eval().

edit If you do this with jQuery instead of directly setting innerHTML (that is, if you use $(something).html()), and you add the content directly to something in the DOM (that is, not just an element that's been instantiated but not appended to the DOM), then jQuery will actively find and extract your <script> content and evaluate it.

Pointy
  • 405,095
  • 59
  • 585
  • 614
  • Well it was invalid as you posted it; all anybody can do here is respond to what you post. – Pointy Feb 08 '13 at 14:08
  • Your code is: `content.setAttribute("class", content);` - can you explain what that does, if not set the "class" attribute to be a reference to the element itself? – Pointy Feb 08 '13 at 14:12
  • @crush maybe because you posted a question on Stackoverflow asking people to diagnose problems with your code. – Pointy Feb 08 '13 at 14:14
  • 1
    @crush dude you need to think hard about how you want to act towards people trying to help you for free. – Pointy Feb 08 '13 at 14:16
  • @crush check my update - I think jQuery can help directly, as it strips ` – Pointy Feb 08 '13 at 14:33
  • Look at it in the JSBin editor - it's an empty `
    ` initially, and then a jQuery "ready" handler sets its contents with `.html()` - [try this URL instead](http://jsbin.com/itawof/2/edit) *edit* - oh sorry; it was messed up; I'm having some weird Internet issues here so I must have fat-fingered it accidentally. I think it should be OK now via that new link.
    – Pointy Feb 08 '13 at 14:37
  • `.append()` also seems to work, and work faster, than `.html()` as pointed out below by `eithed`. I'm afraid I have to choose his answer. – crush Feb 08 '13 at 15:09
1

use $.getScript() simple, cachable, straightforward :

http://api.jquery.com/jQuery.getScript/

(obviously you'll have to amend your server script that generates that json containing jscode & change headers)

mikakun
  • 2,203
  • 2
  • 16
  • 24
  • What if my content also contains HTML markup: `{"content" : "

    Hello

    "}` for example.
    – crush Feb 08 '13 at 14:12
  • well then send back a link to the script to get; but that's 2 request to the server instead of one though... or preload the script on page load (is it that big that it has to be loaded separately ?) – mikakun Feb 08 '13 at 14:14
1

A possible solution is to use new Function() as:

$(document).ready(function(){
   var script = "window.alert('hi')";
   new Function(script)();
});

As to why your script is not executing, is what @Pointy suggested (but not entirely + a way to circumvent that), as shown by this code:

<script type='text/javascript'>
$(document).ready(function()
{
    $("<script type='text/javascript'>window.alert('hi');<\/script>").appendTo(document.body); // this will work

    var container = document.createElement("div"); // this won't work
    container.innerHTML = "<script type='text/javascript'>window.alert('hi');<\/script>";
    document.body.appendChild(container);

    var script = document.createElement("script"); // this will
    script.innerHTML = "window.alert('hi');";
    document.body.appendChild(script);
});
</script>

EDIT: added a requested unit test: http://jsperf.com/eithed-hvsa

eithed
  • 3,933
  • 6
  • 40
  • 60
  • That would require me to change the value of `data.content`. Also, this wouldn't handle `data.content` containing regular HTML markup, as well as Javascript. – crush Feb 08 '13 at 14:37
  • That is indeed correct - however, you can either use $.append - as `$(" – eithed Feb 08 '13 at 14:45
  • After adding the `
    ` to the DOM first, then using jQuery's `.html()`, the script is executed, as displayed in Pointy's updated answer.
    – crush Feb 08 '13 at 14:46
  • Yup, as it does what I've suggested :D – eithed Feb 08 '13 at 14:48
  • Well what is more efficient? `$(data.content).appendTo(element);` or `$(element).html(data.content);`? I'm betting on `.html()`. – crush Feb 08 '13 at 14:49
  • 1
    Seeing they operate on different principles... But, to the test unit! http://jsperf.com/eithed-hvsa – eithed Feb 08 '13 at 14:57
  • So `.html()` is slower! Interesting indeed. Also, interestingly enough, the target `div` does not have to be added to the DOM first, for `.append()` to work on it. – crush Feb 08 '13 at 15:01
  • Please add your unit-test and the results to your answer. I have marked yours because it is more efficient than `.html()` and does not require the containing element first be appended to the DOM. – crush Feb 08 '13 at 15:09
  • Nah - appending script is enough for it to work. The thing (I think) that makes `html()` method slower is the fact that the DOM content of the node needs to be first accessed and then is destroyed every time. Appending only adds a node to the end - it shouldn't even need to access the previous content of the XML tree (think of it as adding bits at the end of the queue). However, don't quote me on that :D – eithed Feb 08 '13 at 15:14
  • See, I thought that `$(someHTMLhere)` or `$(element).append(someHTMLHere)` actually iterated the elements in the string of HTML, constructed them one at a time, and then appended them on at a time. Shame on me for never digging into that and seeing what it does under the hood. – crush Feb 08 '13 at 15:30