Reviewers: ihab.awad,
Message:
Please check to see that I'm using rawAtUrl and cajoledAtUrl in the
right places.
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js
File src/com/google/caja/plugin/caja.js (right):
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode264
src/com/google/caja/plugin/caja.js:264:
innerContainer.setAttribute('class', 'caja_innerContainer___');
On 2011/03/14 21:31:13, ihab.awad wrote:
Long whitespace at the end of this line; line should be unchanged.
What do you mean? Leave the whitespace there?
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode269
src/com/google/caja/plugin/caja.js:269:
innerContainer.appendChild(div.childNodes[0]);
On 2011/03/14 21:31:13, ihab.awad wrote:
Does this not *reverse* the order of the nodes as they are added?
No. appendChild(node) places the node at the end of the list of
childNodes, not the start.
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode288
src/com/google/caja/plugin/caja.js:288:
tamingWindow.plugin_dispatchToHandler___;
On 2011/03/14 21:31:13, ihab.awad wrote:
Hm -- how did our stuff work *before* this change?
It didn't (at least, not when domita was attached only to the taming
frame). That's why I changed it.
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode295
src/com/google/caja/plugin/caja.js:295: * cajoled content. The JSON
object has the following properties:
On 2011/03/14 21:31:13, ihab.awad wrote:
Perhaps outdent this line to make the comment readable?
Obsoleted with builder.
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode302
src/com/google/caja/plugin/caja.js:302: * contains the uncajoled
html to be sent to the cajoler.
On 2011/03/14 21:31:13, ihab.awad wrote:
The original API also supported passing just JavaScript ... we would
want to
maintain this ability.
You mean, as though it were passed in a script block?
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode319
src/com/google/caja/plugin/caja.js:319: // then invoke run with the
result
On 2011/03/14 21:31:13, ihab.awad wrote:
Throw "unsupported ${something}" here.
Done.
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode324
src/com/google/caja/plugin/caja.js:324: callback(moduleFunc(imports));
On 2011/03/14 21:31:13, ihab.awad wrote:
Should be changed to an "opt_callback" to be consistent with "runNow".
Done.
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode329
src/com/google/caja/plugin/caja.js:329: // fetch the cajoled content and
invoke run with the result.
On 2011/03/14 21:31:13, ihab.awad wrote:
Throw "unsupported ${something}" here.
Done.
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode336
src/com/google/caja/plugin/caja.js:336: * @param descriptor a JSON
object describing the cajoled content.
On 2011/03/14 21:31:13, ihab.awad wrote:
It seems to me it would be easiest for users of the run* functions if:
* The descriptor schema were precisely the same for both cases; and
* The only difference between "run" and "runNow" is that "runNow"
accepts only
the synchronously satisfiable subset of descriptor contents.
Do you agree that it's desirable to arrange things that way?
Done.
http://codereview.appspot.com/4211045/diff/9015/src/com/google/caja/plugin/caja.js#newcode346
src/com/google/caja/plugin/caja.js:346: function runNow(descriptor,
extraImports, opt_callback) {
On 2011/03/14 21:31:13, ihab.awad wrote:
It seems there's a lot of code duplication between "run" and "runNow"
... it
feels like they should both call some common function.
Done.
Description:
Changes caja.js to support
- telling the cajoling service to fetch and cajole content at a url
- fetching cached, already cajoled code at a url
- posting uncajoled content to the cajoling service
- when the cajoler is part of the web server, running cajoled code
provided as a string in the page.
Please review this at http://codereview.appspot.com/4211045/
Affected files:
M src/com/google/caja/demos/photon/container/main.js
M src/com/google/caja/es53.js
M src/com/google/caja/plugin/caja.js
M tests/com/google/caja/plugin/default-test-driver.js
M tests/com/google/caja/plugin/es53-test-taming-inout.js
M tests/com/google/caja/plugin/es53-test-taming-tamed.js
M tests/com/google/caja/plugin/es53-test-taming-untamed.js