http://gwt-code-reviews.appspot.com/1447812/diff/1/tools/api-checker/config/gwt23_24userApi.conf File tools/api-checker/config/gwt23_24userApi.conf (right):
http://gwt-code-reviews.appspot.com/1447812/diff/1/tools/api-checker/config/gwt23_24userApi.conf#newcode155 tools/api-checker/config/gwt23_24userApi.conf:155: com.google.gwt.user.client.ui.impl.ClippedImageImpl::adjust(Lcom/google/gwt/dom/client/Element;Ljava/lang/String;IIII) MISSING Instead of all of these exceptions for ClippedImageImpl, you can exclude com.google.gwt.user.client.ui.impl from api checks. We don't make any guarantees about the API of impl classes. http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/resources/client/DataResource.java File user/src/com/google/gwt/resources/client/DataResource.java (right): http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/resources/client/DataResource.java#newcode66 user/src/com/google/gwt/resources/client/DataResource.java:66: * will be an absolute URL. Should this be deprecated like ImageResource? http://gwt-code-reviews.appspot.com/1447812/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/1447812/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode305 user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:305: // not a fatal error. Why isn't this a fatal error? Using a SafeUri in the middle of a URL attribute seems just as bad as the above case, and using it outside of the URL context seems like a dev mistake. http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java#newcode432 user/src/com/google/gwt/safehtml/rebind/SafeHtmlTemplatesImplMethodCreator.java:432: if (isSafeUri(parameterType)) { Is it safe to use safeUri in a text context? Seems like a mistake at the least. http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java File user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java (right): http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java#newcode32 user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java:32: public class SafeUriHostedModeUtils { Should this class be package protected? It looks like an impl class, but its public in a non-impl package, which makes it look like anyone can use it. http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java#newcode39 user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java:39: * >RFC 3987bis Web Addresses</a> Will this look correct in javadoc? You might need to exceed the 100 line limit here. http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java#newcode54 user/src/com/google/gwt/safehtml/shared/SafeUriHostedModeUtils.java:54: public static final String FORCE_CHECK_VALID_URI = "com.google.gwt.safehtml.ForceCheckValidUri"; Can you add a comment explaining how this is set? http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/safehtml/shared/UriUtils.java File user/src/com/google/gwt/safehtml/shared/UriUtils.java (right): http://gwt-code-reviews.appspot.com/1447812/diff/1/user/src/com/google/gwt/safehtml/shared/UriUtils.java#newcode209 user/src/com/google/gwt/safehtml/shared/UriUtils.java:209: * safe!</strong> This method worries me. When I saw the name, I assumed it was the equivalent of fromString(). Anyone who looks at the method name without reading the JavaDoc might assume the same. I suggest we remove the method and let users manage unsafe URIs. That forces the user to make the tough decisions, whether they sanitize the URI, or call fromTrustedString() even if the URI isn't trusted. http://gwt-code-reviews.appspot.com/1447812/diff/1/user/test/com/google/gwt/user/client/ui/impl/ClippedImagePrototypeTest.java File user/test/com/google/gwt/user/client/ui/impl/ClippedImagePrototypeTest.java (left): http://gwt-code-reviews.appspot.com/1447812/diff/1/user/test/com/google/gwt/user/client/ui/impl/ClippedImagePrototypeTest.java#oldcode42 user/test/com/google/gwt/user/client/ui/impl/ClippedImagePrototypeTest.java:42: private static class TestLoadListener implements LoadListener { Don't remove this. Its actually used in a commented out test. In theory the issue has been fixed, so I'll take a look at re-enabling the test. http://gwt-code-reviews.appspot.com/1447812/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
