http://codereview.appspot.com/130076/diff/19/24
File src/com/google/caja/plugin/domita.js (right):

http://codereview.appspot.com/130076/diff/19/24#newcode799
Line 799: // Swap last attribute name/value pair in place, and reprocess
here.
I'm wary that you're changing the order of the attributes, which matters
if an attribute is duplicated, but iirc browsers aren't consistent about
what value gets used when an attr is repeated, so it shouldn't matter.
maybe add a comment about that.

http://codereview.appspot.com/130076/diff/19/23
File src/com/google/caja/plugin/html-emitter.js (right):

http://codereview.appspot.com/130076/diff/19/23#newcode220
Line 220: var isLimitClosed = detached[0].parentNode !== limit;
do you mean detached[1] instead of detached[0].parentNode?

http://codereview.appspot.com/130076/diff/19/23#newcode290
Line 290: var eltype = html4.ELEMENTS[tagName];
need hasOwnProperty test here

http://codereview.appspot.com/130076/diff/19/23#newcode291
Line 291: if ((eltype & html4.eflags.UNSAFE) !== 0) { return; }
it looks to me like if a startTag is rejected, the matching endTag will
never find the right tag to close, so it will close all open tags,
screwing up the rest of the structure.  which is ok, but should be
documented.

it also looks like this doesn't cope too well with optional-close, like
<p>.  which is ok, but should also be documented.

http://codereview.appspot.com/130076/diff/19/23#newcode351
Line 351: ___.grantRead(tameDoc, 'writeln');
shouldn't this be grantFunc?

http://codereview.appspot.com/130076

Reply via email to