On Fri, Jun 18, 2010 at 10:30 AM, <[email protected]> wrote: > LGTM > > > http://codereview.appspot.com/1714043/diff/1/5 > File > > > java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaHtmlParser.java > (right): > > http://codereview.appspot.com/1714043/diff/1/5#newcode144 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaHtmlParser.java:144: > DomParser parser = new DomParser(lexer, true, is, ns, mq); > Please add a comment around the param labeling it as /* wantsComments */ >
Will do before commit. > > http://codereview.appspot.com/1714043/diff/1/2 > File > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/AbstractParsingTestBase.java > (right): > > http://codereview.appspot.com/1714043/diff/1/2#newcode1 > > java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/AbstractParsingTestBase.java:1: > /* > Debugging cruft. > removed. > > http://codereview.appspot.com/1714043/diff/1/3 > File > > > java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/caja/CajaSocialMarkupHtmlParserTest.java > (right): > > http://codereview.appspot.com/1714043/diff/1/3#newcode37 > > java/gadgets/src/test/java/org/apache/shindig/gadgets/parse/caja/CajaSocialMarkupHtmlParserTest.java:37: > public void testInvalid() throws Exception { super.testInvalid(); } > valign:"middle" is being treated as an malformed identifier hence the > downgrade to warning. Is there a reason to treated invalid characters > as a continue-is-not-possible error? That's what I wanted to ask you :) I understand the rationale and actually appreciate that the parser keeps trying to chug along. Even so, in a case like this if the intention of the input HTML was to provide an identifier used by subsequent code (JS et al), and we just sort of dropped it, that would seem a little opaque to me. Thoughts? Should we perhaps emit warnings into the code somehow in a comment? > > > http://codereview.appspot.com/1714043/show >
