Remove Cajita/Valija flag as an input to all cajoling services

Remove Cajita/Valija flag as an input to all cajoling services. Rely
only on the source itself to tell us whether it is Valija (the
default) or Cajita (via a "use cajita" pragma). Cajole all files to a
consistent output filename (*.out.js).

These changes allow the (Cajita-level) module loader to predictably
load a module regardless of whether it is Valija or Cajita. Because we
build our Valija module loader on top of Cajita, the Cajita loader did
not have the information of whether a module to be loaded is Cajita or
Valija; as far as it was concerned, everything is Cajita. Which is as
it should be -- Valija is just Cajita with a funky calling convention.

This fixes http://code.google.com/p/google-caja/issues/detail?id=1111.

[MikeSamuel, 2009-10-22 00:50:28]

http://codereview.appspot.com/136053/diff/1/19
File tests/com/google/caja/demos/applet/CajaAppletTest.java (left):

http://codereview.appspot.com/136053/diff/1/19#oldcode62
Line 62: TestUtil.readResource(getClass(), "caja-applet-cajita-golden.js"),
Rather than throwing out a useful test, can you run it twice, once with "use
cajita" at the top.

http://codereview.appspot.com/136053/diff/1/20
File tests/com/google/caja/demos/benchmarks/BenchmarkRunner.java (right):

http://codereview.appspot.com/136053/diff/1/20#newcode185
Line 185: + "  'require cajita';"
What is 'require strict' supposed to do?

http://codereview.appspot.com/136053/diff/1/21
File tests/com/google/caja/demos/benchmarks/BenchmarkSize.java (right):

http://codereview.appspot.com/136053/diff/1/21#newcode200
Line 200: + "  'require cajita';"
ditto

http://codereview.appspot.com/136053/diff/1/6
File tests/com/google/caja/opensocial/DefaultGadgetRewriterTest.java (right):

http://codereview.appspot.com/136053/diff/1/6#newcode102
Line 102: assertRewritePasses("SocialHelloWorld.xml", MessageLevel.WARNING);
So this no longer tests it in Cajita mode?

http://codereview.appspot.com/136053/diff/1/12
File tests/com/google/caja/plugin/domita_test.html (right):

http://codereview.appspot.com/136053/diff/1/12#newcode211
Line 211: loadScript('valija.co.js');
Why are we loading valija.co.js even when it's not valija?

http://codereview.appspot.com/136053/diff/1/16
File tests/com/google/caja/plugin/domita_test_untrusted.html (right):

http://codereview.appspot.com/136053/diff/1/16#newcode463
Line 463: /*@@cajaLanguageSelectionStatements@@*/
Ok.  Maybe a pointer to the code that does the substitution.

http://codereview.appspot.com/136053/diff/1/15
File tests/com/google/caja/plugin/jsunit.js (right):

http://codereview.appspot.com/136053/diff/1/15#newcode223
Line 223: }
cool

[DavidSarah, 2009-10-22 01:48:51]

http://codereview.appspot.com/136053/diff/1/38
File build.xml (right):

http://codereview.appspot.com/136053/diff/1/38#newcode777
Line 777: language="cajita"/>
language="caja"

http://codereview.appspot.com/136053/diff/1/38#newcode783
Line 783: <output file="${www}/lolcat-search/kittens.co.html"
language="cajita"/>
language="caja"

http://codereview.appspot.com/136053/diff/1/38#newcode840
Line 840: language="cajita"/>
language="caja"

http://codereview.appspot.com/136053/diff/1/38#newcode846
Line 846: <output file="${www}/slides/kittens.co.html" language="cajita"/>
language="caja"

http://codereview.appspot.com/136053/diff/1/23
File src/com/google/caja/commonjs-sandbox.js (right):

http://codereview.appspot.com/136053/diff/1/23#newcode14
Line 14: 
"use strict";

http://codereview.appspot.com/136053/diff/1/26
File src/com/google/caja/plugin/PluginMeta.java (right):

