1

I'm loading a fragment of a document via AJAX, and I've managed to load 'external' scripts quite well, but I can't execute all the JavaScript contained inline within <script> tags.

Here's an example of the document fragment/HTML I'm trying to load:

    <textarea></textarea>
    <script src="tinyMCE.js" class="ajax-script"></script>
    <script class="ajax-script">
        alert("I'm inline");
        tinymce.init({
            selector: 'textarea',
        });
    </script>

Here's the JavaScript code I use to load this document (on XHR status 200):

    // * This response is HTML
    response = xhr.responseText;

    // * Set the innerHTML of the body to the response HTML
    document.body.innerHTML = response;

    // * Find all scripts with ajax-script class
    responseScripts = document.getElementsByClassName('ajax-script');

    // * Loop through all those scripts
    for (i = responseScripts.length; i--;) {

        // * Create a 'clone' script element
        cloneScript = document.createElement('script');

        // * If the response script has a src, add it to the clone
        if(responseScripts[0].src) {
            cloneScript.src = responseScripts[0].src;
        }

        // * If the response script has 'inline code', add it 
        if(responseScripts[0].innerHTML) {
            cloneScript.innerHTML = responseScripts[0].innerHTML;
        }

        // * Remove the original script
        responseScripts[0].parentNode.removeChild(responseScripts[0]);

        // * Append the clone script to the document
        document.body.appendChild(cloneScript);
    }

So, with this example, only the alert("I'm inline"); part of the inline code is getting executed, but not the rest. No console errors, no nothing, it just seems like the browser is ignoring the tinymce.init() part.

I don't know if this has to do with TinyMCE itself? But why would it? I've also tried evaling the code, but to no luck. After the document has loaded, I can just copy the tinymce.init() and paste it into the console, and the text editor actually shows (because tinymce.init() gets executed).

Is there any reason you can think of that explains why only the alert function gets called, but not the rest? Do you see anything wrong with this way of loading the script?

Thanks.

