http://codereview.appspot.com/14117/diff/4001/3016 File java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/AbstractTagHandler.java (right):
http://codereview.appspot.com/14117/diff/4001/3016#newcode35 Line 35: public AbstractTagHandler(String namespaceUri, String tagName) { don't think these need to be properties - just make getTagName()/getNamespaceUri() abstract. (Doesn't make much of a difference as long as these are reused instances, but I find it a bit cleaner) http://codereview.appspot.com/14117/diff/4001/3016#newcode48 Line 48: protected <T extends Object> T getValueFromTag(Element tag, String name, Just <T>: <T extends Object> is redundant. http://codereview.appspot.com/14117/diff/4001/3011 File java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/Evaluator.java (right): http://codereview.appspot.com/14117/diff/4001/3011#newcode24 Line 24: public interface Evaluator { for implementing tags that have contents, this interface will need to handle more than just expression evaluation, so it'll soon need to have a different name and some more methods (for re-entering the processor) http://codereview.appspot.com/14117/diff/4001/3017 File java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/HtmlTagHandler.java (right): http://codereview.appspot.com/14117/diff/4001/3017#newcode55 Line 55: } catch (Exception e) { should catch the exact exception types, not Exception (otherwise, RuntimeExceptions get caught inappropriately) http://codereview.appspot.com/14117/diff/4001/3017#newcode57 Line 57: "Error: " + e.getMessage())); need to be very careful here - someone could include content "<script>alert('foo')</script>" - and if that shows up in the exception message, then that script will get executed. http://codereview.appspot.com/14117/diff/4001/3012 File java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TagRegistry.java (right): http://codereview.appspot.com/14117/diff/4001/3012#newcode78 Line 78: hash = (namespaceUri.hashCode() * 37) ^ localName.hashCode(); compute aggressively in the constructor (and store as an int, not an Integer) http://codereview.appspot.com/14117/diff/4001/3010 File java/gadgets/src/main/java/org/apache/shindig/gadgets/templates/TemplateModule.java (right): http://codereview.appspot.com/14117/diff/4001/3010#newcode49 Line 49: handlers.add(nameHandler); handlers = ImmutableSet.of(htmlHandler, nameHandler); http://codereview.appspot.com/14117
