http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java File user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java (right):
http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java#newcode28 user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java:28: RENDERABLE_STAMPER(2), Should this have the same precedence as DOM_ID_HOLDER? http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/Composite.java File user/src/com/google/gwt/user/client/ui/Composite.java (right): http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/Composite.java#newcode111 user/src/com/google/gwt/user/client/ui/Composite.java:111: builder.append(TEMPLATE.renderWithId(stamper.getToken())); getToken() is deprecated, use: builder.append(stamper.stamp(TEMPLATE.render())); http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderablePanel.java File user/src/com/google/gwt/user/client/ui/RenderablePanel.java (right): http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderablePanel.java#newcode198 user/src/com/google/gwt/user/client/ui/RenderablePanel.java:198: String id = stamper.getToken(); Can you stamp the SafeHtml returned from the TEMPLATE instead of using the ID directly? Removing getToken() from our code will help confirm that its safe to hide the implementation. http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java File user/src/com/google/gwt/user/client/ui/RenderableStamper.java (right): http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java#newcode25 user/src/com/google/gwt/user/client/ui/RenderableStamper.java:25: * overly minimalistic javadoc http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java#newcode40 user/src/com/google/gwt/user/client/ui/RenderableStamper.java:40: public String getToken() { Add @Deprecated annotation, and @deprecated javadoc tag http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java#newcode48 user/src/com/google/gwt/user/client/ui/RenderableStamper.java:48: return Document.get().getElementById(token).cast(); The .cast() call will trigger an NPE if the token is not found. Can you add a check, and add a javadoc comment that describes the expected behavior if the element is not attached? Also, this would be a good place for a dev mode only runtime check to verify that the element is attached. A TODO would be fine for now. http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java#newcode58 user/src/com/google/gwt/user/client/ui/RenderableStamper.java:58: int endOfFirstTag = html.indexOf('>'); This doesn't handle all forms of SafeHtml. For example, a simple string "hello world" is considered safe html, as is a self closing tag "<div style='color:red;' />" http://gwt-code-reviews.appspot.com/1472801/diff/1/user/src/com/google/gwt/user/client/ui/RenderableStamper.java#newcode62 user/src/com/google/gwt/user/client/ui/RenderableStamper.java:62: .append(token) Are we sure that the token is always safe? We should escape in RenderableStamper's constructor, or at least add a comment explaining that we assume it is safe. http://gwt-code-reviews.appspot.com/1472801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