http://codereview.appspot.com/136053/diff/1/26#newcode28
Line 28: private boolean valijaMode;
now unused; delete this field

http://codereview.appspot.com/136053/diff/1/37
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/136053/diff/1/37#newcode56
Line 56: 
"use strict";

http://codereview.appspot.com/136053/diff/1/5
File tests/com/google/caja/a.js (right):

http://codereview.appspot.com/136053/diff/1/5#newcode14
Line 14: 
"use strict";

http://codereview.appspot.com/136053/diff/1/11
File tests/com/google/caja/c.js (right):

http://codereview.appspot.com/136053/diff/1/11#newcode14
Line 14: 
"use strict";

http://codereview.appspot.com/136053/diff/1/19
File tests/com/google/caja/demos/applet/CajaAppletTest.java (left):

http://codereview.appspot.com/136053/diff/1/19#oldcode62
Line 62: TestUtil.readResource(getClass(), "caja-applet-cajita-golden.js"),
On 2009/10/22 00:50:28, MikeSamuel wrote:
> Rather than throwing out a useful test, can you run it twice, once with "use
> cajita" at the top.

Or once with "use strict"; "use cajita"; at the top.

http://codereview.appspot.com/136053/diff/1/20
File tests/com/google/caja/demos/benchmarks/BenchmarkRunner.java (right):

http://codereview.appspot.com/136053/diff/1/20#newcode185
Line 185: + "  'require cajita';"
On 2009/10/22 00:50:28, MikeSamuel wrote:
> What is 'require strict' supposed to do?

This should be 'use strict'; 'use cajita';

http://codereview.appspot.com/136053/diff/1/21
File tests/com/google/caja/demos/benchmarks/BenchmarkSize.java (right):

http://codereview.appspot.com/136053/diff/1/21#newcode200
Line 200: + "  'require cajita';"
On 2009/10/22 00:50:28, MikeSamuel wrote:
> ditto

should be 'use strict'; 'use cajita';

http://codereview.appspot.com/136053/diff/1/3
File tests/com/google/caja/foo/b.js (right):

http://codereview.appspot.com/136053/diff/1/3#newcode14
Line 14: 
"use strict";

http://codereview.appspot.com/136053/diff/1/2
File tests/com/google/caja/foo/f.js (right):

http://codereview.appspot.com/136053/diff/1/2#newcode14
Line 14: 
"use strict";

http://codereview.appspot.com/136053/diff/1/10
File tests/com/google/caja/parser/quasiliteral/c.js (right):

http://codereview.appspot.com/136053/diff/1/10#newcode14
Line 14: 
"use strict";

http://codereview.appspot.com/136053/diff/1/9
File tests/com/google/caja/parser/quasiliteral/foo/b.js (right):

http://codereview.appspot.com/136053/diff/1/9#newcode14
Line 14: 
"use strict";

http://codereview.appspot.com/136053/diff/1/8
File tests/com/google/caja/parser/quasiliteral/foo/testPrimordials.js (right):

http://codereview.appspot.com/136053/diff/1/8#newcode14
Line 14: 
"use strict";

http://codereview.appspot.com/136053/diff/1/17
File tests/com/google/caja/plugin/ExpressionSanitizerTest.java (right):

http://codereview.appspot.com/136053/diff/1/17#newcode51
Line 51: "  'use cajita';"
"  'use strict'; 'use cajita';"

http://codereview.appspot.com/136053/diff/1/13
File tests/com/google/caja/plugin/HtmlCompiledPluginTest.java (right):

http://codereview.appspot.com/136053/diff/1/13#newcode133
Line 133: + "  'use strict, cajita';"
Bad syntax; see issue #1111. Should be 'use strict'; 'use cajita';

http://codereview.appspot.com/136053/diff/1/13#newcode145
Line 145: + "        (function () { 'use strict,cajita'; return {}.hello; })(),"
ditto

http://codereview.appspot.com/136053/diff/1/13#newcode146
Line 146: + "        (function f() { 'use strict,cajita'; return {}.hello; })()
]"
ditto

