LGTM!
http://gwt-code-reviews.appspot.com/1384801/diff/8001/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/8001/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode315 user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:315: logger.log(TreeLogger.WARN, On 2011/04/07 15:45:57, jlabanca wrote:
I added a comment explaining that we've already checked that the user
did not
try to use SafeStyles in a non-CSS context.
The reason for the split checks is that each Safe type is only valid
in one
context, so it makes sense to check that separately. Otherwise, we
would have
to have a !SafeHtml/!SafeStyles check in every context where those
classes
aren't supported.
Conversely, some contexts have or will have a Safe value that is
appropriate for
that context. Adding the warning in the switch statement allows us to
do check
if the user could have used a Safe class specific to the current
context. If we
move the warnings out of the switch statement, then we would have to
check the
current context to see if the user could have chosen a better Safe
value, which
would basically add a duplicate (subset) switch statement.
I see... Makes sense and SGTM. http://gwt-code-reviews.appspot.com/1384801/diff/8025/user/src/com/google/gwt/safecss/shared/SafeStyles.java File user/src/com/google/gwt/safecss/shared/SafeStyles.java (right): http://gwt-code-reviews.appspot.com/1384801/diff/8025/user/src/com/google/gwt/safecss/shared/SafeStyles.java#newcode75 user/src/com/google/gwt/safecss/shared/SafeStyles.java:75: * <li>width: 1em;</li> Should these be {@code}? http://gwt-code-reviews.appspot.com/1384801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
