Snapshotted.
http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/caja.js File src/com/google/caja/plugin/caja.js (right): http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/caja.js#newcode250 src/com/google/caja/plugin/caja.js:250: if (typeof partial.targetAttributePresets.whitelist.length === 0) { On 2012/05/06 00:09:28, felix8a wrote:
stray 'typeof'
Done. http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/caja.js#newcode258 src/com/google/caja/plugin/caja.js:258: }; On 2012/05/06 00:09:28, felix8a wrote:
it bothers me a little that the default value is duplicated here and
in
domado.js, but I don't feel strongly about it.
You're right; I removed it from here. http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/domado.js File src/com/google/caja/plugin/domado.js (right): http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/domado.js#newcode5039 src/com/google/caja/plugin/domado.js:5039: console.log("##### rewriteTargetAttribute(" + value + ", " + tagName + ", " + attribName + ")") On 2012/05/06 00:09:28, felix8a wrote:
delete
Done. http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/es53-frame-group.js File src/com/google/caja/plugin/es53-frame-group.js (right): http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/es53-frame-group.js#newcode259 src/com/google/caja/plugin/es53-frame-group.js:259: for (var i = 0; i < config.targetAttributePresets.whitelist.length; i++) { On 2012/05/06 00:09:28, felix8a wrote:
how about use slice to make the copy?
Done. http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/es53-frame-group.js#newcode267 src/com/google/caja/plugin/es53-frame-group.js:267: undefined, targetAttributePresets, true); On 2012/05/06 00:09:28, felix8a wrote:
"true" is an extraneous arg
Done. http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/templates/TemplateCompiler.java File src/com/google/caja/plugin/templates/TemplateCompiler.java (right): http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/templates/TemplateCompiler.java#newcode308 src/com/google/caja/plugin/templates/TemplateCompiler.java:308: /* On 2012/05/06 00:09:28, felix8a wrote:
this is commented out?
Yes, cruft. Removed. http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/templates/TemplateCompiler.java#newcode320 src/com/google/caja/plugin/templates/TemplateCompiler.java:320: System.err.println("r is not safe: " + r.result); On 2012/05/06 00:09:28, felix8a wrote:
is this an error or not?
No, per the below comment, we drop stuff on the floor and rely on previously executed code to have warned. This was just debugging cruft. Removed. http://codereview.appspot.com/6182049/diff/3001/tests/com/google/caja/plugin/container.js File tests/com/google/caja/plugin/container.js (right): http://codereview.appspot.com/6182049/diff/3001/tests/com/google/caja/plugin/container.js#newcode55 tests/com/google/caja/plugin/container.js:55: return "rewritten-" + value; On 2012/05/06 00:09:28, felix8a wrote:
this rewrite doesn't show up in any tests. it looks like container.js
is only
used by HtmlCompiledPluginTest.java, and none of those trigger rewriteTargetAttribute___.
which is odd, because one of the tests has an <a> tag and tests that
the result
has target="_self", which should not have happened. did that test
really pass? You are right -- I must have forgotten to re-run the test before submitting. A simple corresponding change to HtmlCompiledPluginTest and all is well. http://codereview.appspot.com/6182049/