http://codereview.appspot.com/136053/diff/1/13#newcode152
Line 152: + "    function f() { 'use strict,cajita'; return {}.hello; }"
ditto

http://codereview.appspot.com/136053/diff/1/13#newcode168
Line 168: "  'use strict, cajita';\n" +
ditto

http://codereview.appspot.com/136053/diff/1/13#newcode178
Line 178: "<script>'use strict, cajita';throw new Error('panic');a=1;</script>"
+
ditto

http://codereview.appspot.com/136053/diff/1/13#newcode180
Line 180: "  'use strict, cajita';\n" +
ditto

http://codereview.appspot.com/136053/diff/1/13#newcode184
Line 184: "  'use strict, cajita';\n" +
ditto

http://codereview.appspot.com/136053/diff/1/14
File tests/com/google/caja/plugin/stages/DebuggingSymbolsStageTest.java (right):

http://codereview.appspot.com/136053/diff/1/14#newcode37
Line 37: "'use strict, cajita';\n"
"'use strict'; 'use cajita';\n"

http://codereview.appspot.com/136053/diff/1/14#newcode47
Line 47: "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode65
Line 65: "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode84
Line 84: "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode89
Line 89: "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode97
Line 97: "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode111
Line 111: "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode119
Line 119: "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode128
Line 128: + "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode138
Line 138: + "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode149
Line 149: + "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode169
Line 169: + "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode184
Line 184: + "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode197
Line 197: "'use strict, cajita';\n"
ditto

http://codereview.appspot.com/136053/diff/1/14#newcode283
Line 283: "  'use strict, cajita';"
"'use strict'; 'use cajita';"

http://codereview.appspot.com/136053/diff/1/4
File tests/com/google/caja/recursion.js (right):

http://codereview.appspot.com/136053/diff/1/4#newcode14
Line 14: 
"use strict";

[ihab.awad, 2009-10-30 06:17:42]

Snapshotted.

http://codereview.appspot.com/136053/diff/1/38
File build.xml (right):

http://codereview.appspot.com/136053/diff/1/38#newcode777
Line 777: language="cajita"/>
On 2009/10/22 01:48:51, DavidSarah wrote:
> language="caja"

Done.

http://codereview.appspot.com/136053/diff/1/38#newcode783
Line 783: <output file="${www}/lolcat-search/kittens.co.html"
language="cajita"/>
On 2009/10/22 01:48:51, DavidSarah wrote:
> language="caja"

Done.

http://codereview.appspot.com/136053/diff/1/38#newcode840
Line 840: language="cajita"/>
On 2009/10/22 01:48:51, DavidSarah wrote:
> language="caja"

Done.

http://codereview.appspot.com/136053/diff/1/38#newcode846
Line 846: <output file="${www}/slides/kittens.co.html" language="cajita"/>
On 2009/10/22 01:48:51, DavidSarah wrote:
> language="caja"

Done.

http://codereview.appspot.com/136053/diff/1/23
File src/com/google/caja/commonjs-sandbox.js (right):

http://codereview.appspot.com/136053/diff/1/23#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/26
File src/com/google/caja/plugin/PluginMeta.java (right):

http://codereview.appspot.com/136053/diff/1/26#newcode28
Line 28: private boolean valijaMode;
On 2009/10/22 01:48:51, DavidSarah wrote:
> now unused; delete this field

Done.

http://codereview.appspot.com/136053/diff/1/37
File src/com/google/caja/valija-cajita.js (right):

http://codereview.appspot.com/136053/diff/1/37#newcode56
Line 56: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/5
File tests/com/google/caja/a.js (right):

http://codereview.appspot.com/136053/diff/1/5#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/11
File tests/com/google/caja/c.js (right):

http://codereview.appspot.com/136053/diff/1/11#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/19
File tests/com/google/caja/demos/applet/CajaAppletTest.java (left):

