updated snapshot.
http://codereview.appspot.com/89073/diff/10001/10008 File src/com/google/caja/plugin/domita.js (right): http://codereview.appspot.com/89073/diff/10001/10008#newcode817 Line 817: if (0 < n && str.substring(n) === suffix) { On 2009/09/23 23:29:43, MikeSamuel wrote:
0 < n should be 0 <= n since str is a suffix of str for all str.
the way unsuffix is used, it should never return '' unless fail is also ''. I'll fix the comment to clarify that. http://codereview.appspot.com/89073/diff/10001/10008#newcode828 Line 828: function virtualizeAttributeValue(attrType, realValue) { On 2009/09/23 23:29:43, MikeSamuel wrote:
do we need to force realValue to a string or do all clients do that.
I was thinking that attribute values don't necessarily need to be strings, so this leaves the value unchanged when no transformation needs to be done. but the DOM spec says they are always strings, and I can't think of a case where they aren't. ok, adding a coercion. http://codereview.appspot.com/89073/diff/10001/10008#newcode837 Line 837: return unsuffix(id, idSuffix, '') + spaces; On 2009/09/23 23:29:43, MikeSamuel wrote:
can we normalize spaces. instead of returning unsuffix(...) + spaces, maybe return unsuffix(...) + ' '.
ok http://codereview.appspot.com/89073/diff/10001/10008#newcode910 Line 910: function(_, id, spaces) { return id + idSuffix + spaces; }); On 2009/09/23 23:29:43, MikeSamuel wrote:
Same as at 837.
ok http://codereview.appspot.com/89073/diff/10001/10007 File tests/com/google/caja/plugin/templates/TemplateCompilerTest.java (right): http://codereview.appspot.com/89073/diff/10001/10007#newcode553 Line 553: } On 2009/09/23 23:29:43, MikeSamuel wrote:
Can you add an assertMessagesLessSevereThan(MessageLevel.WARNING) to the end of the new tests. If there should be warnings, you can use assertMessage(true, ...) to check them and remove them from the queue. I want to make sure that any warnings about sketchy classes stay
visible. ok. http://codereview.appspot.com/89073
