Most of my comments should be seen as "TODOs", as I really don't want to
delay landing: this is highly experimental anyway, and we know it'll
need some more work.


http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/AppSpace.java
File dev/codeserver/java/com/google/gwt/dev/codeserver/AppSpace.java
(right):

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/AppSpace.java#newcode2
dev/codeserver/java/com/google/gwt/dev/codeserver/AppSpace.java:2: *
Copyright 2011 Google Inc.
2012 ?

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/ModuleState.java
File dev/codeserver/java/com/google/gwt/dev/codeserver/ModuleState.java
(right):

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/ModuleState.java#newcode49
dev/codeserver/java/com/google/gwt/dev/codeserver/ModuleState.java:49:
defaultProps.put("user.agent", "safari");
This is just a "default" for the very first compilation, right?
(but hard-coding things that way will prevent detecting the "right"
permutation later on, no?)

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/Options.java
File dev/codeserver/java/com/google/gwt/dev/codeserver/Options.java
(right):

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/Options.java#newcode134
dev/codeserver/java/com/google/gwt/dev/codeserver/Options.java:134:
private class SourceFlag extends ArgHandler {
Why not use ArgHandlerDir?

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/Options.java#newcode148
dev/codeserver/java/com/google/gwt/dev/codeserver/Options.java:148:
return "The root of a directory tree containing GWT source code to
compile.";
Does this mean SuperDevMode no longer loads sources from the classpath?

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java
File dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java
(right):

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java#newcode164
dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java:164: //
This is a GET because a bookmarklet can call it from a different origin
(JSONP).
We should probably use CORS and XMLHttpRequest with a POST request,
instead of JSON-P. The bookmarklet could handle XDomainRequest for IE8/9
and even possibly create an HTML form if we want to support IE6/7.

White-listing origins from command-line arguments would also add some
security.

http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java#newcode456
dev/codeserver/java/com/google/gwt/dev/codeserver/WebServer.java:456:
private static String escapeHtmlCharacters(String line) {
Couldn't we use SafeHtml.fromString() here?

http://gwt-code-reviews.appspot.com/1727804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

Reply via email to