http://codereview.appspot.com/136053/diff/1/19#oldcode62
Line 62: TestUtil.readResource(getClass(), "caja-applet-cajita-golden.js"),
On 2009/10/22 01:48:51, DavidSarah wrote:
> On 2009/10/22 00:50:28, MikeSamuel wrote:
> > Rather than throwing out a useful test, can you run it twice, once with "use
> > cajita" at the top.
> 
> Or once with "use strict"; "use cajita"; at the top.

See reply to Mike.

http://codereview.appspot.com/136053/diff/1/19#oldcode62
Line 62: TestUtil.readResource(getClass(), "caja-applet-cajita-golden.js"),
On 2009/10/22 00:50:28, MikeSamuel wrote:
> Rather than throwing out a useful test, can you run it twice, once with "use
> cajita" at the top.

Per our conversation: the applet is now no longer passing through any
"cajita/valija" signal; it just calls the generic cajoler. So it's just adequate
to test the valija case, knowing that the Valija rewriter will fall back to
Cajita if it sees the proper use subset directives.

http://codereview.appspot.com/136053/diff/1/20
File tests/com/google/caja/demos/benchmarks/BenchmarkRunner.java (right):

http://codereview.appspot.com/136053/diff/1/20#newcode185
Line 185: + "  'require cajita';"
On 2009/10/22 00:50:28, MikeSamuel wrote:
> What is 'require strict' supposed to do?

And I claim not to be a Perl programmer. :)

http://codereview.appspot.com/136053/diff/1/20#newcode185
Line 185: + "  'require cajita';"
On 2009/10/22 01:48:51, DavidSarah wrote:
> On 2009/10/22 00:50:28, MikeSamuel wrote:
> > What is 'require strict' supposed to do?
> 
> This should be 'use strict'; 'use cajita';

Done.

http://codereview.appspot.com/136053/diff/1/21
File tests/com/google/caja/demos/benchmarks/BenchmarkSize.java (right):

http://codereview.appspot.com/136053/diff/1/21#newcode200
Line 200: + "  'require cajita';"
On 2009/10/22 01:48:51, DavidSarah wrote:
> On 2009/10/22 00:50:28, MikeSamuel wrote:
> > ditto
> 
> should be 'use strict'; 'use cajita';

Done.

http://codereview.appspot.com/136053/diff/1/21#newcode200
Line 200: + "  'require cajita';"
On 2009/10/22 00:50:28, MikeSamuel wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/3
File tests/com/google/caja/foo/b.js (right):

http://codereview.appspot.com/136053/diff/1/3#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/2
File tests/com/google/caja/foo/f.js (right):

http://codereview.appspot.com/136053/diff/1/2#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/6
File tests/com/google/caja/opensocial/DefaultGadgetRewriterTest.java (right):

http://codereview.appspot.com/136053/diff/1/6#newcode102
Line 102: assertRewritePasses("SocialHelloWorld.xml", MessageLevel.WARNING);
On 2009/10/22 00:50:28, MikeSamuel wrote:
> So this no longer tests it in Cajita mode?

Correct, based on the premise that gadgets are HTML, and HTML is henceforth
always Valija at the top level. (Specific <script>s or functions can, of course,
opt into Cajita.)

http://codereview.appspot.com/136053/diff/1/10
File tests/com/google/caja/parser/quasiliteral/c.js (right):

http://codereview.appspot.com/136053/diff/1/10#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/9
File tests/com/google/caja/parser/quasiliteral/foo/b.js (right):

http://codereview.appspot.com/136053/diff/1/9#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/8
File tests/com/google/caja/parser/quasiliteral/foo/testPrimordials.js (right):

http://codereview.appspot.com/136053/diff/1/8#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

http://codereview.appspot.com/136053/diff/1/17
File tests/com/google/caja/plugin/ExpressionSanitizerTest.java (right):

http://codereview.appspot.com/136053/diff/1/17#newcode51
Line 51: "  'use cajita';"
On 2009/10/22 01:48:51, DavidSarah wrote:
> "  'use strict'; 'use cajita';"

Done.

http://codereview.appspot.com/136053/diff/1/13
File tests/com/google/caja/plugin/HtmlCompiledPluginTest.java (right):

