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) {
stray 'typeof'

http://codereview.appspot.com/6182049/diff/3001/src/com/google/caja/plugin/caja.js#newcode258
src/com/google/caja/plugin/caja.js:258: };
it bothers me a little that the default value is duplicated here and in
domado.js, but I don't feel strongly about it.

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 +
")")
delete

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++) {
how about use slice to make the copy?

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);
"true" is an extraneous arg

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: /*
this is commented out?

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);
is this an error or not?

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;
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?

http://codereview.appspot.com/6182049/

Reply via email to