LGTM (provided you address the comments below)

http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/GenerateCssAst.java
File user/src/com/google/gwt/resources/css/GenerateCssAst.java (right):

http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/GenerateCssAst.java#newcode1056
user/src/com/google/gwt/resources/css/GenerateCssAst.java:1056: // Just
return a String representation of the original value
That comment should probably be deleted.

http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/ast/CssProperty.java
File user/src/com/google/gwt/resources/css/ast/CssProperty.java (right):

http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/ast/CssProperty.java#newcode148
user/src/com/google/gwt/resources/css/ast/CssProperty.java:148: name,
values.getExpression());
Shouldn't 'name' be passed through Generator.escape()?
(just in case, such as someone using f\u006Fo as the function name)

http://gwt-code-reviews.appspot.com/1735804/diff/1/user/test/com/google/gwt/resources/client/CSSResourceTest.java
File user/test/com/google/gwt/resources/client/CSSResourceTest.java
(right):

http://gwt-code-reviews.appspot.com/1735804/diff/1/user/test/com/google/gwt/resources/client/CSSResourceTest.java#newcode378
user/test/com/google/gwt/resources/client/CSSResourceTest.java:378:
assertEquals("1px solid rgba(0 , 0 , 0 , 0.2)",
defines.multiValueBorderDef());
Does that mean this test actually failed?

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

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

Reply via email to