[gwt-contrib] Re: Fix CssObfuscationStyle to escape stable-shorttype types, so it works for cases like MyClientB... (issue1795803)
On 2012/07/26 00:33:24, ehwang wrote: LGTM http://gwt-code-reviews.appspot.com/1795803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] GWT 2.5 Final - Next Week
Hi Guys - We are going to be pushing the 2.5 RC2 (plus 1-2 minor cherrypicks) to Final next week - if there are any problems, please let me (unn...@google.com) know before Monday morning. Note that problems includes only regressions - so stuff that worked in 2.4 and is now broken in 2.5. Thanks everyone! - Unnur -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Update validator version to Final rather than GA (issue1847805)
Reviewers: rchandia, Description: Update validator version to Final rather than GA Review by: rchan...@google.com Please review this at http://gwt-code-reviews.appspot.com/1847805/ Affected files: M samples/dynatablerf/pom.xml M samples/mobilewebapp/pom.xml M samples/validation/pom.xml Index: samples/dynatablerf/pom.xml === --- samples/dynatablerf/pom.xml (revision 11305) +++ samples/dynatablerf/pom.xml (working copy) @@ -61,7 +61,7 @@ dependency groupIdorg.hibernate/groupId artifactIdhibernate-validator/artifactId - version4.1.0.GA/version + version4.1.0.Final/version exclusions exclusion groupIdjavax.xml.bind/groupId Index: samples/mobilewebapp/pom.xml === --- samples/mobilewebapp/pom.xml(revision 11305) +++ samples/mobilewebapp/pom.xml(working copy) @@ -68,7 +68,7 @@ dependency groupIdorg.hibernate/groupId artifactIdhibernate-validator/artifactId - version4.1.0.GA/version + version4.1.0.Final/version exclusions exclusion groupIdjavax.xml.bind/groupId Index: samples/validation/pom.xml === --- samples/validation/pom.xml (revision 11305) +++ samples/validation/pom.xml (working copy) @@ -44,7 +44,7 @@ dependency groupIdorg.hibernate/groupId artifactIdhibernate-validator/artifactId - version4.1.0.GA/version + version4.1.0.Final/version exclusions exclusion groupIdjavax.xml.bind/groupId @@ -62,7 +62,7 @@ dependency groupIdorg.hibernate/groupId artifactIdhibernate-validator/artifactId - version4.1.0.GA/version + version4.1.0.Final/version classifiersources/classifier exclusions exclusion -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Delete SimpleXML and SimpleRPC samples (issue1847804)
Reviewers: rdayal, Description: Delete SimpleXML and SimpleRPC samples Please review this at http://gwt-code-reviews.appspot.com/1847804/ Affected files: M samples/build.xml D samples/simplerpc/build.xml D samples/simplerpc/src/com/google/gwt/sample/simplerpc/COPYING D samples/simplerpc/src/com/google/gwt/sample/simplerpc/SimpleRPC.gwt.xml D samples/simplerpc/src/com/google/gwt/sample/simplerpc/client/SimpleRPC.java D samples/simplerpc/src/com/google/gwt/sample/simplerpc/client/SimpleRPCException.java D samples/simplerpc/src/com/google/gwt/sample/simplerpc/client/SimpleRPCService.java D samples/simplerpc/src/com/google/gwt/sample/simplerpc/client/SimpleRPCServiceAsync.java D samples/simplerpc/src/com/google/gwt/sample/simplerpc/server/SimpleRPCServiceImpl.java D samples/simplerpc/war/SimpleRPC.css D samples/simplerpc/war/SimpleRPC.html D samples/simplerpc/war/WEB-INF/classes/marker D samples/simplerpc/war/WEB-INF/web.xml D samples/simplexml/build.xml D samples/simplexml/src/com/google/gwt/sample/simplexml/COPYING D samples/simplexml/src/com/google/gwt/sample/simplexml/SimpleXML.gwt.xml D samples/simplexml/src/com/google/gwt/sample/simplexml/client/SimpleXML.java D samples/simplexml/src/com/google/gwt/sample/simplexml/public/customerRecord.xml D samples/simplexml/war/SimpleXML.css D samples/simplexml/war/SimpleXML.html D samples/simplexml/war/WEB-INF/classes/marker D samples/simplexml/war/WEB-INF/web.xml -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Update the hibernate version from 4.0.2 to 4.1.0 in the samples (issue1841803)
Reviewers: Nick Chalko, Description: Update the hibernate version from 4.0.2 to 4.1.0 in the samples Review by: ncha...@google.com Please review this at http://gwt-code-reviews.appspot.com/1841803/ Affected files: M samples/dynatablerf/pom.xml M samples/mobilewebapp/pom.xml M samples/validation/pom.xml Index: samples/dynatablerf/pom.xml === --- samples/dynatablerf/pom.xml (revision 11305) +++ samples/dynatablerf/pom.xml (working copy) @@ -61,7 +61,7 @@ dependency groupIdorg.hibernate/groupId artifactIdhibernate-validator/artifactId - version4.0.2.GA/version + version4.1.0.GA/version exclusions exclusion groupIdjavax.xml.bind/groupId Index: samples/mobilewebapp/pom.xml === --- samples/mobilewebapp/pom.xml(revision 11305) +++ samples/mobilewebapp/pom.xml(working copy) @@ -68,7 +68,7 @@ dependency groupIdorg.hibernate/groupId artifactIdhibernate-validator/artifactId - version4.0.2.GA/version + version4.1.0.GA/version exclusions exclusion groupIdjavax.xml.bind/groupId Index: samples/validation/pom.xml === --- samples/validation/pom.xml (revision 11305) +++ samples/validation/pom.xml (working copy) @@ -44,7 +44,7 @@ dependency groupIdorg.hibernate/groupId artifactIdhibernate-validator/artifactId - version4.0.2.GA/version + version4.1.0.GA/version exclusions exclusion groupIdjavax.xml.bind/groupId @@ -62,7 +62,7 @@ dependency groupIdorg.hibernate/groupId artifactIdhibernate-validator/artifactId - version4.0.2.GA/version + version4.1.0.GA/version classifiersources/classifier exclusions exclusion -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Delete the Expenses sample (issue1843803)
Reviewers: rdayal, Description: Delete the Expenses sample Please review this at http://gwt-code-reviews.appspot.com/1843803/ Affected files: M samples/build.xml D samples/expenses/README-MAVEN.txt D samples/expenses/build.xml D samples/expenses/pom.xml D samples/expenses/src/log4j.properties D samples/expenses/src/main/java/META-INF/jdoconfig.xml D samples/expenses/src/main/java/META-INF/persistence.xml D samples/expenses/src/main/java/com/google/gwt/mobile/Mobile.gwt.xml D samples/expenses/src/main/java/com/google/gwt/mobile/client/MobileScrollPanel.java D samples/expenses/src/main/java/com/google/gwt/mobile/client/Momentum.java D samples/expenses/src/main/java/com/google/gwt/mobile/client/Point.java D samples/expenses/src/main/java/com/google/gwt/mobile/client/Scroller.java D samples/expenses/src/main/java/com/google/gwt/mobile/client/Touch.java D samples/expenses/src/main/java/com/google/gwt/mobile/client/TouchEvent.java D samples/expenses/src/main/java/com/google/gwt/mobile/client/TouchHandler.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/Expenses.gwt.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/ExpensesCommon.gwt.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/ExpensesMobile.gwt.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/LoadExpensesDB.gwt.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/Approval.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/DataGenerationService.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/DataGenerationServiceAsync.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/DenialPopup.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseDetailsCellTable.css D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseListCellTable.css D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseReportDetails.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseReportDetails.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseReportList.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseReportList.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpenseTree.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/Expenses.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesActivityMapper.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesApp.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesMobile.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesMobileShell.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesMobileShell.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesShell.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ExpensesShell.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/GetValue.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/LoadExpensesDB.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseDetails.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseDetails.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseEntry.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseEntry.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileExpenseList.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobilePage.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileReportEntry.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileReportEntry.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/MobileReportList.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/PhaseAnimation.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ScaffoldMobileShell.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/ScaffoldShell.ui.xml D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/SlidingPanel.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/SortableHeader.java D samples/expenses/src/main/java/com/google/gwt/sample/expenses/client/SpacerColumn.java D
[gwt-contrib] Re: CellTree disappeared when clicking in the widget but outside tree nodes. (issue1827803)
Can you also add skybrian to the CL? I'll do the review, but I suspect
he'll go ahead and patch/submit your branches in one swoop.
https://gwt-code-reviews.appspot.com/1827803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java
File user/src/com/google/gwt/user/cellview/client/CellTree.java (right):
https://gwt-code-reviews.appspot.com/1827803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java#newcode743
user/src/com/google/gwt/user/cellview/client/CellTree.java:743: if
(!nodeView.isRootNode()
nodeView.getImageElement().isOrHasChild(target)) {
Does the root node never need to be opened? Or does setNode() only
add that CSS that we're trying to get rid of?
https://gwt-code-reviews.appspot.com/1827803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: CellTree disappeared when clicking in the widget but outside tree nodes. (issue1827803)
On 2012/09/10 23:15:08, tbroyer wrote:
Brian: Unnur thinks you'd review that patch faster than her ;-)
https://gwt-code-reviews.appspot.com/1827803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java
File user/src/com/google/gwt/user/cellview/client/CellTree.java
(right):
https://gwt-code-reviews.appspot.com/1827803/diff/1/user/src/com/google/gwt/user/cellview/client/CellTree.java#newcode743
user/src/com/google/gwt/user/cellview/client/CellTree.java:743: if
(!nodeView.isRootNode()
nodeView.getImageElement().isOrHasChild(target)) {
On 2012/09/10 23:02:41, unnurg wrote:
Does the root node never need to be opened? Or does setNode()
only add that
CSS that we're trying to get rid of?
Actually, the CSS is not the issue, it's only what triggers it: the
actual issue
is that the root node is closed when receiving a mousedown event; the
margin:20px makes it so that there are 20px high regions between
top-level nodes
where clicks target the root node.
And the real issue is that getImageElement accesses some specific
element in the
DOM, assuming the node's element contains a first child for the node's
value and
disclosure image (the one that getImageElement returns) and a second
one with
the children. Except that for the root node, there's no visible value
(it's a
virtual node) so the second child is actually the first, and
getImageElement
returns a DIV that contains all children (so when clicking in between
child
nodes, it targets getImageElement, producing a false positive here).
Yes, it's a bit of a complex thing ;-)
The root node is opened in the constructor and should never ever be
closed
(remember: it's invisible, it's only there to contain the top-level
nodes).
LGTM
Great - thanks for the explanation (and the patch). I'll add this to
the list of your patches that are going in (Brian will probably do the
actual patching and submitting unless he hasn't gotten to it by Friday,
in which case I'll probably go through and do them)
https://gwt-code-reviews.appspot.com/1827803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding an option to change the debug id property and prefix. Some users prefer to use a random a... (issue1618804)
On 2011/12/16 17:38:57, jlabanca wrote: LGTM http://gwt-code-reviews.appspot.com/1618804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Avoid bottlenecks by using ConcurrentHashMap instead of a synchronized IdentityHashMap. (issue1609803)
On 2011/12/08 18:50:13, rdayal wrote: LGTM http://gwt-code-reviews.appspot.com/1609803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix GWT emulated Logger behavior to match java.util.logging.Logger (issue1602804)
On 2011/11/28 23:05:47, baker wrote: LGTM http://gwt-code-reviews.appspot.com/1602804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Allow @defs with multiple values (issue1556804)
Reviewers: rjrjr,
Description:
Allow @defs with multiple values
Please review this at http://gwt-code-reviews.appspot.com/1556804/
Affected files:
M user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
M user/test/com/google/gwt/resources/client/CSSResourceTest.java
M user/test/com/google/gwt/resources/client/deftest.css
Index: user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
===
--- user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
(revision 10663)
+++ user/src/com/google/gwt/resources/rg/CssResourceGenerator.java (working
copy)
@@ -70,6 +70,7 @@
import com.google.gwt.resources.ext.ResourceContext;
import com.google.gwt.resources.ext.ResourceGeneratorUtil;
import com.google.gwt.resources.ext.SupportsGeneratorResultCaching;
+import com.google.gwt.thirdparty.guava.common.base.Joiner;
import com.google.gwt.user.rebind.SourceWriter;
import com.google.gwt.user.rebind.StringSourceWriter;
@@ -896,6 +897,11 @@
operableTypes);
}
+ private boolean isReturnTypeString(JClassType classReturnType) {
+return (classReturnType != null
+
String.class.getName().equals(classReturnType.getQualifiedSourceName()));
+ }
+
/**
* Check for the presence of the NotStrict annotation on the method.
This will
* also perform some limited sanity-checking for the now-deprecated
Strict
@@ -1079,21 +1085,21 @@
throw new UnableToCompleteException();
}
-// TODO: Allow returning an array of values
-if (def.getValues().size() != 1) {
+JClassType classReturnType = toImplement.getReturnType().isClass();
+
+if (def.getValues().size() != 1
!isReturnTypeString(classReturnType)) {
logger.log(TreeLogger.ERROR, @def rule + name
- + must define exactly one value);
+ + must define exactly one value or return type must be
String);
throw new UnableToCompleteException();
}
-NumberValue numberValue = def.getValues().get(0).isNumberValue();
-
String returnExpr = ;
-JClassType classReturnType = toImplement.getReturnType().isClass();
-if (classReturnType != null
-
java.lang.String.equals(classReturnType.getQualifiedSourceName())) {
- returnExpr = \ +
Generator.escape(def.getValues().get(0).toString())
- + \;
+if (isReturnTypeString(classReturnType)) {
+ ListString returnValues = new ArrayListString();
+ for (Value val : def.getValues()) {
+returnValues.add(Generator.escape(val.toString()));
+ }
+ returnExpr = \ + Joiner.on( ).join(returnValues) + \;
} else {
JPrimitiveType returnType =
toImplement.getReturnType().isPrimitive();
if (returnType == null) {
@@ -1102,6 +1108,7 @@
+ @def accessors);
throw new UnableToCompleteException();
}
+ NumberValue numberValue = def.getValues().get(0).isNumberValue();
if (returnType == JPrimitiveType.INT || returnType ==
JPrimitiveType.LONG) {
returnExpr = + Math.round(numberValue.getValue());
} else if (returnType == JPrimitiveType.FLOAT) {
Index: user/test/com/google/gwt/resources/client/CSSResourceTest.java
===
--- user/test/com/google/gwt/resources/client/CSSResourceTest.java
(revision 10663)
+++ user/test/com/google/gwt/resources/client/CSSResourceTest.java (working
copy)
@@ -60,6 +60,8 @@
float rawFloat();
int rawInt();
+
+String multiValueBorderDef();
}
/**
@@ -362,6 +364,8 @@
assertNotNull(defines.overrideIntClass());
assertFalse(10px.equals(defines.overrideIntClass()));
assertFalse(10.equals(defines.overrideIntClass()));
+
+assertEquals(1px solid rgba(0,0,0,0.2),
defines.multiValueBorderDef());
}
public void testEnsureInjected() {
Index: user/test/com/google/gwt/resources/client/deftest.css
===
--- user/test/com/google/gwt/resources/client/deftest.css (revision 10663)
+++ user/test/com/google/gwt/resources/client/deftest.css (working copy)
@@ -27,6 +27,8 @@
@def lengthString 100px;
@def colorString #f00;
+ @def multiValueBorderDef 1px solid rgba(0,0,0,0.2);
+
/* Uncomment this, and you should get an error about a @def shadowing a
name */
/*
.colorString {
@@ -41,5 +43,3 @@
.overrideInt {
width: 10px;
}
-
-
\ No newline at end of file
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Allow @defs with multiple values (issue1557803)
Reviewers: rjrjr,
Description:
Allow @defs with multiple values
Please review this at http://gwt-code-reviews.appspot.com/1557803/
Affected files:
M user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
M user/test/com/google/gwt/resources/client/CSSResourceTest.java
M user/test/com/google/gwt/resources/client/deftest.css
Index: user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
===
--- user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
(revision 10663)
+++ user/src/com/google/gwt/resources/rg/CssResourceGenerator.java (working
copy)
@@ -70,6 +70,7 @@
import com.google.gwt.resources.ext.ResourceContext;
import com.google.gwt.resources.ext.ResourceGeneratorUtil;
import com.google.gwt.resources.ext.SupportsGeneratorResultCaching;
+import com.google.gwt.thirdparty.guava.common.base.Joiner;
import com.google.gwt.user.rebind.SourceWriter;
import com.google.gwt.user.rebind.StringSourceWriter;
@@ -896,6 +897,11 @@
operableTypes);
}
+ private boolean isReturnTypeString(JClassType classReturnType) {
+return (classReturnType != null
+
String.class.getName().equals(classReturnType.getQualifiedSourceName()));
+ }
+
/**
* Check for the presence of the NotStrict annotation on the method.
This will
* also perform some limited sanity-checking for the now-deprecated
Strict
@@ -1079,21 +1085,21 @@
throw new UnableToCompleteException();
}
-// TODO: Allow returning an array of values
-if (def.getValues().size() != 1) {
+JClassType classReturnType = toImplement.getReturnType().isClass();
+
+if (def.getValues().size() != 1
!isReturnTypeString(classReturnType)) {
logger.log(TreeLogger.ERROR, @def rule + name
- + must define exactly one value);
+ + must define exactly one value or return type must be
String);
throw new UnableToCompleteException();
}
-NumberValue numberValue = def.getValues().get(0).isNumberValue();
-
String returnExpr = ;
-JClassType classReturnType = toImplement.getReturnType().isClass();
-if (classReturnType != null
-
java.lang.String.equals(classReturnType.getQualifiedSourceName())) {
- returnExpr = \ +
Generator.escape(def.getValues().get(0).toString())
- + \;
+if (isReturnTypeString(classReturnType)) {
+ ListString returnValues = new ArrayListString();
+ for (Value val : def.getValues()) {
+returnValues.add(Generator.escape(val.toString()));
+ }
+ returnExpr = \ + Joiner.on( ).join(returnValues) + \;
} else {
JPrimitiveType returnType =
toImplement.getReturnType().isPrimitive();
if (returnType == null) {
@@ -1102,6 +1108,7 @@
+ @def accessors);
throw new UnableToCompleteException();
}
+ NumberValue numberValue = def.getValues().get(0).isNumberValue();
if (returnType == JPrimitiveType.INT || returnType ==
JPrimitiveType.LONG) {
returnExpr = + Math.round(numberValue.getValue());
} else if (returnType == JPrimitiveType.FLOAT) {
Index: user/test/com/google/gwt/resources/client/CSSResourceTest.java
===
--- user/test/com/google/gwt/resources/client/CSSResourceTest.java
(revision 10663)
+++ user/test/com/google/gwt/resources/client/CSSResourceTest.java (working
copy)
@@ -60,6 +60,8 @@
float rawFloat();
int rawInt();
+
+String multiValueBorderDef();
}
/**
@@ -362,6 +364,8 @@
assertNotNull(defines.overrideIntClass());
assertFalse(10px.equals(defines.overrideIntClass()));
assertFalse(10.equals(defines.overrideIntClass()));
+
+assertEquals(1px solid rgba(0,0,0,0.2),
defines.multiValueBorderDef());
}
public void testEnsureInjected() {
Index: user/test/com/google/gwt/resources/client/deftest.css
===
--- user/test/com/google/gwt/resources/client/deftest.css (revision 10663)
+++ user/test/com/google/gwt/resources/client/deftest.css (working copy)
@@ -27,6 +27,8 @@
@def lengthString 100px;
@def colorString #f00;
+ @def multiValueBorderDef 1px solid rgba(0,0,0,0.2);
+
/* Uncomment this, and you should get an error about a @def shadowing a
name */
/*
.colorString {
@@ -41,5 +43,3 @@
.overrideInt {
width: 10px;
}
-
-
\ No newline at end of file
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix typo (issue1497807)
On 2011/08/04 16:22:03, fredsa wrote: LGTM http://gwt-code-reviews.appspot.com/1497807/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add font-face support to CssResource. (issue1502806)
LGTM On 2011/07/26 19:40:41, unnurg wrote: sure On Tue, Jul 26, 2011 at 12:39 PM, Ray Ryan mailto:rj...@google.com wrote: Unnur, would you be comfortable taking this? On Tue, Jul 26, 2011 at 12:12 PM, mailto:b...@google.com wrote: Reviewers: rjrjr, Message: This is a re-spin of a patch that was written back in October but that got dropped. The original patch is at http://gwt-code-reviews.**appspot.com/943802%3Chttp://gwt-code-reviews.appspot.com/943802 . Description: Add font-face support to CssResource. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.**appspot.com/1502806/%3Chttp://gwt-code-reviews.appspot.com/1502806/ Affected files: M user/src/com/google/gwt/**resources/css/**CssGenerationVisitor.java M user/src/com/google/gwt/**resources/css/GenerateCssAst.**java A user/src/com/google/gwt/**resources/css/ast/CssFontFace.**java M user/src/com/google/gwt/**resources/css/ast/**CssNodeCloner.java M user/src/com/google/gwt/**resources/css/ast/CssVisitor.**java M user/test/com/google/gwt/**resources/client/**CSSResourceTest.java M user/test/com/google/gwt/**resources/client/test.css -- DO NOT FORWARD http://gwt-code-reviews.appspot.com/1502806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] sanitize the bad codeserver name before outputting the error message for security (issue1483804)
Reviewers: cromwellian,
Description:
sanitize the bad codeserver name before outputting the error message for
security
Please review this at http://gwt-code-reviews.appspot.com/1483804/
Affected files:
M dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js
M dev/core/src/com/google/gwt/core/ext/linker/impl/hosted.html
Index: dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js
===
--- dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js (revision
10456)
+++ dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js (working
copy)
@@ -314,10 +314,18 @@
if ($errFn) {
$errFn($moduleName);
} else {
-__gwt_displayGlassMessage(Plugin failed to connect to Development
Mode server at + codeServer,
+__gwt_displayGlassMessage(Plugin failed to connect to Development
Mode server at +
+simpleEscape(codeServer),
Follow the underlying troubleshooting instructions);
loadIframe(http://code.google.com/p/google-web-toolkit/wiki/TroubleshootingOOPHM;);
}
+}
+
+function simpleEscape(originalString) {
+ return originalString.replace(,amp;)
+.replace(,lt;)
+.replace(,gt;)
+.replace(\,quot;);
}
function tryConnectingToPlugin(sessionId, url) {
Index: dev/core/src/com/google/gwt/core/ext/linker/impl/hosted.html
===
--- dev/core/src/com/google/gwt/core/ext/linker/impl/hosted.html (revision
10456)
+++ dev/core/src/com/google/gwt/core/ext/linker/impl/hosted.html (working
copy)
@@ -295,12 +295,20 @@
if (errFn) {
errFn(modName);
} else {
-__gwt_displayGlassMessage(Plugin failed to connect to Development
Mode server at + $hosted,
+__gwt_displayGlassMessage(Plugin failed to connect to Development
Mode server at +
+simpleEscape($hosted),
Follow the underlying troubleshooting instructions);
loadIframe(http://code.google.com/p/google-web-toolkit/wiki/TroubleshootingOOPHM;);
}
}
}
+}
+
+function simpleEscape(originalString) {
+ return originalString.replace(,amp;)
+.replace(,lt;)
+.replace(,gt;)
+.replace(\,quot;);
}
window.onunload = function() {
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add chunking to the xsiframe linker (issue1477802)
Reviewers: zundel, Description: Add chunking to the xsiframe linker Please review this at http://gwt-code-reviews.appspot.com/1477802/ Affected files: M dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptAlreadyIncluded.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java M dev/core/src/com/google/gwt/core/linker/IFrameLinker.java M dev/core/test/com/google/gwt/core/linker/ScriptChunkingTest.java M user/test/com/google/gwt/core/ext/LinkerTest.gwt.xml M user/test/com/google/gwt/core/ext/test/LinkerTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: A general purpose script injection class for injecting a script directly (issue1451818)
On 2011/06/27 19:25:36, zundel wrote: Updated documentation. http://gwt-code-reviews.appspot.com/1451818/diff/9016/user/test/com/google/gwt/core/CoreSuite.java File user/test/com/google/gwt/core/CoreSuite.java (right): http://gwt-code-reviews.appspot.com/1451818/diff/9016/user/test/com/google/gwt/core/CoreSuite.java#newcode28 user/test/com/google/gwt/core/CoreSuite.java:28: import com.google.gwt.core.client.impl.XhrLoadingStrategyTest; ignore, from sync LGTM Actually, this class has the added benefit of being a pretty good set of documentation for exactly what breaks in which browser for script injection :-) http://gwt-code-reviews.appspot.com/1451818/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Refactor LoadingStrategy for xsiframe linker and rename it to ScriptLoadingStrategy since it is ... (issue1468802)
Reviewers: zundel, Description: Refactor LoadingStrategy for xsiframe linker and rename it to ScriptLoadingStrategy since it is not inherently tied to the xsiframe linker. Bring it more in line with the XhrLoadingStrategy and give them API compatibility so the xsiframe linker could use either one. Add features, such as retrying the download if it fails (like the XhrLoadingStrategy has) Lay the groundwork for adding prefetching, although this will need some changes higher up in the runAsynch framework to finish Review by: zun...@google.com Please review this at http://gwt-code-reviews.appspot.com/1468802/ Affected files: M dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptAlreadyIncluded.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptDirect.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js A dev/core/src/com/google/gwt/core/ext/linker/impl/runAsync.js M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js M dev/core/src/com/google/gwt/core/linker/IFrameLinker.java M tools/api-checker/config/gwt23_24userApi.conf M user/src/com/google/gwt/core/CrossSiteIframeLinker.gwt.xml M user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java D user/src/com/google/gwt/core/client/impl/CrossSiteIframeLoadingStrategy.java A user/src/com/google/gwt/core/client/impl/LoadingStrategyBase.java A user/src/com/google/gwt/core/client/impl/ScriptTagLoadingStrategy.java M user/src/com/google/gwt/core/client/impl/XhrLoadingStrategy.java M user/test/com/google/gwt/core/CoreSuite.java D user/test/com/google/gwt/core/client/impl/XhrLoadingStrategyTest.java M user/test/com/google/gwt/dev/jjs/CrossSiteIframeRunAsyncFailure.gwt.xml D user/test/com/google/gwt/dev/jjs/CrossSiteIframeRunAsyncMetrics.gwt.xml M user/test/com/google/gwt/dev/jjs/CrossSiteIframeRunAsyncSuite.java M user/test/com/google/gwt/dev/jjs/test/CrossSiteIframeRunAsyncFailureTest.java D user/test/com/google/gwt/dev/jjs/test/CrossSiteIframeRunAsyncMetricsTest.java M user/test/com/google/gwt/dev/jjs/test/CrossSiteRunAsyncFailureTest.java M user/test/com/google/gwt/dev/jjs/test/RunAsyncFailureTest.java M user/test/com/google/gwt/user/server/runasync/RunAsyncFailureServlet.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes a problem where inheriting LoggingDisabled (like RequestFactory does) clobbers the value o... (issue1451816)
On 2011/06/10 21:08:17, rjrjr wrote: LGTM http://gwt-code-reviews.appspot.com/1451816/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: DirectInstallLinker should not immediately remove the script tag it has inserted into the IFRAME... (issue1454802)
On 2011/06/07 22:05:56, fredsa wrote: Tell me more... this is not new - the xsiframe linker does this and we haven't heard of any problems there... http://gwt-code-reviews.appspot.com/1454802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: DirectInstallLinker should not immediately remove the script tag it has inserted into the IFRAME... (issue1454802)
LGTM ok - I'm dumb - the file that xsiframe linker is setting the contents of the script tag and then removing it. This code sets the src of the script tag. Probably not a good idea to remove it before the contents come back :-) Thanks!! - Unnur On 2011/06/07 22:38:26, unnurg wrote: On 2011/06/07 22:05:56, fredsa wrote: Tell me more... this is not new - the xsiframe linker does this and we haven't heard of any problems there... http://gwt-code-reviews.appspot.com/1454802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Creates a delegate for CompilationResult for use by third party linkers. (issue1451808)
LGTM On 2011/06/02 12:37:54, bobv wrote: LGTM http://gwt-code-reviews.appspot.com/1451808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding additional testing for GWT RPC. Some custom serialized objects (issue1441804)
LGTM On 2011/05/12 23:04:46, Stephen Chenney wrote: http://gwt-code-reviews.appspot.com/1441804/diff/1/user/test/com/google/gwt/user/client/rpc/LoggingRPCTest.java File user/test/com/google/gwt/user/client/rpc/LoggingRPCTest.java (right): http://gwt-code-reviews.appspot.com/1441804/diff/1/user/test/com/google/gwt/user/client/rpc/LoggingRPCTest.java#newcode84 user/test/com/google/gwt/user/client/rpc/LoggingRPCTest.java:84: */ On 2011/05/12 22:07:04, tobyr wrote: Stale comment? Done. http://gwt-code-reviews.appspot.com/1441804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add runtime-locale support for Localizable subtypes. (issue1425816)
Sorry for the delay - LGTM On 2011/05/04 21:24:23, jat wrote: Ping http://gwt-code-reviews.appspot.com/1425816/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Update the flute jar to accept double colon notation in css selectors, rename (issue1431801)
Reviewers: cromwellian,
Description:
Update the flute jar to accept double colon notation in css selectors,
rename
it to version 2, and use it throughout gwt. Also update the AST
generation code
to output double colons for any pseudo class/element that was is not
supported
with the single colon notation.
Please review this at http://gwt-code-reviews.appspot.com/1431801/
Affected files:
M common.ant.xml
M dev/build.xml
M eclipse/servlet/.classpath
M eclipse/user/.classpath
M user/build.xml
M user/src/com/google/gwt/resources/css/GenerateCssAst.java
Index: common.ant.xml
===
--- common.ant.xml (revision 10132)
+++ common.ant.xml (working copy)
@@ -236,7 +236,7 @@
pathelement location=${gwt.tools.lib}/junit/junit-4.8.2.jar /
pathelement
location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
pathelement location=${gwt.tools.lib}/w3c/sac/sac-1.3.jar /
- pathelement
location=${gwt.tools.lib}/w3c/flute/flute-1.3-gg1.jar /
+ pathelement
location=${gwt.tools.lib}/w3c/flute/flute-1.3-gg2.jar /
extraclasspaths /
/classpath
Index: dev/build.xml
===
--- dev/build.xml (revision 10132)
+++ dev/build.xml (working copy)
@@ -39,7 +39,7 @@
pathelement
location=${gwt.tools.lib}/jfreechart/jfreechart-1.0.3.jar /
pathelement
location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
pathelement location=${gwt.tools.lib}/w3c/sac/sac-1.3.jar /
-pathelement
location=${gwt.tools.lib}/w3c/flute/flute-1.3-gg1.jar /
+pathelement
location=${gwt.tools.lib}/w3c/flute/flute-1.3-gg2.jar /
pathelement
location=${gwt.tools}/redist/json/r2_20080312/json-1.5.jar /
pathelement
location=${gwt.tools.lib}/hibernate/validator/hibernate-validator-4.1.0.Final.jar
/
pathelement
location=${gwt.tools.lib}/javax/validation/validation-api-1.0.0.GA.jar /
Index: eclipse/servlet/.classpath
===
--- eclipse/servlet/.classpath (revision 10132)
+++ eclipse/servlet/.classpath (working copy)
@@ -6,7 +6,7 @@
classpathentry kind=var
path=GWT_TOOLS/lib/tomcat/servlet-api-2.4.jar/
classpathentry combineaccessrules=false kind=src path=/gwt-user/
classpathentry combineaccessrules=false kind=src path=/gwt-dev/
- classpathentry kind=var
path=GWT_TOOLS/lib/w3c/flute/flute-1.3-gg1.jar/
+ classpathentry kind=var
path=GWT_TOOLS/lib/w3c/flute/flute-1.3-gg2.jar/
classpathentry kind=var path=GWT_TOOLS/lib/w3c/sac/sac-1.3.jar/
classpathentry kind=output path=bin/
/classpath
Index: eclipse/user/.classpath
===
--- eclipse/user/.classpath (revision 10132)
+++ eclipse/user/.classpath (working copy)
@@ -27,7 +27,7 @@
classpathentry kind=var
path=GWT_TOOLS/lib/xerces/xerces-2_9_1/xercesImpl-NoMetaInf.jar/
classpathentry kind=var
path=GWT_TOOLS/lib/xerces/xerces-2_9_1/xml-apis.jar/
classpathentry kind=var path=GWT_TOOLS/lib/w3c/sac/sac-1.3.jar/
- classpathentry kind=var
path=GWT_TOOLS/lib/w3c/flute/flute-1.3-gg1.jar/
+ classpathentry kind=var
path=GWT_TOOLS/lib/w3c/flute/flute-1.3-gg2.jar/
classpathentry kind=var path=GWT_TOOLS/lib/cglib/cglib-2.2.jar/
classpathentry kind=var
path=GWT_TOOLS/lib/objenesis/objenesis-1.2.jar/
classpathentry kind=var
path=GWT_TOOLS/lib/easymock/easymock-3.0.jar/
Index: user/build.xml
===
--- user/build.xml (revision 10132)
+++ user/build.xml (working copy)
@@ -98,7 +98,7 @@
pathelement
location=${gwt.tools.lib}/jfreechart/jfreechart-1.0.3.jar /
pathelement
location=${gwt.tools.lib}/selenium/selenium-java-client-driver.jar /
pathelement location=${gwt.tools.lib}/w3c/sac/sac-1.3.jar /
-pathelement
location=${gwt.tools.lib}/w3c/flute/flute-1.3-gg1.jar /
+pathelement
location=${gwt.tools.lib}/w3c/flute/flute-1.3-gg2.jar /
pathelement
location=${gwt.tools}/redist/json/r2_20080312/json-1.5.jar /
pathelement
location=${gwt.tools.lib}/javax/validation/validation-api-1.0.0.GA.jar /
!-- The source is included so validation is available from client
code --
@@ -150,7 +150,7 @@
fileset dir=${javac.out} /
zipfileset src=${gwt.tools.lib}/tomcat/servlet-api-2.5.jar
excludes=**/*.java/
zipfileset src=${gwt.tools.lib}/w3c/sac/sac-1.3.jar /
- zipfileset src=${gwt.tools.lib}/w3c/flute/flute-1.3-gg1.jar /
+ zipfileset src=${gwt.tools.lib}/w3c/flute/flute-1.3-gg2.jar /
/gwt.jar
/target
Index: user/src/com/google/gwt/resources/css/GenerateCssAst.java
[gwt-contrib] Re: Update the flute jar to accept double colon notation in css selectors, rename (issue1431801)
http://gwt-code-reviews.appspot.com/1431801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Update the flute jar to accept double colon notation in css selectors, rename (issue1431801)
http://gwt-code-reviews.appspot.com/1431801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding additional testing for GWT RPC. Some custom serialized objects (issue1424801)
LGTM On 2011/04/28 15:12:50, Stephen Chenney wrote: I have removed the out-of-date comment and ran a smoke test with a fresh pull of GWT. Everything is checking out. http://gwt-code-reviews.appspot.com/1424801/diff/1/user/test/com/google/gwt/user/client/rpc/CoreJavaTest.java File user/test/com/google/gwt/user/client/rpc/CoreJavaTest.java (right): http://gwt-code-reviews.appspot.com/1424801/diff/1/user/test/com/google/gwt/user/client/rpc/CoreJavaTest.java#newcode126 user/test/com/google/gwt/user/client/rpc/CoreJavaTest.java:126: * This test is disabled because it times out, apparently due to an infinite On 2011/04/28 00:18:48, unnurg wrote: Where is the code to disable this? I forgot to update the comment after you helped me fix it. About to be done. http://gwt-code-reviews.appspot.com/1424801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds ability to include SafeHtml objects in dom based UI's if the lazy (issue1421811)
LGTM On 2011/05/02 22:19:32, rjrjr wrote: Hey there Rodrigo. Unnur is on point for this review, but you might find it interesting. http://gwt-code-reviews.appspot.com/1421811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Improve runtime locales support, so runtime locales that are under a (issue1421812)
LGTM On 2011/05/02 22:31:23, jat wrote: http://gwt-code-reviews.appspot.com/1421812/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
http://gwt-code-reviews.appspot.com/1425811/diff/5001/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java File user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java (right): http://gwt-code-reviews.appspot.com/1425811/diff/5001/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java#newcode52 user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java:52: assertEquals(Hello bBob/b, domUi.div.getInnerHTML()); On 2011/04/27 23:21:30, rjrjr wrote: You might want call .toLowerCase() on both strings to normalize. IE does funny things to tag names, you'll get dinged in the cross browser tests. Done. http://gwt-code-reviews.appspot.com/1425811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Check for IsWidget rather than Widget when determining if a class is a Widget in UiBinder (issue1421809)
Reviewers: rjrjr,
Description:
Check for IsWidget rather than Widget when determining if a class is a
Widget in UiBinder
Please review this at http://gwt-code-reviews.appspot.com/1421809/
Affected files:
M user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
Index: user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
===
--- user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java (revision
10100)
+++ user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java (working
copy)
@@ -39,7 +39,7 @@
import com.google.gwt.uibinder.rebind.model.OwnerClass;
import com.google.gwt.uibinder.rebind.model.OwnerField;
import com.google.gwt.user.client.ui.Attachable;
-import com.google.gwt.user.client.ui.Widget;
+import com.google.gwt.user.client.ui.IsWidget;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
@@ -687,7 +687,7 @@
public boolean isWidgetElement(XMLElement elem)
throws UnableToCompleteException {
-return isSubclassOf(elem, Widget.class);
+return isSubclassOf(elem, IsWidget.class);
}
/**
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the lazy (issue1420814)
LGTM http://gwt-code-reviews.appspot.com/1420814/diff/1/user/test/com/google/gwt/uibinder/test/client/UiChildTest.java File user/test/com/google/gwt/uibinder/test/client/UiChildTest.java (right): http://gwt-code-reviews.appspot.com/1420814/diff/1/user/test/com/google/gwt/uibinder/test/client/UiChildTest.java#newcode100 user/test/com/google/gwt/uibinder/test/client/UiChildTest.java:100: // @UiFactory remove this commented out code? http://gwt-code-reviews.appspot.com/1420814/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Additional infrastructure for generating source code outside of a (issue1421805)
LGTM once you upload the most recent changes I saw http://gwt-code-reviews.appspot.com/1421805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Additional infrastructure for generating source code outside of a (issue1421805)
LGTM ++ On 2011/04/28 23:35:48, jat wrote: http://gwt-code-reviews.appspot.com/1421805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
Reviewers: rjrjr, Description: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being used (this is the only way that the setters will work correctly) Please review this at http://gwt-code-reviews.appspot.com/1425811/ Affected files: M user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java M user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java M user/test/com/google/gwt/uibinder/UiBinderGwtSuite.java M user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java A user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml A user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.DomBasedUi.ui.xml A user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java A user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
http://gwt-code-reviews.appspot.com/1425811/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java (right): http://gwt-code-reviews.appspot.com/1425811/diff/1/user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java#newcode91 user/src/com/google/gwt/uibinder/elementparsers/HtmlInterpreter.java:91: return writer.tokenForSafeHtmlExpression( On 2011/04/27 19:29:58, rjrjr wrote: I think instead you want to use the tokenForExpression method that Rafa just added. Done. http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java File user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java (right): http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java#newcode98 user/test/com/google/gwt/uibinder/elementparsers/AbsolutePanelParserTest.java:98: b.append( g:Label/); On 2011/04/27 19:29:58, rjrjr wrote: This is a bit disturbing. Why did it fix it? Or is the real question why did it pass in the first place? Sorry - I thought you saw my comment on the other review thread - I have no idea why this didn't work for TextBox and was hoping that perhaps you would have some inkling? http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml File user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml (right): http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml#newcode18 user/test/com/google/gwt/uibinder/test/LazyWidgetBuilderTest.gwt.xml:18: entry-point class=com.google.gwt.uibinder.test.client.LazyWidgetBuilderTest / On 2011/04/27 19:29:58, rjrjr wrote: I don't think you need the entry-point Done. http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java File user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java (right): http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java#newcode52 user/test/com/google/gwt/uibinder/test/client/LazyWidgetBuildersTest.java:52: assertEquals(Hello Bob, domUi.div.getInnerHTML()); On 2011/04/27 19:29:58, rjrjr wrote: check for the b, make sure it didn't get escaped. Done. http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java File user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java (right): http://gwt-code-reviews.appspot.com/1425811/diff/1/user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java#newcode28 user/test/com/google/gwt/uibinder/test/client/SafeHtmlObject.java:28: return Hello + name; On 2011/04/27 19:29:58, rjrjr wrote: Should put some markup in here, so we can test for improper escaping. Hello b + name + /b. Done. http://gwt-code-reviews.appspot.com/1425811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ability to include SafeHtml objects in dom based UI's if the laay widget option is being use... (issue1425811)
http://gwt-code-reviews.appspot.com/1425811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding additional testing for GWT RPC. Some custom serialized objects (issue1424801)
Hmm - I just noticed this review - sorry! http://gwt-code-reviews.appspot.com/1424801/diff/1/user/test/com/google/gwt/user/client/rpc/CoreJavaTest.java File user/test/com/google/gwt/user/client/rpc/CoreJavaTest.java (right): http://gwt-code-reviews.appspot.com/1424801/diff/1/user/test/com/google/gwt/user/client/rpc/CoreJavaTest.java#newcode126 user/test/com/google/gwt/user/client/rpc/CoreJavaTest.java:126: * This test is disabled because it times out, apparently due to an infinite Where is the code to disable this? http://gwt-code-reviews.appspot.com/1424801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Remove the ability to deduce the script base using the marker script in the xsiframe linker sinc... (issue1425801)
Reviewers: zundel, Description: Remove the ability to deduce the script base using the marker script in the xsiframe linker since it doesn't work in FF3.5 and we can't make it work without sacrificing late loading Review by: zun...@google.com Please review this at http://gwt-code-reviews.appspot.com/1425801/ Affected files: A dev/core/src/com/google/gwt/core/ext/linker/impl/ComputeScriptBaseOld.js M dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java M dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Makes gadget linker shardable, derived from CrossSiteIframeLinker (issue1370808)
LGTM On 2011/04/15 21:25:03, zundel wrote: Obviously, my testing before uploading the previous patch left something to be desired. I uploaded some of the compiled samples to app engine and looked at them this time and made sure I wasn't looking at cached resources. Sorry, the diff got messed up switching between home and work working on this. http://gwt-code-reviews.appspot.com/1370808/diff/14001/gadgets/src/com/google/gwt/gadgets/linker/computeUrlForGadgetResource.js File gadgets/src/com/google/gwt/gadgets/linker/computeUrlForGadgetResource.js (right): http://gwt-code-reviews.appspot.com/1370808/diff/14001/gadgets/src/com/google/gwt/gadgets/linker/computeUrlForGadgetResource.js#newcode31 gadgets/src/com/google/gwt/gadgets/linker/computeUrlForGadgetResource.js:31: return __MODULE_FUNC__.__moduleBase + resource; On 2011/04/06 21:51:50, unnurg wrote: How is this file different than the computeUrlForResource.js that the linker uses? Umm, that isn't the file I intended to upload. http://gwt-code-reviews.appspot.com/1370808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Clean up the CrossSiteIFrameLoadingStrategy class, removing some dead code, (issue1413801)
Reviewers: zundel,
Description:
Clean up the CrossSiteIFrameLoadingStrategy class, removing some dead
code,
adding comments to clarify what is going on, and making it throw an
error
which is more explicit about what triggered the error
Please review this at http://gwt-code-reviews.appspot.com/1413801/
Affected files:
M user/src/com/google/gwt/core/client/CodeDownloadException.java
M
user/src/com/google/gwt/core/client/impl/CrossSiteIframeLoadingStrategy.java
Index: user/src/com/google/gwt/core/client/CodeDownloadException.java
===
--- user/src/com/google/gwt/core/client/CodeDownloadException.java
(revision 9972)
+++ user/src/com/google/gwt/core/client/CodeDownloadException.java (working
copy)
@@ -27,10 +27,10 @@
* need to be down loaded.
*/
public enum Reason {
-/**
- * Generic code for terminating the download.
- */
-TERMINATED
+ONERROR,
+ONLOAD,
+READYSTATELOADED,
+READYSTATECOMPLETE,
}
private final Reason reason;
Index:
user/src/com/google/gwt/core/client/impl/CrossSiteIframeLoadingStrategy.java
===
---
user/src/com/google/gwt/core/client/impl/CrossSiteIframeLoadingStrategy.java
(revision 9972)
+++
user/src/com/google/gwt/core/client/impl/CrossSiteIframeLoadingStrategy.java
(working copy)
@@ -25,14 +25,6 @@
/**
* Load runAsync code using a script tag. Intended for use with the
* {@link com.google.gwt.core.linker.CrossSiteIframeLinker}.
- *
- * p
- * The linker wraps its selection script code with a function refered to by
- * code__gwtModuleFunction/code. On that function is a property
- * codeinstallCode/code that can be invoked to eval more code in a
scope
- * nested somewhere within that function. The loaded script for fragment
123 is
- * expected to invoke code__gwtModuleFunction.runAsyncCallback123/code
- * as the final thing it does.
*/
public class CrossSiteIframeLoadingStrategy implements LoadingStrategy {
/**
@@ -58,10 +50,19 @@
}-*/;
}
- private static final RuntimeException LoadTerminated =
- new CodeDownloadException(Code download terminated,
-CodeDownloadException.Reason.TERMINATED);
-
+ private static final RuntimeException LoadTerminatedError =
+ new CodeDownloadException(Code download terminated - onError,
+CodeDownloadException.Reason.ONERROR);
+ private static final RuntimeException LoadTerminatedLoaded =
+new CodeDownloadException(Code download terminated - onLoad,
+ CodeDownloadException.Reason.ONLOAD);
+ private static final RuntimeException LoadTerminatedReadyComplete =
+new CodeDownloadException(Code download terminated - onReadyComplete,
+
CodeDownloadException.Reason.READYSTATECOMPLETE);
+ private static final RuntimeException LoadTerminatedReadyLoaded =
+new CodeDownloadException(Code download terminated - onReadyLoaded,
+
CodeDownloadException.Reason.READYSTATELOADED);
+
/**
* Clear callbacks on script objects. This is important on IE 6 and 7 to
* prevent a memory leak. If the callbacks aren't cleared, there is a
cyclical
@@ -71,13 +72,6 @@
private static native void clearCallbacks(JavaScriptObject script) /*-{
var nop = new Function('');
script.onerror = script.onload = script.onreadystatechange = nop;
- }-*/;
-
- /**
- * Clear the success callback for fragment codefragment/code.
- */
- private static native void clearOnSuccess(int fragment) /*-{
-delete __gwtModuleFunction['runAsyncCallback'+fragment];
}-*/;
private static native JavaScriptObject createScriptTag(String url) /*-{
@@ -97,13 +91,18 @@
LoadTerminatedHandler loadFinishedHandler) /*-{
return function(exception) {
if (tag.parentNode == null) {
- // onSuccess or onFailure must have already been called.
+ // This function must have already been called.
return;
}
var head = document.getElementsByTagName('head').item(0);
-
@com.google.gwt.core.client.impl.CrossSiteIframeLoadingStrategy::clearOnSuccess(*)(fragment);
@com.google.gwt.core.client.impl.CrossSiteIframeLoadingStrategy::clearCallbacks(*)(tag);
head.removeChild(tag);
+ // It seems unintuitive to call the error function every time, but
+ // it appears that AsyncFragmentLoader::fragmentHasLoaded (which is
+ // called by each fragment) will set the fragmentLoading variable to
+ // -1 when the code in this fragment executes, so this
+ // loadTerminated call will fail the (fragmentLoading == fragment)
check
+ // and will immediately exit, so no errors are actually fired.
function callLoadTerminated() {
[gwt-contrib] Some small refactors to Css Resource Generation (issue1406802)
Reviewers: robertvawter, Description: Some small refactors to Css Resource Generation Please review this at http://gwt-code-reviews.appspot.com/1406802/ Affected files: M user/src/com/google/gwt/resources/client/CssResource.java A user/src/com/google/gwt/resources/client/CssResourceBase.java A user/src/com/google/gwt/resources/rg/CssObfuscationStyle.java M user/src/com/google/gwt/resources/rg/CssResourceGenerator.java M user/test/com/google/gwt/resources/rg/CssTestCase.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] make it possible to just use devmode on a particular module while allowing the (issue1408802)
Reviewers: fabiomfv,
Description:
make it possible to just use devmode on a particular module while
allowing the
others to run in prod mode
Please review this at http://gwt-code-reviews.appspot.com/1408802/
Affected files:
M dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js
M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java
M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
Index: dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js
===
--- dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js (revision
9963)
+++ dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js (working
copy)
@@ -252,7 +252,10 @@
var query = $wnd.location.search;
var idx = query.indexOf(gwt.codesvr=);
if (idx = 0) {
-idx += 12; // gwt.codesvr=.length() == 12
+idx += 12; // gwt.codesvr=.length == 12
+ } else {
+idx = query.indexOf(gwt.codesvr.__MODULE_NAME__=);
+idx += (13 + __MODULE_NAME__.length); //
}
if (idx = 0) {
var amp = query.indexOf(, idx);
Index: dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java
===
--- dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java
(revision 9963)
+++ dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java
(working copy)
@@ -339,6 +339,7 @@
outputFilename = getHostedFilenameFull(context);
}
+replaceAll(buffer, __MODULE_NAME__, context.getModuleName());
String script =
generatePrimaryFragmentString(logger, context, result,
buffer.toString(), 1, artifacts);
Index: dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
===
--- dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
(revision 9963)
+++ dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
(working copy)
@@ -34,7 +34,8 @@
function isHostedMode() {
var query = $wnd.location.search;
-return (query.indexOf('gwt.codesvr=') != -1);
+return ((query.indexOf('gwt.codesvr.__MODULE_NAME__=') != -1) ||
+(query.indexOf('gwt.codesvr=') != -1));
}
// Helper function to send statistics to the __gwtStatsEvent function if
it
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Makes gadget linker shardable, derived from CrossSiteIframeLinker (issue1370808)
http://gwt-code-reviews.appspot.com/1370808/diff/14001/gadgets/src/com/google/gwt/gadgets/linker/computeUrlForGadgetResource.js File gadgets/src/com/google/gwt/gadgets/linker/computeUrlForGadgetResource.js (right): http://gwt-code-reviews.appspot.com/1370808/diff/14001/gadgets/src/com/google/gwt/gadgets/linker/computeUrlForGadgetResource.js#newcode31 gadgets/src/com/google/gwt/gadgets/linker/computeUrlForGadgetResource.js:31: return __MODULE_FUNC__.__moduleBase + resource; How is this file different than the computeUrlForResource.js that the linker uses? http://gwt-code-reviews.appspot.com/1370808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Provides a wrapper around transforming a resource name (filename or URL) (issue1385801)
http://gwt-code-reviews.appspot.com/1385801/diff/1/dev/core/src/com/google/gwt/core/ext/linker/impl/HostedModeTemplate.js
File
dev/core/src/com/google/gwt/core/ext/linker/impl/HostedModeTemplate.js
(right):
http://gwt-code-reviews.appspot.com/1385801/diff/1/dev/core/src/com/google/gwt/core/ext/linker/impl/HostedModeTemplate.js#newcode358
dev/core/src/com/google/gwt/core/ext/linker/impl/HostedModeTemplate.js:358:
var url = computeUrlForResource(__MODULE_NAME__.nocache.js);
I don't think you want to touch this file - I don't recognize it, so I
think it's for one of the old linkers, which won't have
computeUrlForResource defined (note how it uses the 'base' variable,
which is part of the legacy linkers).
http://gwt-code-reviews.appspot.com/1385801/diff/1/dev/core/src/com/google/gwt/core/ext/linker/impl/loadExternalStylesheets.js
File
dev/core/src/com/google/gwt/core/ext/linker/impl/loadExternalStylesheets.js
(right):
http://gwt-code-reviews.appspot.com/1385801/diff/1/dev/core/src/com/google/gwt/core/ext/linker/impl/loadExternalStylesheets.js#newcode13
dev/core/src/com/google/gwt/core/ext/linker/impl/loadExternalStylesheets.js:13:
l.setAttribute('href', computeUrlForResource(stylesheetUrl));
I think you should leave this file as is, and instead change
ResourceInjectionUtil - line 85. Otherwise we calculate the URL in 2
places.
http://gwt-code-reviews.appspot.com/1385801/diff/1/dev/core/src/com/google/gwt/core/ext/linker/impl/permutations.js
File dev/core/src/com/google/gwt/core/ext/linker/impl/permutations.js
(right):
http://gwt-code-reviews.appspot.com/1385801/diff/1/dev/core/src/com/google/gwt/core/ext/linker/impl/permutations.js#newcode45
dev/core/src/com/google/gwt/core/ext/linker/impl/permutations.js:45:
return __HOSTED_FILENAME__;
On 2011/03/11 16:53:17, zundel wrote:
these 2 changes make me a bit queasy. They change the API for
getCompiledCodeFilename().. but if I don't leave these resources
un-based, then
we lose flexibility in the computeUrlForResource() method.
I'm confused - why don't you just return
computeUrlForResource(__HOSTED_FILENAME__) here?
http://gwt-code-reviews.appspot.com/1385801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Provides a wrapper around transforming a resource name (filename or URL) (issue1385801)
LGTM
http://gwt-code-reviews.appspot.com/1385801/diff/1010/dev/core/src/com/google/gwt/core/ext/linker/impl/computeUrlForResource.js
File
dev/core/src/com/google/gwt/core/ext/linker/impl/computeUrlForResource.js
(right):
http://gwt-code-reviews.appspot.com/1385801/diff/1010/dev/core/src/com/google/gwt/core/ext/linker/impl/computeUrlForResource.js#newcode24
dev/core/src/com/google/gwt/core/ext/linker/impl/computeUrlForResource.js:24:
if (resource.match(/^\//)) {
On 2011/03/11 20:03:16, zundel wrote:
I could have used resources.substr() here. Any preference?
To be honest, I don't know about the tradeoffs. In general, this should
happen only a few times per page and each stylesheet fetched has a huge
amount of overhead already compared to this (since it involves a
resource fetch), so I think either is fine.
http://gwt-code-reviews.appspot.com/1385801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Makes gadget linker shardable, derived from CrossSiteIframeLinker (issue1370808)
http://gwt-code-reviews.appspot.com/1370808/diff/1010/gadgets/src/com/google/gwt/gadgets/linker/computeGadgetsBase.js
File gadgets/src/com/google/gwt/gadgets/linker/computeGadgetsBase.js
(right):
http://gwt-code-reviews.appspot.com/1370808/diff/1010/gadgets/src/com/google/gwt/gadgets/linker/computeGadgetsBase.js#newcode23
gadgets/src/com/google/gwt/gadgets/linker/computeGadgetsBase.js:23: base
= tmpBase
You can delete this line - we only set the base variable in
computeScriptBase for backwards compatibility with the std and other old
linkers.
http://gwt-code-reviews.appspot.com/1370808/diff/1010/gadgets/src/com/google/gwt/gadgets/linker/installGadgetsScript.js
File gadgets/src/com/google/gwt/gadgets/linker/installGadgetsScript.js
(right):
http://gwt-code-reviews.appspot.com/1370808/diff/1010/gadgets/src/com/google/gwt/gadgets/linker/installGadgetsScript.js#newcode38
gadgets/src/com/google/gwt/gadgets/linker/installGadgetsScript.js:38:
script.src = $wnd.gadgets.io.getProxyUrl(filename);
It seems heavy handed to me for you to maintain whole copies of
installScriptEarlyDownload and loadExternalStylesheets.js just to make
this one line switch in each of them. Also - in order to make devmode
work, I think you'll need to do this switch for line 45 of
permutations.js (where we reference the devmode.js file).
At a minimum, I think you want to override permutations.js and update
the devmode.js and md5.js paths there, rather than overriding this file
and updating the md5.js filepath here.
What I think is a better long term solution, would be the following:
- Add a function called getFullUrl(path) { return
__MODULE_FUNC__.__moduleBase + path; } to the xsiframe linker. For
now, it's probably convenient enough to add that function to
computeScriptBase.js if you don't want to set up a whole new .js
include.
- Do a code search to find all the places where the linker does a
__MODULE_FUNC__.__moduleBase + something, and replace it with a
getFullUrl(something); call (I believe there's only 3 places where we do
that, and all 3 are the places you need to override anyway.
- In your overridden computeGadgetsBase.js file, override the
getFullUrl() function to do the getProxyUrl() call rather than appending
the path to the moduleBase
Does that sound like it would work? It covers all the changes that I
noticed in these files, although it's possible that I missed something.
It's a little more upfront work, but I think that long term, it'll make
maintaining this linker a lot easier...
http://gwt-code-reviews.appspot.com/1370808/diff/1010/gadgets/src/com/google/gwt/gadgets/linker/processGadgetsMetas.js
File gadgets/src/com/google/gwt/gadgets/linker/processGadgetsMetas.js
(right):
http://gwt-code-reviews.appspot.com/1370808/diff/1010/gadgets/src/com/google/gwt/gadgets/linker/processGadgetsMetas.js#newcode33
gadgets/src/com/google/gwt/gadgets/linker/processGadgetsMetas.js:33: *
place in the gadget linker to propagate gadget logic.
So the problem with putting this here is that now you're maintaining a
copy of all of the processMetas logic, and if that changes, you won't
get updates. Would it make more sense if we put in an arbitrary JS
hook into the xsiframe linker, so you could just override that (which
would be empty by default) rather than the processMetas JS?
http://gwt-code-reviews.appspot.com/1370808/diff/1010/gadgets/src/com/google/gwt/gadgets/linker/processGadgetsMetas.js#newcode41
gadgets/src/com/google/gwt/gadgets/linker/processGadgetsMetas.js:41: //
hack! assumes the installLocation() is window
Why would you not get rid of this hack and just call
getInstallLocation() here?
http://gwt-code-reviews.appspot.com/1370808/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Refactor css generation code to prepare for eventually adding an external version (issue1362801)
Reviewers: robertvawter, Description: Refactor css generation code to prepare for eventually adding an external version which uses a generated .css file. Please review this at http://gwt-code-reviews.appspot.com/1362801/ Affected files: A user/src/com/google/gwt/resources/css/CheckStaticCssVisitor.java M user/src/com/google/gwt/resources/css/CssGenerationVisitor.java M user/src/com/google/gwt/resources/css/ast/CollapsedNode.java M user/src/com/google/gwt/resources/css/ast/CssDef.java M user/src/com/google/gwt/resources/css/ast/CssExternalSelectors.java M user/src/com/google/gwt/resources/css/ast/CssIf.java M user/src/com/google/gwt/resources/css/ast/CssMediaRule.java M user/src/com/google/gwt/resources/css/ast/CssNoFlip.java M user/src/com/google/gwt/resources/css/ast/CssNode.java M user/src/com/google/gwt/resources/css/ast/CssPageRule.java M user/src/com/google/gwt/resources/css/ast/CssProperty.java M user/src/com/google/gwt/resources/css/ast/CssRule.java M user/src/com/google/gwt/resources/css/ast/CssSelector.java M user/src/com/google/gwt/resources/css/ast/CssSprite.java M user/src/com/google/gwt/resources/css/ast/CssStylesheet.java A user/src/com/google/gwt/resources/css/ast/CssSubstitution.java M user/src/com/google/gwt/resources/css/ast/CssUnknownAtRule.java M user/src/com/google/gwt/resources/css/ast/CssUrl.java M user/src/com/google/gwt/resources/rg/CssResourceGenerator.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Do not put ?serial=0 at the end of the URL for the first fetch attempt on a runAsync fragement... (issue1358804)
On 2011/02/18 23:24:10, jaredking wrote: I'd vote for leaving the CrossSite linker alone for now. It's been around a long time, and will be deprecated soon, so changing the behavior probably isn't worth it, unless there's a specific reason you need it changed along with the CrossSiteIframeLinker. http://gwt-code-reviews.appspot.com/1358804/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Do not put ?serial=0 at the end of the URL for the first fetch attempt on a runAsync fragement... (issue1358804)
On 2011/02/18 23:46:55, jaredking wrote: LGTM http://gwt-code-reviews.appspot.com/1358804/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add option to use JSONP in ExternalTextResource (issue1310801)
http://gwt-code-reviews.appspot.com/1310801/diff/1/3
File user/src/com/google/gwt/jsonp/client/JsonpRequest.java (right):
http://gwt-code-reviews.appspot.com/1310801/diff/1/3#newcode70
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:70: }
On 2011/01/24 14:31:26, bobv wrote:
Extra whitespace. GPE usually prevents formatting weirdness.
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/3#newcode75
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:75: // The P
suffix must stay in sync with ExternalTextResourceGenerator.java
On 2011/01/24 14:31:26, bobv wrote:
Use a shared constant field?
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/3#newcode149
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:149: String id) {
On 2011/01/24 14:31:26, bobv wrote:
Formatting.
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/3#newcode219
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:219: return
callbackId.startsWith(P);
On 2011/01/24 14:31:26, bobv wrote:
This seems kind of hacky. Is there a way to do this in a way that
would prevent
someone from accidentally triggering this behavior with a
poorly-chosen prefix?
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/3#newcode279
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:279: if
(!canHaveMultipleRequestsForId) {
On 2011/01/24 14:31:26, bobv wrote:
Could you reverse the order of the then and else clauses to prevent a
negative
comparison?
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/3#newcode289
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:289: if
(callbackWrapper == null) {
On 2011/01/24 14:31:26, bobv wrote:
if (!callbackWrapper) is more idiomatic.
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/5
File user/src/com/google/gwt/resources/Resources.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1310801/diff/1/5#newcode83
user/src/com/google/gwt/resources/Resources.gwt.xml:83: !-- This can be
used to make ExternalTextResource use JSONP rather than XHR --
On 2011/01/24 14:31:26, bobv wrote:
Why not always use JSONP? It's guaranteed to work across more
deployment
schemes than XHR, right?
Mostly because I'm just a little nervous about changing the default
behavior. I could be convinced if you feel strongly... Another option
would be to leave it as is for now, and then flip the default value for
the property (or remove the XHR implementation entirely if you feel
strongly) once AW3 is successfully using this version.
http://gwt-code-reviews.appspot.com/1310801/diff/1/6
File
user/src/com/google/gwt/resources/client/impl/ExternalTextResourcePrototype.java
(right):
http://gwt-code-reviews.appspot.com/1310801/diff/1/6#newcode87
user/src/com/google/gwt/resources/client/impl/ExternalTextResourcePrototype.java:87:
};
On 2011/01/24 14:31:26, bobv wrote:
Extra whitespace.
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/6#newcode170
user/src/com/google/gwt/resources/client/impl/ExternalTextResourcePrototype.java:170:
// }
On 2011/01/24 14:31:26, bobv wrote:
Delete this?
Actually - it should stay - I just commented it out for some debugging -
good catch!
http://gwt-code-reviews.appspot.com/1310801/diff/1/6#newcode172
user/src/com/google/gwt/resources/client/impl/ExternalTextResourcePrototype.java:172:
if (!.equals(md5Hash)) {
On 2011/01/24 14:31:26, bobv wrote:
How about a null comparison instead?
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/7
File
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java
(right):
http://gwt-code-reviews.appspot.com/1310801/diff/1/7#newcode54
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:54:
static final String JSONP_CALLBACK_PREFIX = __gwt_jsonp__.P;
On 2011/01/24 14:31:26, bobv wrote:
If the .P is special, it should be in a constant field.
We discussed this in the previous CL - the problem is that if I want to
make shared with the other file, then I cross the user/dev dependency
boundary, so you said to go ahead and just add a comment to keep them in
sync
http://gwt-code-reviews.appspot.com/1310801/diff/1/7#newcode184
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:184:
if (Boolean.parseBoolean(useJsonpProp)) {
On 2011/01/24 14:31:26, bobv wrote:
How about
return Boolean.parseBoolean(useJsonProp);
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/11
File
user/test/com/google/gwt/resources/client/ExternalTextResourceJsonpTest.java
(right):
http://gwt-code-reviews.appspot.com/1310801/diff/1/11#newcode2
user/test/com/google/gwt/resources/client/ExternalTextResourceJsonpTest.java:2:
* Copyright 2010 Google Inc.
On 2011/01/24 14:31:26, bobv wrote:
2011
Done.
http://gwt-code-reviews.appspot.com/1310801/diff/1/12
File
user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java
(right):
http://gwt-code-reviews.appspot.com/1310801/diff/1/12#newcode2
[gwt-contrib] Re: Add option to use JSONP in ExternalTextResource (issue1310801)
http://gwt-code-reviews.appspot.com/1310801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add option to use JSONP in ExternalTextResource (issue1310801)
http://gwt-code-reviews.appspot.com/1310801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add option to use JSONP in ExternalTextResource (issue1310801)
http://gwt-code-reviews.appspot.com/1310801/diff/24001/25002 File user/src/com/google/gwt/jsonp/client/JsonpRequest.java (right): http://gwt-code-reviews.appspot.com/1310801/diff/24001/25002#newcode39 user/src/com/google/gwt/jsonp/client/JsonpRequest.java:39: On 2011/01/25 19:49:32, bobv wrote: Whitespace. Done. http://gwt-code-reviews.appspot.com/1310801/diff/24001/25002#newcode85 user/src/com/google/gwt/jsonp/client/JsonpRequest.java:85: // The P suffix must stay in sync with ExternalTextResourceGenerator.java On 2011/01/25 19:49:32, bobv wrote: Move this comment to the constant declaration. Done. http://gwt-code-reviews.appspot.com/1310801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add option to use JSONP in ExternalTextResource (issue1310801)
http://gwt-code-reviews.appspot.com/1310801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add option to use JSONP in ExternalTextResource (issue1310801)
http://gwt-code-reviews.appspot.com/1310801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add option to use JSONP in ExternalTextResource (issue1310801)
http://gwt-code-reviews.appspot.com/1310801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Updated StackTraceDeobfuscator javadoc in follow up to r9162. (issue1321801)
LGTM On 2011/01/24 19:53:04, fredsa wrote: http://gwt-code-reviews.appspot.com/1321801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add option to use JSONP in ExternalTextResource (issue1310801)
http://gwt-code-reviews.appspot.com/1310801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add option to use JSONP in ExternalTextResource (issue1310801)
Reviewers: robertvawter, Description: Add option to use JSONP in ExternalTextResource Review by: robertvaw...@google.com Please review this at http://gwt-code-reviews.appspot.com/1310801/show Affected files: M tools/api-checker/config/gwt21_22userApi.conf M user/src/com/google/gwt/jsonp/client/JsonpRequest.java M user/src/com/google/gwt/jsonp/client/JsonpRequestBuilder.java M user/src/com/google/gwt/resources/Resources.gwt.xml M user/src/com/google/gwt/resources/client/impl/ExternalTextResourcePrototype.java M user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java M user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java A user/test/com/google/gwt/resources/ExternalTextResourceJsonp.gwt.xml M user/test/com/google/gwt/resources/ResourcesSuite.java A user/test/com/google/gwt/resources/client/ExternalTextResourceJsonpTest.java A user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java M user/test/com/google/gwt/resources/client/TextResourceTest.java A user/test/com/google/gwt/resources/client/shouldBeEscaped.txt -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add property provider generators. (issue1063801)
LGTM On 2010/12/20 20:20:49, jat wrote: Annotating changes which came from merges, and are not part of this change. http://gwt-code-reviews.appspot.com/1063801/diff/3001/4004 File dev/core/src/com/google/gwt/core/ext/linker/impl/PropertiesUtil.java (right): http://gwt-code-reviews.appspot.com/1063801/diff/3001/4004#newcode1 dev/core/src/com/google/gwt/core/ext/linker/impl/PropertiesUtil.java:1: /* All the delta from patch set 1 came from merges, and are not part of my change. http://gwt-code-reviews.appspot.com/1063801/diff/3001/4011 File tools/api-checker/config/gwt20_21userApi.conf (right): http://gwt-code-reviews.appspot.com/1063801/diff/3001/4011#newcode1 tools/api-checker/config/gwt20_21userApi.conf:1: #existing API All the delta from patch set 1 came from merges, and are not part of my change. http://gwt-code-reviews.appspot.com/1063801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make the logging framework provide a default uncaught exception handler. (issue1223802)
LGTM On 2010/12/17 02:11:10, rjrjr wrote: http://gwt-code-reviews.appspot.com/1223802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make the logging framework provide a default uncaught exception handler. (issue1223802)
Unfortunately not On 2010/12/17 20:33:56, unnurg wrote: LGTM On 2010/12/17 02:11:10, rjrjr wrote: http://gwt-code-reviews.appspot.com/1223802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make ExternalTextResource use Jsonp (issue1214801)
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21004
File user/src/com/google/gwt/resources/Resources.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21004#newcode83
user/src/com/google/gwt/resources/Resources.gwt.xml:83: !-- This can be
used to make ExternalTextResource use JSONP rather than XHR --
On 2010/12/15 12:51:57, bobv wrote:
... by setting the value to true
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21005
File
user/src/com/google/gwt/resources/client/impl/ExternalTextResourcePrototype.java
(right):
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21005#newcode138
user/src/com/google/gwt/resources/client/impl/ExternalTextResourcePrototype.java:138:
TextResource[] cache, int index, String md5Hash, boolean useJsonp) {
On 2010/12/15 12:51:57, bobv wrote:
The presence of md5Hash implies useJsonp. It would be better to add
the jsonp
variant as a separate constructor.
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21007
File user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java
(right):
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21007#newcode239
user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java:239: public
void testPredeterminedIds() {
On 2010/12/15 12:51:57, bobv wrote:
Sort order. It's not enforced by the checkstyle rules in test code.
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21009
File user/test/com/google/gwt/resources/ResourcesSuite.java (right):
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21009#newcode62
user/test/com/google/gwt/resources/ResourcesSuite.java:62:
suite.addTestSuite(UnknownAtRuleTest.class);
On 2010/12/15 12:51:57, bobv wrote:
The order of these tests doesn't matter, can you sort the
addTestSuite() while
you're in the area?
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21011
File
user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java
(right):
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21011#newcode26
user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java:26:
static interface Resources extends ClientBundleWithLookup {
On 2010/12/15 12:51:57, bobv wrote:
Redundant modifier, interfaces are implicitly static.
To make sure the bundling is working correctly, there should be more
than one
ExternalTextResource in the ClientBundle. Also, make sure that the
JSON
escaping is working correctly by adding a file containing characters
that must
be escaped. A sequence like dobule-quotesingle-quotebackslash
should be
sufficient.
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/20001/21011#newcode59
user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java:59:
On 2010/12/15 12:51:57, bobv wrote:
Extra blank line.
Done.
http://gwt-code-reviews.appspot.com/1214801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Have separate devmode.js files for each permutation if we are outputting (issue1208802)
http://gwt-code-reviews.appspot.com/1208802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Have separate devmode.js files for each permutation if we are outputting (issue1208802)
http://gwt-code-reviews.appspot.com/1208802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make ExternalTextResource use Jsonp (issue1214801)
http://gwt-code-reviews.appspot.com/1214801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make ExternalTextResource use Jsonp (issue1214801)
http://gwt-code-reviews.appspot.com/1214801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make ExternalTextResource use Jsonp (issue1214801)
http://gwt-code-reviews.appspot.com/1214801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make ExternalTextResource use Jsonp (issue1214801)
http://gwt-code-reviews.appspot.com/1214801/diff/1/2
File user/src/com/google/gwt/jsonp/client/JsonpRequest.java (right):
http://gwt-code-reviews.appspot.com/1214801/diff/1/2#newcode33
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:33:
On 2010/12/13 21:42:50, jat wrote:
Added whitespace.
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/1/2#newcode78
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:78:
On 2010/12/13 21:42:50, jat wrote:
Whitespace.
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/1/2#newcode129
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:129: * Create a
new JSONP request.
On 2010/12/13 21:42:50, jat wrote:
Shouldn't this distinguish from the above constructor, explaining why
this would
be used?
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/1/2#newcode142
user/src/com/google/gwt/jsonp/client/JsonpRequest.java:142: String
callbackParam, String failureCallbackParam, String id) {
On 2010/12/13 21:42:50, jat wrote:
Need javadoc for id.
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/1/6
File
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java
(right):
http://gwt-code-reviews.appspot.com/1214801/diff/1/6#newcode67
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:67:
On 2010/12/13 21:42:50, jat wrote:
Whitespace.
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/1/6#newcode91
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:91:
On 2010/12/13 21:42:50, jat wrote:
Whitespace.
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/1/6#newcode154
user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java:154:
On 2010/12/13 21:42:50, jat wrote:
Whitespace.
Something must be set wrong in your IDE to generate all these where
they didn't
exist previously.
It seems like it doesn't it? I have the gwt formatting stuff imported
though, and as far as I can tell, indent is not checked for blank
lines...
http://gwt-code-reviews.appspot.com/1214801/diff/1/8
File
user/test/com/google/gwt/resources/ExternalTextResourceJsonp.gwt.xml
(right):
http://gwt-code-reviews.appspot.com/1214801/diff/1/8#newcode16
user/test/com/google/gwt/resources/ExternalTextResourceJsonp.gwt.xml:16:
set-configuration-property name=ExternalTextResource.useJsonp
value=true /
On 2010/12/13 21:42:50, jat wrote:
80 columns
Done.
http://gwt-code-reviews.appspot.com/1214801/diff/1/11
File
user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java
(right):
http://gwt-code-reviews.appspot.com/1214801/diff/1/11#newcode26
user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java:26:
static interface Resources extends ClientBundleWithLookup {
On 2010/12/13 21:42:50, jat wrote:
Why package protected?
Done.
http://gwt-code-reviews.appspot.com/1214801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Make ExternalTextResource use Jsonp (issue1214801)
Reviewers: robertvawter, Description: Make ExternalTextResource use Jsonp Review by: robertvaw...@google.com Please review this at http://gwt-code-reviews.appspot.com/1214801/show Affected files: M user/src/com/google/gwt/jsonp/client/JsonpRequest.java M user/src/com/google/gwt/jsonp/client/JsonpRequestBuilder.java M user/src/com/google/gwt/resources/Resources.gwt.xml M user/src/com/google/gwt/resources/client/impl/ExternalTextResourcePrototype.java M user/src/com/google/gwt/resources/rg/ExternalTextResourceGenerator.java M user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java A user/test/com/google/gwt/resources/ExternalTextResourceJsonp.gwt.xml M user/test/com/google/gwt/resources/ResourcesSuite.java A user/test/com/google/gwt/resources/client/ExternalTextResourceJsonpTest.java A user/test/com/google/gwt/resources/client/ExternalTextResourceTest.java M user/test/com/google/gwt/resources/client/TextResourceTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Have separate devmode.js files for each permutation if we are outputting (issue1208802)
Reviewers: conroy, Description: Have separate devmode.js files for each permutation if we are outputting bootstrap in the primary fragment so we can put the properties info in them and avoid putting it in the primary fragment. Also, fail explicitly when we encounter a script tag in the gwt.xml rather than failing silently (xsiframe linker does not support this feature) Please review this at http://gwt-code-reviews.appspot.com/1208802/show Affected files: M dev/core/src/com/google/gwt/core/ext/linker/impl/PropertiesUtil.java M dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java M dev/core/src/com/google/gwt/core/ext/linker/impl/propertiesNull.js M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: The symbolMaps directory provided to StackTraceDeobfuscator should accepts directory name with (issue1180801)
LGTM On 2010/12/02 20:00:55, fredsa wrote: http://gwt-code-reviews.appspot.com/1180801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: StackTraceDeobfuscator now extracts source file and (method declaration) line number information... (issue1175801)
LGTM On 2010/12/01 19:23:57, fredsa wrote: http://gwt-code-reviews.appspot.com/1175801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix up the plugin initialization code in devmode.js. Also, attach the plugin (issue1120801)
http://gwt-code-reviews.appspot.com/1120801/diff/1/2
File dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js
(right):
http://gwt-code-reviews.appspot.com/1120801/diff/1/2#newcode322
dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js:322: if
(maybePlugin != null maybePlugin.init(window)) {
I see, so we may find a non-null plugin that doesn't init, in which case
we have to keep searching rather than erroring out.
http://gwt-code-reviews.appspot.com/1120801/diff/1/2#newcode331
dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js:331:
pluginConnectionError(codeServer);
Are you sure this should be a connection error? Returning null will:
loadIframe(http://gwt.google.com/missing-plugin/;);
However, if you call pluginConnectionError, it will show the failed to
connect popup and load the TroubleshootingOOPHM iframe, which preempts
the loading of the missing plugin iframe.
Therefore, if we really are missing the plugin, we need to return null
without the connection error.
At this point, it's hard to tell if there never was a maybePlugin, or
whether there was and it just didn't init. I'm not 100% sure which of
those corresponds to there not being a plugin vs. a real connection
error - do you know?
http://gwt-code-reviews.appspot.com/1120801/diff/1/2#newcode334
dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js:334: if
(!plugin.init(window)) {
When will this happen? From what I can tell, plugin will not be
assigned, unless maybePlugin.init(window) succeeded?
http://gwt-code-reviews.appspot.com/1120801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix up the plugin initialization code in devmode.js. Also, attach the plugin (issue1120801)
LGTM Thanks! On 2010/11/17 21:56:13, conroy wrote: http://gwt-code-reviews.appspot.com/1120801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] In a previous patch, I fixed the doc.findElements... function to be document.findElements, which... (issue1118801)
Reviewers: conroy,
Description:
In a previous patch, I fixed the doc.findElements... function to be
document.findElements, which uncovered
this bug where we were calling append rather than appendChild, which
hoses on Safari. I'm not sure why
it was ok for Chrome, which is where we tested the original fix...
Please review this at http://gwt-code-reviews.appspot.com/1118801/show
Affected files:
M dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js
Index: dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js
===
--- dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js (revision
9233)
+++ dev/core/src/com/google/gwt/core/ext/linker/impl/devmode.js (working
copy)
@@ -215,8 +215,8 @@
obj.CLASSID = 'CLSID:1D6156B6-002B-49E7-B5CA-C138FB843B4E';
var dochead = document.getElementsByTagName('head')[0];
- dochead.append(embed);
- dochead.append(obj);
+ dochead.appendChild(embed);
+ dochead.appendChild(obj);
}
function findPluginObject() {
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Check for null as well as undefined in isBodyLoaded() (issue1111801)
Reviewers: conroy, Description: Check for null as well as undefined in isBodyLoaded() Please review this at http://gwt-code-reviews.appspot.com/801/show Affected files: M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js Index: dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js === --- dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js (revision 9233) +++ dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js (working copy) @@ -27,7 +27,7 @@ // FF 3.5 and below does not have readyState, but it does allow us to // append to the body before it has finished loading, so we return whether // the body element exists. - return (typeof $doc.body != undefined); + return (typeof $doc.body != undefined $doc.body != null); } return (/loaded|complete/.test($doc.readyState)); } -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Update some references to document to be $doc (issue1112801)
Reviewers: conroy,
Description:
Update some references to document to be $doc
Please review this at http://gwt-code-reviews.appspot.com/1112801/show
Affected files:
M
dev/core/src/com/google/gwt/core/ext/linker/impl/installLocationIframe.js
M
dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js
M dev/core/src/com/google/gwt/core/ext/linker/impl/processMetas.js
Index:
dev/core/src/com/google/gwt/core/ext/linker/impl/installLocationIframe.js
===
---
dev/core/src/com/google/gwt/core/ext/linker/impl/installLocationIframe.js
(revision 9233)
+++
dev/core/src/com/google/gwt/core/ext/linker/impl/installLocationIframe.js
(working copy)
@@ -29,7 +29,7 @@
scriptFrame.id = '__MODULE_NAME__';
scriptFrame.style.cssText = 'position:absolute; width:0; height:0;
border:none; left: -1000px; top: -1000px; !important';
scriptFrame.tabIndex = -1;
- document.body.appendChild(scriptFrame);
+ $doc.body.appendChild(scriptFrame);
frameDoc = scriptFrame.contentDocument;
if (!frameDoc) {
Index:
dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js
===
---
dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js
(revision 9233)
+++
dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js
(working copy)
@@ -32,7 +32,7 @@
if (isBodyLoaded()) {
// if the body is loaded, then the tag to download the script can be
added
// in a non-destructive manner
-var script = document.createElement('script');
+var script = $doc.createElement('script');
script.src = filename;
$doc.getElementsByTagName('head')[0].appendChild(script);
} else {
Index: dev/core/src/com/google/gwt/core/ext/linker/impl/processMetas.js
===
--- dev/core/src/com/google/gwt/core/ext/linker/impl/processMetas.js
(revision 9233)
+++ dev/core/src/com/google/gwt/core/ext/linker/impl/processMetas.js
(working copy)
@@ -26,7 +26,7 @@
var propertyErrorFunc;
var onLoadErrorFunc;
- var metas = document.getElementsByTagName('meta');
+ var metas = $doc.getElementsByTagName('meta');
for (var i = 0, n = metas.length; i n; ++i) {
var meta = metas[i]
, name = meta.getAttribute('name')
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix issue with FF3.5 and below not having readyState (issue1087801)
Reviewers: jgw,
Description:
Fix issue with FF3.5 and below not having readyState
Please review this at http://gwt-code-reviews.appspot.com/1087801/show
Affected files:
M
dev/core/src/com/google/gwt/core/ext/linker/impl/waitForBodyLoadedNull.js
M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
Index:
dev/core/src/com/google/gwt/core/ext/linker/impl/waitForBodyLoadedNull.js
===
---
dev/core/src/com/google/gwt/core/ext/linker/impl/waitForBodyLoadedNull.js
(revision 9194)
+++
dev/core/src/com/google/gwt/core/ext/linker/impl/waitForBodyLoadedNull.js
(working copy)
@@ -1,8 +1,3 @@
-// Check whether the body is loaded.
-function isBodyLoaded() {
- return true;
-}
-
// Setup code which waits for the body to be loaded and then calls the
// callback function
function setupWaitForBodyLoad(callback) {
Index: dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
===
--- dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
(revision 9194)
+++ dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
(working copy)
@@ -23,6 +23,12 @@
***/
function isBodyLoaded() {
+if (typeof $doc.readyState == undefined) {
+ // FF 3.5 and below does not have readyState, but it does allow us to
+ // append to the body before it has finished loading, so we return
whether
+ // the body element exists.
+ return (typeof $doc.body != undefined);
+}
return (/loaded|complete/.test($doc.readyState));
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Ensure that all base URL's are absolut-ify-ed, no matter where we get them from. (issue1066801)
Reviewers: amitmanjhi, Description: Ensure that all base URL's are absolut-ify-ed, no matter where we get them from. Also refactor the compute script base code to be easier to read Please review this at http://gwt-code-reviews.appspot.com/1066801/show Affected files: M dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js M dev/core/test/com/google/gwt/core/ext/linker/impl/SelectionScriptJavaScriptTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Ensure that all base URL's are absolut-ify-ed, no matter where we get them from. (issue1066801)
http://gwt-code-reviews.appspot.com/1066801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix issue with SSSS not getting stylesheets (issue1059801)
Reviewers: jgw, Description: Fix issue with not getting stylesheets Please review this at http://gwt-code-reviews.appspot.com/1059801/show Affected files: M dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java M dev/core/src/com/google/gwt/core/linker/IFrameLinker.java M dev/core/src/com/google/gwt/core/linker/SingleScriptLinker.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove the onFailure call in case of unauthenticated user. This gets rid of the (issue1061801)
On 2010/10/28 22:00:18, amitmanjhi wrote: Bob should confirm this, but LGTM. I believe that this got added when Bob added the recievers, and he saw that we were logging a finest message and assumed that this was a failure case. This is not a failure case, it is expected. There is an event listener which listens for UNAUTHORIZED responses, and when it sees it, it redirects. However, there shouldn't be any errors fired when this happens - it is an expected codepath. http://gwt-code-reviews.appspot.com/1061801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix some issues in the xsiframe linker (issue1057801)
Reviewers: jgw, Description: Fix some issues in the xsiframe linker - Add support for including stylesheet tags - Fix a bug in computeScriptBase.js Please review this at http://gwt-code-reviews.appspot.com/1057801/show Affected files: M dev/core/src/com/google/gwt/core/ext/linker/impl/ResourceInjectionUtil.java M dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptAlreadyIncluded.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptDirect.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptNull.js A dev/core/src/com/google/gwt/core/ext/linker/impl/loadExternalStylesheets.js M dev/core/src/com/google/gwt/core/ext/linker/impl/waitForBodyLoaded.js M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a new artifact visibility Deploy (in addition to existing (issue1055802)
LGTM On 2010/10/27 00:28:51, jat wrote: http://gwt-code-reviews.appspot.com/1055802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make the following method public in StackTraceDeobfuscator, for reuse by user server side code: (issue1040801)
LGTM On 2010/10/21 00:38:43, fredsa wrote: http://gwt-code-reviews.appspot.com/1040801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix deobfuscation of throwables with a cause (issue987801)
Reviewers: fredsa,
Description:
Fix deobfuscation of throwables with a cause
Review by: fre...@google.com
Please review this at http://gwt-code-reviews.appspot.com/987801/show
Affected files:
M user/src/com/google/gwt/logging/server/StackTraceDeobfuscator.java
Index: user/src/com/google/gwt/logging/server/StackTraceDeobfuscator.java
===
--- user/src/com/google/gwt/logging/server/StackTraceDeobfuscator.java
(revision 9030)
+++ user/src/com/google/gwt/logging/server/StackTraceDeobfuscator.java
(working copy)
@@ -82,12 +82,15 @@
return newSt;
}
- private Throwable deobfuscateThrowable(Throwable t, String strongName) {
-if (t.getStackTrace() != null) {
- t.setStackTrace(deobfuscateStackTrace(t.getStackTrace(),
strongName));
+ private Throwable deobfuscateThrowable(Throwable old, String strongName)
{
+Throwable t = new Throwable(old.getMessage());
+if (old.getStackTrace() != null) {
+ t.setStackTrace(deobfuscateStackTrace(old.getStackTrace(),
strongName));
+} else {
+ t.setStackTrace(new StackTraceElement[0]);
}
-if (t.getCause() != null) {
- t.initCause(deobfuscateThrowable(t.getCause(), strongName));
+if (old.getCause() != null) {
+ t.initCause(deobfuscateThrowable(old.getCause(), strongName));
}
return t;
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
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
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
Thanks!
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');
On 2010/10/12 19:46:03, jgw wrote:
I think you need a declaration for 'base' and 'metaVal' here.
Will add it for metaVar (which is a local variable). The base variable
is actually defined in the global space for the old linker templates,
and since they still use this js file (and expect this code to set that
variable for them), it has to remain undeclared. I'll add a comment to
make sure noone fixes this in the future since it's pretty unintuitive.
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;
On 2010/10/12 19:46:03, jgw wrote:
When would you want to turn this off?
If you use the installScriptDirect.js rather than the
installScriptEarlyDownload (which adds the script tag with the md5 file
as the source on the iframe directly rather than adding it to the body
and then installing it on the iframe). This is actually how the addition
of devmode.js worked before this CL, and if you don't care about the
early download, or if you're a late loading module, you may want this
approach for simplicity.
Also - if you wanted to make your super optimized, then you could
set this to false, make your own iframe for the module, and request the
md5 file (which now includes the boobtstrap) from that iframe. This
would probably require some fixing of the $wnd variable which currently
refers to both the doc where the gwt code is working and to the doc
where the moduleFunction() is declared - I haven't tested this case
thoroughly yet, but if someone wanted it it wouldn't be hard to make it
work I think.
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;
On 2010/10/12 19:46:03, jgw wrote:
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.
Sure - seems reasonable - will do.
http://gwt-code-reviews.appspot.com/941802/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
http://gwt-code-reviews.appspot.com/941802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
http://gwt-code-reviews.appspot.com/941802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
http://gwt-code-reviews.appspot.com/941802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
http://gwt-code-reviews.appspot.com/941802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add Support for server side script selection in linker (issue941802)
http://gwt-code-reviews.appspot.com/941802/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add Support for server side script selection in linker (issue941802)
Reviewers: jgw, Description: Add Support for server side script selection in linker Please review this at http://gwt-code-reviews.appspot.com/941802/show Affected files: A dev/core/src/com/google/gwt/core/ext/linker/impl/PermutationsUtil.java A dev/core/src/com/google/gwt/core/ext/linker/impl/PropertiesMappingArtifact.java A dev/core/src/com/google/gwt/core/ext/linker/impl/ResourceInjectionUtil.java M dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java M dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installLocationIframe.js A dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptCommon.js A dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptDirect.js A dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js M dev/core/src/com/google/gwt/core/ext/linker/impl/permutations.js M dev/core/src/com/google/gwt/core/ext/linker/impl/processMetas.js M dev/core/src/com/google/gwt/core/ext/linker/impl/waitForBodyLoaded.js M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js M dev/core/src/com/google/gwt/core/linker/SingleScriptLinker.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: remove SeriaizableLogRecord and SerializableThrowable (issue875803)
http://gwt-code-reviews.appspot.com/875803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: remove SeriaizableLogRecord and SerializableThrowable (issue875803)
http://gwt-code-reviews.appspot.com/875803/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
