LGTM with a couple of minor things left.
http://gwt-code-reviews.appspot.com/771801/diff/11002/15007 File user/src/com/google/gwt/safehtml/shared/OnlyToBeUsedInGeneratedCodeStringBlessedAsSafeHtml.java (right): http://gwt-code-reviews.appspot.com/771801/diff/11002/15007#newcode26 user/src/com/google/gwt/safehtml/shared/OnlyToBeUsedInGeneratedCodeStringBlessedAsSafeHtml.java:26: public class OnlyToBeUsedInGeneratedCodeStringBlessedAsSafeHtml Can this just be a subclass of SafeHtmlString? http://gwt-code-reviews.appspot.com/771801/diff/11002/15011 File user/src/com/google/gwt/safehtml/shared/SafeHtmlUtils.java (right): http://gwt-code-reviews.appspot.com/771801/diff/11002/15011#newcode79 user/src/com/google/gwt/safehtml/shared/SafeHtmlUtils.java:79: * Test 1, complex escaping, 22/391 chars needed to be replaced. On 2010/08/20 18:37:52, rice wrote:
Probably don't need to include timings in the source permanently.
I agree, especially since they don't include machine details that would be relevant. I suggest just state the options you tested and the conclusion rather than including the timings here. http://gwt-code-reviews.appspot.com/771801/diff/11002/15013 File user/src/com/google/gwt/safehtml/shared/UriUtils.java (right): http://gwt-code-reviews.appspot.com/771801/diff/11002/15013#newcode66 user/src/com/google/gwt/safehtml/shared/UriUtils.java:66: return (scheme == null || "http".equalsIgnoreCase(scheme) Rather than equalsIgnoreCase, which is going to wind up converting to lowercase multiple times, why not convert it to lowercase once and just do equals? We don't have fully correct casing in the browser (and to do it right you would need java.text.Collator anyway), and those are implemented the same way just more efficiently. Also, I was incorrect when we talked before about the Turkish locale -- toLowerCase("I") will return the dotless i (U+0131). As we discussed, toUpperCase won't solve it either, as that will map "i" to a dotted capital i (U+130). So an ugly hack would be to also compare to "ma\u0131lto" (or just type it in the source) as well as the others after calling toLowerCase(). Not sure we can do better on the client given the state of Unicode support in JS. http://gwt-code-reviews.appspot.com/771801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
