http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safecss/shared/SafeCss.java
File user/src/com/google/gwt/safecss/shared/SafeCss.java (right):

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safecss/shared/SafeCss.java#newcode72
user/src/com/google/gwt/safecss/shared/SafeCss.java:72: * facility for
restricting deserialization on the Server only (thought this
though

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safecss/shared/SafeCssString.java
File user/src/com/google/gwt/safecss/shared/SafeCssString.java (right):

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safecss/shared/SafeCssString.java#newcode24
user/src/com/google/gwt/safecss/shared/SafeCssString.java:24: *
<p>

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
File
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java
(right):

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode186
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:186:
* context. In a non-text context, the string is not guarenteed to be
guaranteed

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode283
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:283:
switch (htmlContext.getType()) {
Are you sure the SafeCss fragments make sense in both contexts? And not
sure I saw test coverage of both paths.

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode287
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:287:
// safety via sub-formats that specify the in-css context.
drop the todo? Or update it?

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode298
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:298:
// TODO(xtof): Improve support for CSS.
ditto

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode354
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:354:
private void emitTextContextParameterExpression(HtmlContext htmlContext,
It's odd that you're passing the context in here and checking its type
again, rather than keeping all of the type-specific checks up on the
switch statement that calls this method.

http://gwt-code-reviews.appspot.com/1384801/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode368
user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:368:
throw new IllegalArgumentException(NON_TEXT_CONTEXT_ERROR);
Breaking change, yes? Seems like your users will see the warning, and
then get this exception. Also, throwing an IllegalArgumentException for
a user error is wrong.

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

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

Reply via email to