http://codereview.appspot.com/136053/diff/1/13#newcode133
Line 133: + "  'use strict, cajita';"
On 2009/10/22 01:48:51, DavidSarah wrote:
> Bad syntax; see issue #1111. Should be 'use strict'; 'use cajita';

Done.

http://codereview.appspot.com/136053/diff/1/13#newcode145
Line 145: + "        (function () { 'use strict,cajita'; return {}.hello; })(),"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/13#newcode146
Line 146: + "        (function f() { 'use strict,cajita'; return {}.hello; })()
]"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/13#newcode152
Line 152: + "    function f() { 'use strict,cajita'; return {}.hello; }"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/13#newcode168
Line 168: "  'use strict, cajita';\n" +
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/13#newcode178
Line 178: "<script>'use strict, cajita';throw new Error('panic');a=1;</script>"
+
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/13#newcode180
Line 180: "  'use strict, cajita';\n" +
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/13#newcode184
Line 184: "  'use strict, cajita';\n" +
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/12
File tests/com/google/caja/plugin/domita_test.html (right):

http://codereview.appspot.com/136053/diff/1/12#newcode211
Line 211: loadScript('valija.co.js');
On 2009/10/22 00:50:28, MikeSamuel wrote:
> Why are we loading valija.co.js even when it's not valija?

Because, even though the tests themselves are in a 'use cajita' block, the
enclosing HTML is, at the top level, always Valija. And that guff looks for $v
and what not, and fails if it's not there.

The previous way we did this test -- by cajoling HTML as Cajita all the way --
was a special case that we achieved through our build rules; it was not a sort
of cajoling we expected any Caja users to actually attempt.

http://codereview.appspot.com/136053/diff/1/16
File tests/com/google/caja/plugin/domita_test_untrusted.html (right):

http://codereview.appspot.com/136053/diff/1/16#newcode463
Line 463: /*@@cajaLanguageSelectionStatements@@*/
On 2009/10/22 00:50:28, MikeSamuel wrote:
> Ok.  Maybe a pointer to the code that does the substitution.

Done.

http://codereview.appspot.com/136053/diff/1/14
File tests/com/google/caja/plugin/stages/DebuggingSymbolsStageTest.java (right):

http://codereview.appspot.com/136053/diff/1/14#newcode37
Line 37: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> "'use strict'; 'use cajita';\n"

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode47
Line 47: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode65
Line 65: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode84
Line 84: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode89
Line 89: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode97
Line 97: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode111
Line 111: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode119
Line 119: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode128
Line 128: + "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode138
Line 138: + "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode149
Line 149: + "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode169
Line 169: + "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode184
Line 184: + "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode197
Line 197: "'use strict, cajita';\n"
On 2009/10/22 01:48:51, DavidSarah wrote:
> ditto

Done.

http://codereview.appspot.com/136053/diff/1/14#newcode283
Line 283: "  'use strict, cajita';"
On 2009/10/22 01:48:51, DavidSarah wrote:
> "'use strict'; 'use cajita';"

Done.

http://codereview.appspot.com/136053/diff/1/4
File tests/com/google/caja/recursion.js (right):

http://codereview.appspot.com/136053/diff/1/4#newcode14
Line 14: 
On 2009/10/22 01:48:51, DavidSarah wrote:
> "use strict";

Done.

[DavidSarah, 2009-10-30 08:41:27]

http://codereview.appspot.com/136053/diff/6098/5216
File build.xml (right):

http://codereview.appspot.com/136053/diff/6098/5216#newcode870
Line 870: <output file="${www}/lolcat-search/searchbox.co.html"
Is this convention still .co.html, or should it be .out.html?

[ihab.awad, 2009-11-02 05:53:47]

New snapshot.

http://codereview.appspot.com/136053/diff/6098/5216
File build.xml (right):

