http://gwt-code-reviews.appspot.com/1454808/diff/4001/user/src/com/google/gwt/safecss/shared/SafeStylesBuilder.java File user/src/com/google/gwt/safecss/shared/SafeStylesBuilder.java (right):
http://gwt-code-reviews.appspot.com/1454808/diff/4001/user/src/com/google/gwt/safecss/shared/SafeStylesBuilder.java#newcode166 user/src/com/google/gwt/safecss/shared/SafeStylesBuilder.java:166: public SafeStylesBuilder floatprop(Float value) { Oh well. I can't think of anything better. http://gwt-code-reviews.appspot.com/1454808/diff/4001/user/src/com/google/gwt/safecss/shared/SafeStylesHostedModeUtils.java File user/src/com/google/gwt/safecss/shared/SafeStylesHostedModeUtils.java (right): http://gwt-code-reviews.appspot.com/1454808/diff/4001/user/src/com/google/gwt/safecss/shared/SafeStylesHostedModeUtils.java#newcode214 user/src/com/google/gwt/safecss/shared/SafeStylesHostedModeUtils.java:214: } If we exit the loop with ignoreNext = true, I think that's an error and perhaps a security issue. (There should not be a backslash at the end of the string, because it would escape whatever comes next outside the string.) http://gwt-code-reviews.appspot.com/1454808/diff/4001/user/test/com/google/gwt/safecss/shared/GwtSafeStylesHostedModeUtilsTest.java File user/test/com/google/gwt/safecss/shared/GwtSafeStylesHostedModeUtilsTest.java (right): http://gwt-code-reviews.appspot.com/1454808/diff/4001/user/test/com/google/gwt/safecss/shared/GwtSafeStylesHostedModeUtilsTest.java#newcode82 user/test/com/google/gwt/safecss/shared/GwtSafeStylesHostedModeUtilsTest.java:82: fail("Expected an exception for invalid style name."); This doesn't look right. If fail() is called, it will throw AssertionError, which gets silently ignored. If we're actually expecting a different AssertionError, it would be safer to set a boolean and check it outside the try block. (Similarly in the next test.) http://gwt-code-reviews.appspot.com/1454808/diff/4001/user/test/com/google/gwt/safecss/shared/GwtSafeStylesHostedModeUtilsTest.java#newcode83 user/test/com/google/gwt/safecss/shared/GwtSafeStylesHostedModeUtilsTest.java:83: } catch (IllegalArgumentException e) { Which exception do we actually expect? Should we check the message string for a reasonable error? http://gwt-code-reviews.appspot.com/1454808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
