Upgrade Caja to r4135

This change pulls in Caja updates that are relevant to Shindig in two ways:

1. DefaultGadgetRewriter class API has changed from accepting a PluginEnvironment to the pair of UriFetcher and UriPolicy.
  - Previous implementation modified to adapt to the new interfaces.

2. Several parser improvements now enable nearly all HTML parsing tests to succeed, esp. social data-related tests.
  - Caja's parser now compatible w/ os templates and data pipelining!
  - Only remaining test fails since Caja's parser eats a DOMException and turns it into a Warning. Will talk w/ Caja to assess best approach.

[Jasvir, 2010-06-18 17:04:46]

heh.  I *just* sent essentially the same upgrade your way at
http://codereview.appspot.com/1460041/show

I notice however that the introduction of UriManager at r951107 breaks cajoled
gadgets but this is not caught by any of the existing tests.

[johnfargo, 2010-06-18 17:11:38]

Great minds ;)

How does the UriManager break cajoled gadgets? Did I miss some caja=1 param
logic or something?

[johnfargo, 2010-06-18 17:13:41]

Get rid of unnecessary AbstractParsingTestBase edit.

[Jasvir, 2010-06-18 17:30:16]

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 */

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.

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?

[fargo, 2010-06-18 17:41:00]

On Fri, Jun 18, 2010 at 10:30 AM, <jasvir@gmail.com> 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
>