Sorry for the delay, I had interns start and have been busy with them.
:)
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
On 2012/06/09 09:07:18, tbroyer wrote:
That comment should probably be deleted.
Done.
http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/SubstitutionReplacer.java
File user/src/com/google/gwt/resources/css/SubstitutionReplacer.java
(right):
http://gwt-code-reviews.appspot.com/1735804/diff/1/user/src/com/google/gwt/resources/css/SubstitutionReplacer.java#newcode71
user/src/com/google/gwt/resources/css/SubstitutionReplacer.java:71: for
(ListIterator<Value> i = values.listIterator(); i.hasNext();) {
On 2012/06/12 23:34:37, skybrian wrote:
Some cleanup that's unrelated to your change (so you might not want to
do it),
but will make this code easier to understand: I think we shouldn't
modify the
list in place, but instead append all the values to a new list.
Something like
this:
List<Value> result = new ArrayList<Value>(original.size());
for (Value val : original) {
... append stuff to result ...
}
return new ListValue(result);
Done.
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());
On 2012/06/09 09:07:18, tbroyer wrote:
Shouldn't 'name' be passed through Generator.escape()?
(just in case, such as someone using f\u006Fo as the function name)
Done. Thanks for the catch.
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());
On 2012/06/09 09:07:18, tbroyer wrote:
Does that mean this test actually failed?
Yes, it failed before without the extra spaces. I've updated
ListValue.toCss() to check isSpaceRequired(), and the extra spaces
aren't needed now.
http://gwt-code-reviews.appspot.com/1735804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
