A few comments -- will discuss further over chat and report back to the list.
http://codereview.appspot.com/89073/diff/3037/3040 File src/com/google/caja/plugin/domita.js (right): http://codereview.appspot.com/89073/diff/3037/3040#newcode773 Line 773: '([^' + XML_SPACE + ']+)([' + XML_SPACE + ']|$)', 'g'); Plase move the above 4 lines down closer to the point of first use. http://codereview.appspot.com/89073/diff/3037/3040#newcode849 Line 849: function(m0, m1, m2) { Please give these parameters descriptive names. Like m0 -> "_" (unused); m1 -> "idref", m2 -> "trailingWhitespace"? http://codereview.appspot.com/89073/diff/3037/3040#newcode852 Line 852: === m1.substring(m1.length - idSuffix.length)) { Please parcel up the above 3 lines into an "endsWith(aString, aSuffix)" function and use it. http://codereview.appspot.com/89073/diff/3037/3040#newcode892 Line 892: return null; class = cdata-list. Ok. http://codereview.appspot.com/89073/diff/3037/3040#newcode893 Line 893: case html4.atype.GLOBAL_NAME: What is the justification for suffixing a GLOBAL_NAME (or any kind of "name" for that matter) with an ID suffix? And if you suffix a GLOBAL_NAME, why not also a LOCAL_NAME? Do you understand the source of the distinction in our schemata between GLOBAL_NAME and LOCAL_NAME? The W3C page http://www.w3.org/TR/html401/index/attributes.html, which is referenced from our html4-attributes-defs.json, is not helpful in that regard. http://codereview.appspot.com/89073/diff/3037/3040#newcode907 Line 907: return null; Ok, good fix. http://codereview.appspot.com/89073/diff/3037/3040#newcode910 Line 910: if (value && !illegalSuffix.test(value) && isXmlName(value)) { Ok, good fix. http://codereview.appspot.com/89073/diff/3037/3040#newcode2012 Line 2012: switch (atype) { If you are going to add 'idSuffix' to GLOBAL_NAME attributes in rewriteAttribute(), shouldn't you be stripping them off here? http://codereview.appspot.com/89073/diff/3037/3040#newcode2026 Line 2026: function(m0, m1, m2) { Please rename params; see above. http://codereview.appspot.com/89073/diff/3037/3040#newcode2029 Line 2029: === m1.substring(m1.length - idSuffix.length)) { Please use "endsWith"; see above. http://codereview.appspot.com/89073/diff/3037/3041 File src/com/google/caja/plugin/templates/TemplateCompiler.java (right): http://codereview.appspot.com/89073/diff/3037/3041#newcode292 Line 292: if (!checkIllegalSuffixes(value, pos)) { return; } Why remove illegal suffixes? We append an ID suffix where appropriate, and that should be robust enough. http://codereview.appspot.com/89073/diff/3037/3041#newcode297 Line 297: if (!checkIllegalSuffix(value, pos)) { return; } Here and in 2 other places below: Why remove illegal suffixes? http://codereview.appspot.com/89073/diff/3037/3041#newcode432 Line 432: return false; Lookin' good. :) http://codereview.appspot.com/89073
