Nearly there, just a couple of nits. Thanks! After this patch lands we're going to need to think harder about the testing. The coverage introduced here is pretty light. E.g., we're not reproducing any of the escaping checks (like the various "test*FunnyChars" tests in UiBinderTest. I don't have an immediate idea, but we should certainly avoid mass copy and paste of the tests and the appropriate template fragments.
Could you post a sample of the generated code? http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java File user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java (right): http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java#newcode123 user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java:123: public void resetWritten() { uncalled, right? http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java (right): http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode235 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:235: private boolean isRenderer; Should be final http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode861 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:861: die(baseClass.getName() + " must implement UiBinder or SafeHtmlRenderer"); Are we making a comparable check for implements SafeHtmlRenderer? http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1464 user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1464: // TODO(sbrubaker): Fix method signature Is this TODO still valid? I've lost track of what it refers to. If it is, can you make it a bit more informative? http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java File user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java (right): http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java#newcode80 user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java:80: "fieldName.setHTML(\"@mockToken-fieldName-Hello, I <b>caption</b>you.\");", is the "fieldName" string coming from a constant in ElementParserTester or something? If so, can you open up its visibility and use it in these tests, rather having it inlined all over the place? http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java File user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java (right): http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java#newcode73 user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java:73: public void testSafeHtmlRendererText() { Doesn't need to block this patch, but as a follow up could you break this out into its own test class, perhaps SafeHtmlRendererTest? SafeHtmlAsComponentsTest should serve as an example. http://gwt-code-reviews.appspot.com/1442804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