http://codereview.appspot.com/136053/diff/6098/5216#newcode870
Line 870: <output file="${www}/lolcat-search/searchbox.co.html"
On 2009/10/30 08:41:27, DavidSarah wrote:
> Is this convention still .co.html, or should it be .out.html?

Good catch: the demos were not converted over to the new world order yet.

I converted them and added module / promise mojo to the "lolcats" and "slides"
demos.

[ihab.awad, 2009-11-10 18:05:34]

Mike Samuel, could you please look at the Domita parts of this?

Mike Stay agreed to look at everything else.

Thanks!

[metaweta, 2009-11-11 20:25:25]

LGTM.  Please follow up with a message to the list that the API has changed, and
give examples.

http://codereview.appspot.com/136053/diff/7001/7039
File src/com/google/caja/cajita-module.js (right):

http://codereview.appspot.com/136053/diff/7001/7039#newcode270
src/com/google/caja/cajita-module.js:270: // this should not happen
Throw an informative exception.

http://codereview.appspot.com/136053/diff/7001/7059
File src/com/google/caja/service/HtmlHandler.java (right):

http://codereview.appspot.com/136053/diff/7001/7059#newcode67
src/com/google/caja/service/HtmlHandler.java:67:
"http://caja.appspot.com/cajoler";
Good catch

http://codereview.appspot.com/136053/diff/7001/7007
File tests/com/google/caja/a.js (right):

http://codereview.appspot.com/136053/diff/7001/7007#newcode23
tests/com/google/caja/a.js:23: x + y
semicolon

http://codereview.appspot.com/136053/diff/7001/7022
File tests/com/google/caja/c.js (right):

http://codereview.appspot.com/136053/diff/7001/7022#newcode23
tests/com/google/caja/c.js:23: x + 1
semicolon

[MikeSamuel, 2009-11-11 22:46:34]

http://codereview.appspot.com/136053/diff/1/12
File tests/com/google/caja/plugin/domita_test.html (right):

http://codereview.appspot.com/136053/diff/1/12#newcode211
tests/com/google/caja/plugin/domita_test.html:211: loadScript('valija.co.js');
On 2009/10/30 06:17:42, ihab.awad wrote:
> On 2009/10/22 00:50:28, MikeSamuel wrote:
> > Why are we loading valija.co.js even when it's not valija?
> 
> Because, even though the tests themselves are in a 'use cajita' block, the
> enclosing HTML is, at the top level, always Valija. And that guff looks for $v
> and what not, and fails if it's not there.
> 
> The previous way we did this test -- by cajoling HTML as Cajita all the way --
> was a special case that we achieved through our build rules; it was not a sort
> of cajoling we expected any Caja users to actually attempt.

But shouldn't we test under cajita mode as a container probably would -- without
loading the unnecessary valija stuff?

http://codereview.appspot.com/136053/diff/7001/7020
File tests/com/google/caja/opensocial/DefaultGadgetRewriterTest.java (right):

http://codereview.appspot.com/136053/diff/7001/7020#newcode99
tests/com/google/caja/opensocial/DefaultGadgetRewriterTest.java:99: }
How is this test different from testSocialHelloWorld on line 101 now?

http://codereview.appspot.com/136053/diff/7001/7023
File tests/com/google/caja/plugin/domita_test.html (right):

http://codereview.appspot.com/136053/diff/7001/7023#newcode315
tests/com/google/caja/plugin/domita_test.html:315: // TODO(mikesamuel): rewrite
XHR and setTimeout tests to use this scheme.
plz wrap or maybe deindent this entire script block body

http://codereview.appspot.com/136053/diff/7001/7023#newcode377
tests/com/google/caja/plugin/domita_test.html:377: 'assert', 'assertContains',
'assertEquals', 'assertEvaluatesToFalse',
plz wrap

http://codereview.appspot.com/136053/diff/7001/7023#newcode438
tests/com/google/caja/plugin/domita_test.html:438: return
'http://gadget-proxy/?url=' + encodeURIComponent(uri)
plz wrap

