Smalltalk › Amber Smalltalk

Bug report (and proposed fix) for amber.js - writeScriptTag()

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

4 messages Options

Tom Rake

Bug report (and proposed fix) for amber.js - writeScriptTag()

First, Thank you Nicolas (and all the other contributers) for amber.

I am trying to use Google Charts API from amber (in Chrome on Windows 7) and in my debugging I found this code in amber.js where amber will trash the document element by writing a script tag over it.

	function writeScriptTag(src) {
		var scriptString = '<script src="' + src + '" type="text/javascript"></script>';
		document.write(scriptString);
	};

This code is called from loadDependencies() to load JQuery if JQuery has not already been loaded. I believe the correct way to add a script tag is to append the script tag to the head element of the document. My code for this function appendJSScriptTag(src) is in my amber fork at https://github.com/tomrake/amber


loadDependencies() uses writeScriptTag() directly and I believe that it should use loadJSViaScriptTag() but I need further analysis and testing to see if that is the case.

My Google Chart API examples are almost ready to be made public.

Manfred Kröhnert

Re: Bug report (and proposed fix) for amber.js - writeScriptTag()

Hi Thomas,

you only need to add the script tag in the head element of the document if you want to load the script before the document is fully loaded.

Adding the script tag to the end of the document causes the page to load and display while the script is loaded afterwards.

Hope this helps,

Manfred

On Sat, Nov 10, 2012 at 7:57 PM, Thomas Rake <[hidden email]> wrote:

First, Thank you Nicolas (and all the other contributers) for amber.

I am trying to use Google Charts API from amber (in Chrome on Windows 7) and in my debugging I found this code in amber.js where amber will trash the document element by writing a script tag over it.

	function writeScriptTag(src) {
		var scriptString = '<script src="' + src + '" type="text/javascript"></script>';
		document.write(scriptString);
	};


This code is called from loadDependencies() to load JQuery if JQuery has not already been loaded. I believe the correct way to add a script tag is to append the script tag to the head element of the document. My code for this function appendJSScriptTag(src) is in my amber fork at https://github.com/tomrake/amber



loadDependencies() uses writeScriptTag() directly and I believe that it should use loadJSViaScriptTag() but I need further analysis and testing to see if that is the case.

My Google Chart API examples are almost ready to be made public.

Tom Rake

Re: Bug report (and proposed fix) for amber.js - writeScriptTag()

In reply to this post by Tom Rake

I want to revise my post....

It is NOT obvious from the code that writeScriptTag() trashes the document object. This code will write a script code at the position in the document where the function is called and this is where loadAmber() is called. Since I am following the intro examples which place loadAmber() call in the head.

I am trying to reproduce my debugging session where writeScriptTag() is in the debug trace. When I get that I will post it. What I expect to post is an html file that loads amber and other Javascript libraries.

Remember that at this point in loadAmber() we are trying to load jQuery because amber depends on jQuery. If jQuery has been loaded before amber this code is not executed.

On Saturday, November 10, 2012 1:57:07 PM UTC-5, Thomas Rake wrote:

First, Thank you Nicolas (and all the other contributers) for amber.

I am trying to use Google Charts API from amber (in Chrome on Windows 7) and in my debugging I found this code in amber.js where amber will trash the document element by writing a script tag over it.

	function writeScriptTag(src) {
		var scriptString = '<script src="' + src + '" type="text/javascript"></script>';
		document.write(scriptString);
	};

This code is called from loadDependencies() to load JQuery if JQuery has not already been loaded. I believe the correct way to add a script tag is to append the script tag to the head element of the document. My code for this function appendJSScriptTag(src) is in my amber fork at https://github.com/tomrake/amber


loadDependencies() uses writeScriptTag() directly and I believe that it should use loadJSViaScriptTag() but I need further analysis and testing to see if that is the case.

My Google Chart API examples are almost ready to be made public.

Nicolas Petton

Re: Bug report (and proposed fix) for amber.js - writeScriptTag()

In reply to this post by Tom Rake

Thanks for reporting (and bonus points for the fix ;-) )!

We are used to pull request, next time you can just create an issue on
github and send me a pull request if you also have a fix.

PS: Are you going to put your examples online?

Cheers,
Nico

Thomas Rake <[hidden email]> writes:

> First, Thank you Nicolas (and all the other contributers) for amber.
>
> I am trying to use Google Charts API from amber (in Chrome on Windows 7)
> and in my debugging I found this code in amber.js where amber will trash
> the document element by writing a script tag over it.
>
>
> function writeScriptTag(src) {
> var scriptString = '<script src="' + src + '" type="text/javascript"></script>';
> document.write(scriptString);
> };
>
> This code is called from loadDependencies() to load JQuery if JQuery has not already been loaded. I believe the correct way to add a script tag is to append the script tag to the head element of the document. My code for this function appendJSScriptTag(src) is in my amber fork at https://github.com/tomrake/amber
>
>
> loadDependencies() uses writeScriptTag() directly and I believe that it should use loadJSViaScriptTag() but I need further analysis and testing to see if that is the case.
>
>
> My Google Chart API examples are almost ready to be made public.
>

--
Nicolas Petton
http://nicolas-petton.fr