Updated as per comments
http://codereview.appspot.com/14041/diff/1/2 File java/gadgets/src/main/java/org/apache/shindig/gadgets/parse/caja/CajaCssSanitizer.java (right): http://codereview.appspot.com/14041/diff/1/2#newcode41 Line 41: public class CajaCssSanitizer { On 2009/02/03 07:33:58, etnu00 wrote:
I'm a little concerned about tying this too closely with Caja. This
effectively
forces a caja dependency even on someone who doesn't want to allow
CSS. It
should be possible to extract a CssSanitizer interface from this.
Not really without having an implementation agnostic CSS DOM interface which I dont think makes sense at this point. Nor would it be useful without a viable alternative CSS parser. Ill track this in JIRA http://codereview.appspot.com/14041/diff/1/2#newcode85 Line 85: + ((CssTree.Property) ancestorChain.node).getPropertyName()); On 2009/02/03 07:33:58, etnu00 wrote:
INFO is probably too high a warning level here. FINE or lower seems to
make more
sense. Otherwise this is just log spam for most gadgets.
Done. http://codereview.appspot.com/14041/diff/1/10 File java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/CssRewriter.java (right): http://codereview.appspot.com/14041/diff/1/10#newcode45 Line 45: public class CssRewriter { On 2009/02/03 07:33:58, etnu00 wrote:
The relationship between CssRewriter and CSSContentRewriter isn't
terribly clear
(other than CSSContentRewriter implementing the ContentRewriter
interface). The
naming is also inconsistent. Now seems like a good time to clean that
up. Indeed, done. Also removed all lexer based rewriter artifacts. http://codereview.appspot.com/14041/diff/1/10#newcode53 Line 53: private static final Pattern urlMatcher = On 2009/02/03 07:33:58, etnu00 wrote:
Is this necessary any longer?
Nope. gone via refactor http://codereview.appspot.com/14041