http://codereview.appspot.com/136053/diff/7001/7023#newcode448
tests/com/google/caja/plugin/domita_test.html:448: // Create a readonly mirror
of document so that we can test that mutations
plz wrap

http://codereview.appspot.com/136053/diff/7001/7026
File tests/com/google/caja/plugin/jsunit.js (right):

http://codereview.appspot.com/136053/diff/7001/7026#newcode57
tests/com/google/caja/plugin/jsunit.js:57: console.log('TEST ERROR: pass without
a test ID');
Maybe console.error instead of console.log

[ihab.awad, 2009-11-12 01:28:50]

Snapshotted. See replies below.

http://codereview.appspot.com/136053/diff/7001/7039
File src/com/google/caja/cajita-module.js (right):

http://codereview.appspot.com/136053/diff/7001/7039#newcode270
src/com/google/caja/cajita-module.js:270: // this should not happen
On 2009/11/11 20:25:25, metaweta wrote:
> Throw an informative exception.

Good point, done.

Not sure what an *informative* exception is here, but I threw something. This
will get bubbled up to the top-level <script> onload event, which will probably
drop it on the floor, but it might show up in a debugger and we can see it if it
happens in reality. Which it shouldn't. But....

http://codereview.appspot.com/136053/diff/7001/7059
File src/com/google/caja/service/HtmlHandler.java (right):

http://codereview.appspot.com/136053/diff/7001/7059#newcode67
src/com/google/caja/service/HtmlHandler.java:67:
"http://caja.appspot.com/cajoler";
On 2009/11/11 20:25:25, metaweta wrote:
> Good catch

We are _sporty_!

http://codereview.appspot.com/136053/diff/7001/7007
File tests/com/google/caja/a.js (right):

http://codereview.appspot.com/136053/diff/7001/7007#newcode23
tests/com/google/caja/a.js:23: x + y
On 2009/11/11 20:25:25, metaweta wrote:
> semicolon

Done.

http://codereview.appspot.com/136053/diff/7001/7022
File tests/com/google/caja/c.js (right):

http://codereview.appspot.com/136053/diff/7001/7022#newcode23
tests/com/google/caja/c.js:23: x + 1
On 2009/11/11 20:25:25, metaweta wrote:
> semicolon

Done.

http://codereview.appspot.com/136053/diff/7001/7020
File tests/com/google/caja/opensocial/DefaultGadgetRewriterTest.java (right):

http://codereview.appspot.com/136053/diff/7001/7020#newcode99
tests/com/google/caja/opensocial/DefaultGadgetRewriterTest.java:99: }
On 2009/11/11 22:46:34, MikeSamuel wrote:
> How is this test different from testSocialHelloWorld on line 101 now?

It's not. Bobbitted.

http://codereview.appspot.com/136053/diff/7001/7023
File tests/com/google/caja/plugin/domita_test.html (right):

http://codereview.appspot.com/136053/diff/7001/7023#newcode315
tests/com/google/caja/plugin/domita_test.html:315: // TODO(mikesamuel): rewrite
XHR and setTimeout tests to use this scheme.
On 2009/11/11 22:46:34, MikeSamuel wrote:
> plz wrap or maybe deindent this entire script block body

Went with the deindenting.

http://codereview.appspot.com/136053/diff/7001/7023#newcode377
tests/com/google/caja/plugin/domita_test.html:377: 'assert', 'assertContains',
'assertEquals', 'assertEvaluatesToFalse',
On 2009/11/11 22:46:34, MikeSamuel wrote:
> plz wrap

Done.

http://codereview.appspot.com/136053/diff/7001/7023#newcode438
tests/com/google/caja/plugin/domita_test.html:438: return
'http://gadget-proxy/?url=' + encodeURIComponent(uri)
On 2009/11/11 22:46:34, MikeSamuel wrote:
> plz wrap

Done.

http://codereview.appspot.com/136053/diff/7001/7023#newcode448
tests/com/google/caja/plugin/domita_test.html:448: // Create a readonly mirror
of document so that we can test that mutations
On 2009/11/11 22:46:34, MikeSamuel wrote:
> plz wrap
> 

