Very close. Is there a way to create test cases for the 'cajoled' case? Since you're remote, I think it's ok to check in as-is and I'll add a test case myself if you want right afterwards.
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/23 01:25:09, metaweta wrote:
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?
Oh -- it just seemed like the line was showing up as a diff even though there is not different in content, so I expected it must have been a whitespace change. No biggie, but it does seem like there's extra whitespace at the end of the line. 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/23 01:25:09, metaweta wrote:
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.
Geflurpft. You're right. 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/23 01:25:09, metaweta wrote:
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?
Never mind; see comments in next rev. http://codereview.appspot.com/4211045/diff/25001/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: So fwiw -- given that we've implemented this, it looks to me like there is no need for a Builder _per se_ since there is no use case for a fluent interface (aBuilder.withFoo().andBar().butBaz().exceptBoo()...). The calls are always only one deep. So maybe cajoledAtUrl(), cajoled(), and friends should just be functions on the frame. The could return a run()-able, or they could incorporate the parameters of run() in their own interface -- whichever. Does this make sense? http://codereview.appspot.com/4211045/diff/25001/src/com/google/caja/plugin/caja.js#newcode305 src/com/google/caja/plugin/caja.js:305: * @param callback a function that is called providing the completion What is 'src' for? http://codereview.appspot.com/4211045/diff/25001/src/com/google/caja/plugin/caja.js#newcode330 src/com/google/caja/plugin/caja.js:330: } What is 'src' for? http://codereview.appspot.com/4211045/diff/25001/src/com/google/caja/plugin/caja.js#newcode382 src/com/google/caja/plugin/caja.js:382: callback({ Isn't this the 'rawAtUrl' case? loader.async(url) will go fetch the (raw) content via a cajoling service *and cajole* it. http://codereview.appspot.com/4211045/diff/25001/src/com/google/caja/plugin/caja.js#newcode396 src/com/google/caja/plugin/caja.js:396: What happens if a client calls builder() twice, diddles with both of them, then calls run() on one builder, then calls run() on the second? By my interpretation, since the variable 'run___' is state in the builder, not in the frame itself, caja.js would attempt to run two modules in the same frame. http://codereview.appspot.com/4211045/diff/25001/tests/com/google/caja/plugin/default-test-driver.js File tests/com/google/caja/plugin/default-test-driver.js (right): http://codereview.appspot.com/4211045/diff/25001/tests/com/google/caja/plugin/default-test-driver.js#newcode27 tests/com/google/caja/plugin/default-test-driver.js:27: .cajoledAtUrl(testCase) Happiness prevaileth. http://codereview.appspot.com/4211045/
