Thanks!
On 2010/10/12 19:46:03, jgw wrote:
Wow, that's one hell of a refactoring. I added a couple of 'taste'
comments that
you're free to do with as you will; and there's one spot where I think
you're
missing a 'var' or two.
No need to re-review. Once you're satisfied, feel free to submit.
http://gwt-code-reviews.appspot.com/941802/diff/24001/25006
File
dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js
(right):
http://gwt-code-reviews.appspot.com/941802/diff/24001/25006#newcode31
dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js:31:
metaVal = __gwt_getMetaProperty('baseUrl');
I think you need a declaration for 'base' and 'metaVal' here.
http://gwt-code-reviews.appspot.com/941802/diff/24001/25022
File
dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java
(right):
http://gwt-code-reviews.appspot.com/941802/diff/24001/25022#newcode59
dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java:59:
protected
boolean installCode = true;
When would you want to turn this off?
http://gwt-code-reviews.appspot.com/941802/diff/24001/25022#newcode79
dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java:79:
"com/google/gwt/core/ext/linker/impl/waitForBodyLoaded.js";
I assume all of the above fields are meant to be overwritten by
subclasses. I
think they're fairly self-explanatory, but could we not make them
template
methods instead (e.g., "boolean installCode()", "String
getProcessMetasJs()",
etc)? Seems like it might be slightly less prone to confusion in
subclasses.
It's a taste thing, though, so I'll leave it to your discretion.
http://gwt-code-reviews.appspot.com/941802/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
