views:

139

answers:

5

Hi,

I am trying to format this line of code in my popup window, but i am facing unterminated string literal error.

Can somebody please tell me how best I could format this.

window.setTimeout("winId.document.write('<script src="../js/tiny_mce/tiny_mce.js" type="text/javascript"></script>\n')", 10);

Also point out if this particular line of code would work fine in the popup?

+1  A: 

It's because you're using nested double quotes. Quotes delimit strings so when you get to the second one, it thinks the string has ended, as you can see from the colour highlighting in the code you posted. You need to escape them with \":

window.setTimeout("winId.document.write('<script src=\"../js/tiny_mce/tiny_mce.js\" type=\"text/javascript\"></script>\n')", 10);
DisgruntledGoat
I am still having the same `unterminated string` error in the firebug console after using the above snippet
dkris
+3  A: 

You may try it by escaping the double-quotes and <>, as well as \n:

window.setTimeout("winId.document.write('\<script src=\"../js/tiny_mce/tiny_mce.js\" type=\"text/javascript\"\>\</script\>\\n')", 10);
marapet
I am still having the same `unterminated string` error in the firebug console after using the above snippet
dkris
Check out http://stackoverflow.com/questions/1081573/escaping-double-quotes-in-javascript-onclick-event-handler and try \x22 instead of \"
marapet
Other than that, I second the anonymous function mentioned in an other answer, it's a nicer solution.
marapet
@marapet +1 for your suggestion as you pointed out the \x22 method. Thanks for helping out.
dkris
Now it should be working - escaping < and > with a backslash was also needed. The same for \n which became \\n.
marapet
A: 

You have to escape the " in your script tag attributes with \" so it would read:

window.setTimeout("winId.document.write('<script src=\"../js/tiny_mce/tiny_mce.js\" type=\"text/javascript\"></script>\n')", 10);
Charlie boy
+4  A: 

Best not to use a string, but an anonymous function instead:

window.setTimeout(function () {
    winId.document.write(
      '<script src="../js/tiny_mce/tiny_mce.js" type="text/javascript"></script>\n'
    );
}, 10);

Using strings in setTimeout and setInterval is closely related to eval(), and should only be used in rare cases. See http://dev.opera.com/articles/view/efficient-javascript/?page=2

It might also be worth noting that document.write() will not work correctly on an already parsed document. Different browsers will give different results, most will clear the contents. The alternative is to add the script using the DOM:

window.setTimeout(function () {
    var winDoc = winId.document;
    var sEl = winDoc.createElement("script");
    sEl.src = "../js/tiny_mce/tiny_mce.js";
    winDoc.getElementsByTagName("head")[0].appendChild(sEL);
}, 10);
Andy E
@Andy thanks a lot for the help! I have a quick question. The solution is working i.e the JS file is getting included in the popup only on IE6 and Opera and is failing on Chrome,IE8 and Safari.Any idea on why this might be happening?
dkris
@dkris: any error messages? What is the location of your popup window and, does the relative JS source match up with it? Also, in Chrome, use the developer tools (Ctrl+Shift+I) to inspect the elements in the document to see if the script element is successfully added to the popup.
Andy E
@Andy just noticed on Chrome `Uncaught ReferenceError: tinyMCE is not defined` . I guess this means its not loading the source file
dkris
@Andy The TinyMCE script is not loading and desired effect is not achieved. Here is some additional info - Screen shot of the page http://img231.imageshack.us/img231/7190/72961203.png - Rendered HTML http://pastebin.org/295861 - Original JSPF http://pastebin.org/297641
dkris
@Andy here is what I have finally used. But i guess i'm missing something important here http://pastebin.org/297670 and I have removed the setTimeout
dkris
@dkris: I can only see a few points of concern. One, purely speculation, when combining DOM manipulation and `document.write()` elements added to the DOM before parsing may not be subject to completion before the `onload` event fires. Scripts added to the DOM are certainly asynchronous and behave differently to scripts that are part of the HTML and `document.write()`. It could be, and seems likely in the browser where it fails, that `onload="inittextarea()"` fires before the script download completes and this causes the error because TinyMCE is not available yet... *(continued...)*
Andy E
The options are to return to using `document.write()` without a timer or using another timer to check for the existence of TinyMCE before running your functions.
Andy E
@Andy Thanks. This helped to an extent. I guess this was the root cause.After doing this http://pastebin.org/297788 the issue was fixed on Chrome,IE6,Safari,Opera and Firefox.For some strange reason IE8 still says `tinyMCE is not defined` meaning js files wasn't laoded.Is it a browser issue?
dkris
@dkris: I'm not sure why it might not be working in IE8, maybe you could ask that in another question to see if anyone else knows or even has any better ideas on how to achieve your desired effect. Chances are, if it's not working in IE8 then it's probably not working in IE7 either.
Andy E
@Andy Ya its not working on IE7 either. Thanks a lot. You have helped me solve the issue to an extent and given me a perspective on how to solve the issue.
dkris
+2  A: 

Aside from you needing to escape your quotes (as other people have mentioned), you cannot include "</script>" (even if it's within a string) anywhere within a <script> tag

Use:

window.setTimeout(function () {
    winId.document.write(
      '<script src="../js/tiny_mce/tiny_mce.js" type="text/javascript"></scr' + 'ipt>\n'
    );
}, 10);

Instead.

Matt
+1, it didn't occur to me that the code might have been residing in a script tag, I'm surprised I missed that. I prefer to use `<\/script>` rather than concatenating, though.
Andy E
@Matt So does this mean I need to have a matching closing tag for the `script` tag in @Andy's approach?
dkris
@dkris: If you're using Andy's first approach, you'll need to escape the "`</script>`" using either "`</scr' + 'ipt>`" or "`<\/script>`". If you're using his second idea, you don't need to alter anything.
Matt