NameJames
  • 33
  • 6
  • I really dislike code that relies on side effects in DOM lists by removing DOM elements because it looks buggy until you realize there's a side effect in play. Why not just actually process the list from back to front and not have to rely on this uncommon DOM feature? – jfriend00 Mar 30 '15 at 01:24
  • Do you mean the fact that I'm accessing each DOM elements with `[0]`? Well, even if I loop normally, how could I not rely on that? Because whenever I remove an element from the list, the list "updates" and I can't use nothing but the `0` index to access the elements that remain in the list. I'm doing it this way because I couldn't find another one, but any advice on this is great. How could I do it differently? Perhaps turning the list into a "regular" array? Or going backwards but using the actual index, maybe? – NameJames Mar 30 '15 at 01:36
  • You have another issue here. inline script tags assume they are executed in the order they appear in the page. But, the way you are appending the scripts, you are executing them all async. They may not execute in the proper order. If one depends on another being loaded first, that could cause a script error. You probably have to wait for each script to load and run before loading the next one. Whether this is an actual problem in your page depends entirely upon the scripts and timing and cache hits, but it is problem for the general case or worst case. – jfriend00 Mar 30 '15 at 01:56
  • Probably safest to just make the list into an actual array (that won't change) because you probably do have to execute front to back in order. Or, just don't delete the old script elements until the end (in a separate loop) to avoid causing the dynamic list to change while operating on it. – jfriend00 Mar 30 '15 at 01:56
  • On second thought, you almost have to make the dynamic list into an actual array because executing the scripts themselves could insert other scripts which could cause side affects on your dynamic list that will mess you up. – jfriend00 Mar 30 '15 at 01:59
  • And that's why I can't really loop backwards while using the actual index, I think. Because I can do that, but the scripts don't get appended in order. Or maybe I could reverse the list? Either way, I didn't know that they were executing async, I assumed it was all synchronous. How can I 'check' when one is done loading before appending the next? – NameJames Mar 30 '15 at 01:59
  • In modern browsers, the script tag has an `onload` attribute and fires a `load` event when the script is loaded. – jfriend00 Mar 30 '15 at 02:00
  • Oh, that's true. I hadn't thought about that scenario (scripts adding other scripts). I think that it'd be a not very likely scenario, because the script would have to insert the other script into that specific list, right? But it's still worth considering, thanks. I will turn it into an array. – NameJames Mar 30 '15 at 02:02
  • Ok, great, I didn't know. I'll see what I can do with the `load` event, thanks. – NameJames Mar 30 '15 at 02:04
  • One more question: If the scripts to load were all external (via the `src` attribute), would that execute synchronously? (Assuming they are appended in the correct order) – NameJames Mar 30 '15 at 02:08
  • Ha! I found quite a nasty way of doing this with the `onload` callback. It's a "recursive" function I guess, because `onload` calls the function again until there are no DOM elems in the list. It's nasty because it relies on the side effect you mentioned, though. Will try to think of another way of doing this. – NameJames Mar 30 '15 at 02:18
  • No, inserting ` – jfriend00 Mar 30 '15 at 02:47

3 Answers3

2

Although David Water's response did the trick in Firefox, it didn't in Chrome. So, by following the suggestions of jfriend, I turned the dynamic list into an array, and made sure the scripts loaded synchronously. Here's what I did:

response = xhr.responseText;
document.body.innerHTML = response;
responseScripts = document.getElementsByClassName('ajax-script');
i = 0;
// * Convert DOM dynamic list into an array
function listToArray(list) {
  var array = [];
  for (var i = list.length >>> 0; i--;) { 
    array[i] = list[i];
  }
  return array;
}
function loadScripts() {
    if(responseScripts[i]) {
        cloneScript = document.createElement('script');
        if(responseScripts[i].src) {
            cloneScript.src = responseScripts[i].src;
        }
        if(responseScripts[i].innerHTML) {
            cloneScript.innerHTML = responseScripts[i].innerHTML;
        }
        responseScripts[i].parentNode.removeChild(responseScripts[i]);
        document.body.appendChild(cloneScript);
        if(cloneScript.src) {
            // * For external scripts, wait 'till they load
            cloneScript.onload = function () {
                loadScripts(i++);
            };
        } else {
            // * For inline scripts, just call the function again
            loadScripts(i++);
        }
    }
}
if(responseScripts.length > 0) {
    responseScripts = listToArray(responseScripts);
    // * Start loading the scripts
    loadScripts();
}
NameJames
  • 33
  • 6
  • Why this form instead of a plain `for` loop?: `for (var i = list.length >>> 0; i--;) { ` – jfriend00 Mar 30 '15 at 03:54
  • Oh, I just stole that in a rush from [here](http://stackoverflow.com/questions/2735067/how-to-convert-a-dom-node-list-to-an-array-in-javascript). It says it's making sure length is an unsigned integer. However, now that I think about it, doing >>> 0 on a negative number gives back a crazy big number. Is this what you mean? – NameJames Mar 30 '15 at 04:06
  • But seriously, why would length not be an integer? I think I'll remove that bit. Haha. Either way, looping backwards is nice since I can start with the list from 0. I think that's less confusing than decrementing the index everytime. – NameJames Mar 30 '15 at 04:12
1

Here's a cleaned up version of the idea in your answer (derived from my comments). Here's a partial list of the improvements:

  1. Turned it into a function so all code is scoped local
  2. Declared all variables used with var
  3. Made it generic with passed in arguments for code reuse
  4. Avoided stack build-up of actual recursion
  5. Put some repeated references into local variables
  6. Used Array.slice trick to make array copy
  7. Since a script tag can only have a src= or .innerHTML (not both), made it into an if/else
  8. Combine some common code into one place
  9. Switched to .textContent from .innerHTML since all we want is text

Code:

function insertHtmlAndExecutescripts(elem, html, cls) {
    elem.innerHTML = html;
    // make actual array of all ajax-script tags
    var scripts = Array.prototype.slice.call(elem.getElementsByClassName(cls), 0);
    var i = 0;

    function loadScripts() {
        if (i < scripts.length) {
            var cloneScript = document.createElement('script');
            var tag = scripts[i++];
            if (tag.src) {
                cloneScript.src = tag.src;
                // * For external scripts, wait 'till they load
                cloneScript.onload = function () {
                    loadScripts();
                }
                document.body.appendChild(cloneScript);
            } else if (tag.innerHTML) {
                cloneScript.textContent = tag.textContent;
                document.body.appendChild(cloneScript);
                // avoid stack build-up of actual recursion
                setTimeout(function() {
                    loadScripts();
                }, 0);
            }

            // remove the original embedded script tag (this is likely not necessary)
            tag.parentNode.removeChild(tag);
        }
    }
    loadScripts();
}
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • Great! All the variables I used weren't declared with `var` because I had already declared them at the top of the function (which I didn't include in my code sample). This code is way better than mine, though, with all of the improvements. I guess `setTimeout(fn, 0)` puts everything in a queue, at the end of execution? Anyway, this is awesome, really, thanks. – NameJames Mar 30 '15 at 04:40
  • @NameJames - made one more change - Switched to `.textContent` from `.innerHTML` because we only want text. – jfriend00 Mar 30 '15 at 04:42
  • is `.textContent` as widely supported as `.innerHTML`? Hmm, apparently IE 9+. – NameJames Mar 30 '15 at 04:47
  • 1
    @NameJames - it's IE9 or higher. If you want earlier versions of IE, they use `.innerText`. Maybe `.innerHTML` causes no problems, but I wasn't sure so I wanted to be safer. – jfriend00 Mar 30 '15 at 04:49
0

Try logging what you get back from responseScripts[0].innerHTML if this is the full script then you can just eval the result. If this is just the first line then you have found your problem.

David Waters
  • 11,979
  • 7
  • 41
  • 76
  • `eval()` does the trick in Firefox but not in Chrome. Although the tinyMCE external script is supposedly already in DOM before `eval()` is executed, in Chrome, `tinymce` appears undefined, I guess because the script didn't load fully before executing the eval. Is there any way I can ensure the external script loads before doing the eval? – NameJames Mar 30 '15 at 01:48