On Fri, Mar 5, 2010 at 5:41 PM, <[email protected]> wrote: > > http://codereview.appspot.com/224097/diff/2001/2003 > File > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatLinkRewriter.java > (right): > > http://codereview.appspot.com/224097/diff/2001/2003#newcode122 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatLinkRewriter.java:122: > ConcatUriManager.ConcatUri.fromList(gadget, uriBatches, type), !split); > It would be nice to actually figure out if split JS is needed. Meaning > if split js support is enabled, but there is really just 2 scripts that > are sibling, split js is not needed. >
I agree -- for simplicity, I'm starting with single-mode though, to add conditional-split later. > > http://codereview.appspot.com/224097/diff/2001/2003#newcode166 > > java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/ConcatLinkRewriter.java:166: > elem.getAttribute("type").contains("css")); > Check for null from elem.getAttribute > Per JavaDoc, Element.getAttribute() returns the empty string for missing attribs. > > http://codereview.appspot.com/224097/diff/2001/2005 > File > > java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ConcatLinkRewriterTest.java > (right): > > http://codereview.appspot.com/224097/diff/2001/2005#newcode88 > > java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/ConcatLinkRewriterTest.java:88: > css4 = elem("link", "rel", "stylesheet", "type", "text/css", "href", > CSS4_URL_STR); > Test also bad nodes (btw - where is elem defined?) A few tests added -- elem is defined in the test base class as a DOM helper. > > > http://codereview.appspot.com/224097/show >
