http://gwt-code-reviews.appspot.com/1451818/diff/10011/user/src/com/google/gwt/core/client/ScriptInjector.java File user/src/com/google/gwt/core/client/ScriptInjector.java (right):
http://gwt-code-reviews.appspot.com/1451818/diff/10011/user/src/com/google/gwt/core/client/ScriptInjector.java#newcode55 user/src/com/google/gwt/core/client/ScriptInjector.java:55: private boolean removeTag = true; On 2011/06/21 16:31:50, zundel wrote:
On 2011/06/21 14:51:15, jlabanca wrote: > removeTag should default to false. I think the most common use case
is to
> inject a library, in which case you do not want to remove it. > > What was the rationale for defaulting to true?
This is a trick lifted from the linker. Removing the script tag does
not cause
the definitions in the script to go away, it just removes it from the
DOM saving
some memory
Here's one article on it: http://www.javascriptkit.com/javatutors/loadjavascriptcss2.shtml
Weird. http://gwt-code-reviews.appspot.com/1451818/diff/10011/user/test/com/google/gwt/core/client/ScriptInjectorTest.java File user/test/com/google/gwt/core/client/ScriptInjectorTest.java (right): http://gwt-code-reviews.appspot.com/1451818/diff/10011/user/test/com/google/gwt/core/client/ScriptInjectorTest.java#newcode73 user/test/com/google/gwt/core/client/ScriptInjectorTest.java:73: assertFalse("cleanup failed", nativeTest1Worked()); On 2011/06/21 16:31:50, zundel wrote:
On 2011/06/21 14:51:15, jlabanca wrote: > If the script fails to inject (worked is false), you'll hit this
error, which
is > misleading. If you move the assertTrue() above the assertFalse(),
then the
fail > case returns a more meaningful error. > > Of course that means you won't cleanup, but if an error occurs
anyway, that's
> okay. And, if an error occurs, you probably don't have to cleanup
because the
> script was never injected.
If the injection fails, then nativeTest1Worked() returns false... so
this test
would pass.
SGTM http://gwt-code-reviews.appspot.com/1451818/diff/5008/user/src/com/google/gwt/core/client/ScriptInjector.java File user/src/com/google/gwt/core/client/ScriptInjector.java (right): http://gwt-code-reviews.appspot.com/1451818/diff/5008/user/src/com/google/gwt/core/client/ScriptInjector.java#newcode245 user/src/com/google/gwt/core/client/ScriptInjector.java:245: scriptElement.onload = function() { All javascript handles should be wrapped in $entry(...) to ensure proper reentry into GWT code, which is essential for catching exceptions. That;s probably why you had trouble debugging the bug you mentioned below. Syntax: scriptElement.onload = $entry(function() { // Put stuff here. }); http://gwt-code-reviews.appspot.com/1451818/diff/5008/user/src/com/google/gwt/core/client/ScriptInjector.java#newcode250 user/src/com/google/gwt/core/client/ScriptInjector.java:250: scriptElement.onerror = function() { Wrap in $entry http://gwt-code-reviews.appspot.com/1451818/diff/5008/user/src/com/google/gwt/core/client/ScriptInjector.java#newcode252 user/src/com/google/gwt/core/client/ScriptInjector.java:252: var ex = @com.google.gwt.core.client.CodeDownloadException::new(Ljava/lang/String;)("onerror() called."); spaces http://gwt-code-reviews.appspot.com/1451818/diff/5008/user/src/com/google/gwt/core/client/ScriptInjector.java#newcode255 user/src/com/google/gwt/core/client/ScriptInjector.java:255: scriptElement.onreadystatechange = function() { Wrap in $entry http://gwt-code-reviews.appspot.com/1451818/diff/5008/user/src/com/google/gwt/core/client/ScriptInjector.java#newcode281 user/src/com/google/gwt/core/client/ScriptInjector.java:281: private static void doSuccessCb(Callback<Void, Exception> callback) { On 2011/06/21 16:31:50, zundel wrote:
I introduced a bug in my test and was pulling my hair out to debug it
before I
added this code.
Wrap the onload/onerror functions in $entry, and GWT will forward errors to the UncaughtExceptionHandler. Sorry I missed that before. http://gwt-code-reviews.appspot.com/1451818/diff/5008/user/test/com/google/gwt/core/client/ScriptInjectorTest.java File user/test/com/google/gwt/core/client/ScriptInjectorTest.java (right): http://gwt-code-reviews.appspot.com/1451818/diff/5008/user/test/com/google/gwt/core/client/ScriptInjectorTest.java#newcode63 user/test/com/google/gwt/core/client/ScriptInjectorTest.java:63: public void gwtTearDown() { I'm pretty sure gwtTearDown is protected in GWTTestCase. Probably no reason to make it public here. http://gwt-code-reviews.appspot.com/1451818/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