Done.

http://codereview.appspot.com/136053/diff/7001/7026
File tests/com/google/caja/plugin/jsunit.js (right):

http://codereview.appspot.com/136053/diff/7001/7026#newcode57
tests/com/google/caja/plugin/jsunit.js:57: console.log('TEST ERROR: pass without
a test ID');
On 2009/11/11 22:46:34, MikeSamuel wrote:
> Maybe console.error instead of console.log

Done.

[MikeSamuel, 2009-11-12 03:29:04]

I reviewed all the parts that Mike Stay didn't.   Those parts LGTM

[MikeSamuel, 2009-11-13 20:52:30]

LGTM

http://codereview.appspot.com/136053/diff/9157/9203
File src/com/google/caja/parser/js/Directive.java (right):

http://codereview.appspot.com/136053/diff/9157/9203#newcode103
src/com/google/caja/parser/js/Directive.java:103: // Escaping has modified the
directive. Render nothing.
maybe add a comment pointing to the longer discussion in the bug.

http://codereview.appspot.com/136053/diff/9157/9203#newcode107
src/com/google/caja/parser/js/Directive.java:107: rc.getOut().consume(";");
cool

http://codereview.appspot.com/136053/diff/9157/9175
File tests/com/google/caja/parser/js/parsertest6.js (right):

http://codereview.appspot.com/136053/diff/9157/9175#newcode3
tests/com/google/caja/parser/js/parsertest6.js:3: "use f\u0008oo";  // Should
not be rendered
cool

[ihab.awad, 2009-11-13 21:47:37]

@3846

[DavidSarah, 2009-11-14 02:13:08]

http://codereview.appspot.com/136053/diff/9157/9203
File src/com/google/caja/parser/js/Directive.java (right):

http://codereview.appspot.com/136053/diff/9157/9203#newcode102
src/com/google/caja/parser/js/Directive.java:102: if
(!escaped.toString().contains(directiveString)) {
.contains is wrong, I think. Suppose directiveString is a single backslash
character; it will be escaped to something that contains a backslash.

Use !escaped.substring(1, escaped.length()-1).equals(directiveString).

(StringBuilder implements substring.)

http://codereview.appspot.com/136053/diff/9157/9206
File src/com/google/caja/parser/js/DirectivePrologue.java (right):

http://codereview.appspot.com/136053/diff/9157/9206#newcode50
src/com/google/caja/parser/js/DirectivePrologue.java:50: * <a
href="http://wiki.ecmascript.org/lib/exe/fetch.php?id=es3.1%3Aes3.1_proposal_working_draft&cache=cache&media=es3.1:es5-tc392008-040.pdf">the
ES5 spec</a>.
That isn't the final version. Just refer to
http://wiki.ecmascript.org/doku.php?id=es3.1:es3.1_proposal_working_draft

http://codereview.appspot.com/136053/diff/9157/9204
File src/com/google/caja/parser/js/Parser.java (right):

http://codereview.appspot.com/136053/diff/9157/9204#newcode338
src/com/google/caja/parser/js/Parser.java:338: case KEYWORD:
What about infix keywords such as 'in' or 'instanceof'?
For example, '"use strict" in {}'.

http://codereview.appspot.com/136053/diff/9157/9204#newcode340
src/com/google/caja/parser/js/Parser.java:340: isDirective = true;
Nope.
For example, '"use strict" /*foo*/ + ""'.

http://codereview.appspot.com/136053/diff/9157/9204#newcode349
src/com/google/caja/parser/js/Parser.java:349: } else if
(tq.lookaheadToken(Punctuation.RCURLY)) {
'}' is not the only punctuation that will cause semicolon insertion here.

http://codereview.appspot.com/136053/diff/9157/9204#newcode357
src/com/google/caja/parser/js/Parser.java:357: break;
Line continuations prevent a string from being a directive, even if followed by
another string, so looping is wrong.

Stopped looking for more problems; I think duplicating the semicolon insertion
logic here is probably not a good approach.