All done, thanks. Committed at r7354.
On 2009/12/27 03:29:04, Dan Rice wrote: > LGTM with minor nits. > http://gwt-code-reviews.appspot.com/128801/diff/1/3 > File user/src/com/google/gwt/user/client/Cookies.java (right): > http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode112 > Line 112: if (uriEncoding) { > Prettier and less duplicated code to do: > if (uriEncoding) { > name = uriEncode(name); > } > removeCookieNative(name); > http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode115 > Line 115: else { > } else { > on same line > http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode128 > Line 128: if (uriEncoding) { > Ditto > http://gwt-code-reviews.appspot.com/128801/diff/1/3#newcode184 > Line 184: throw new IllegalArgumentException("Illegal cookie format."); > Include name and value in exception message, maybe distinguish between bad name > and bad value to make debugging easier. > http://gwt-code-reviews.appspot.com/128801/diff/1/2 > File user/test/com/google/gwt/user/client/CookieTest.java (right): > http://gwt-code-reviews.appspot.com/128801/diff/1/2#newcode166 > Line 166: // Make sure cookie values are URI encoded > Probably best to add 'Cookies.setUriEncode(true);' here (redundantly) to avoid > dependency between sections of this method. http://gwt-code-reviews.appspot.com/128801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
