http://codereview.appspot.com/120050/diff/1/27 File ../trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/DefaultHtmlSerializer.java (right):
http://codereview.appspot.com/120050/diff/1/27#newcode105 Line 105: private Element replaceElement(Element elem) { Some Javadoc (e.g., explaining why this exists). I'd call it "substituteElement", perhaps. http://codereview.appspot.com/120050/diff/1/27#newcode110 Line 110: replacement.setAttribute("type", scriptType); it looks as though the parsing code doesn't remove the "type" attribute. http://codereview.appspot.com/120050/diff/1/7 File ../trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/GadgetHtmlParser.java (right): http://codereview.appspot.com/120050/diff/1/7#newcode54 Line 54: // Bi-map of OpenSocial tags to their script type attribute values. Should be Javadoc, not // http://codereview.appspot.com/120050/diff/1/18 File ../trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParser.java (right): http://codereview.appspot.com/120050/diff/1/18#newcode68 Line 68: public NekoSimplifiedHtmlParser(DOMImplementation documentFactory) { indent here through line 166 looks off by two http://codereview.appspot.com/120050/diff/1/18#newcode113 Line 113: HTMLScanner htmlScanner = new HTMLScanner(); some comments showing the order of filters would be helpful http://codereview.appspot.com/120050/diff/1/18#newcode119 Line 119: if (config.getFeature("http://xml.org/sax/features/namespaces";)) { This feature should always be on, so you could lose the else block. http://codereview.appspot.com/120050/diff/1/18#newcode164 Line 164: protected boolean isElementImportant(QName qName) { method could be killed, ditto all references to the "false" cases (e.g. startUnimportantElement()) http://codereview.appspot.com/120050/diff/1/18#newcode173 Line 173: protected class OSMLDocumentFilter extends DefaultFilter { static? http://codereview.appspot.com/120050/diff/1/18#newcode490 Line 490: protected final void appendChild(Node node) { Could be private now - was only exposed for SocialMarkupHtmlParser http://codereview.appspot.com/120050/diff/1/31 File ../trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/TemplateRewriter.java (right): http://codereview.appspot.com/120050/diff/1/31#newcode152 Line 152: DomUtil.getElementsByTagNameCaseInsensitive(content.getDocument(), why not document.getElementsByTagName, as in the pipeline code? http://codereview.appspot.com/120050/diff/1/11 File ../trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/nekohtml/NekoSimplifiedHtmlParserTest.java (right): http://codereview.appspot.com/120050/diff/1/11#newcode37 Line 37: * Test for the social markup parser. comment a bit out-of-date http://codereview.appspot.com/120050
