Some initial comments but still much more to look at.
http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/es53.js File src/com/google/caja/es53.js (right): http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/es53.js#newcode2339 src/com/google/caja/es53.js:2339: * {@code typename} is used for toString and {@code errorMessage} is used Maybe @param typename http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/bridal.js File src/com/google/caja/plugin/bridal.js (right): http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/bridal.js#newcode29 src/com/google/caja/plugin/bridal.js:29: // TODO(kpreid): These were previously gotten from the taming frame, not the host frame; is this OK? Getting them from the host frame allows the host frame to easily remove/wrap/monkey patch these which may be useful. Getting them from the taming frame means more work is required to do any of those. http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/bridal.js#newcode744 src/com/google/caja/plugin/bridal.js:744: } ; http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/caja.js File src/com/google/caja/plugin/caja.js (right): http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/caja.js#newcode328 src/com/google/caja/plugin/caja.js:328: * TODO(kpreid): document this. Yes please. http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/caja.js#newcode332 src/com/google/caja/plugin/caja.js:332: if (typeof o !== 'undefined' && o !== null) { Isn't the below only effective for objects and functions. Are we expecting functions to be passed to this function? Can the test become ('object' === typeof o)? http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/caja.js#newcode339 src/com/google/caja/plugin/caja.js:339: if (!(typeof p === 'number' || ('' + (+p)) === p) && !(/__$/).test(p)) { How can typeof p be number when you've just converted it to a string above? http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/caja.js#newcode340 src/com/google/caja/plugin/caja.js:340: if (typeof o[p] === 'function') { Should we read o[p] once instead of here and again below before applying. http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/caja.js#newcode347 src/com/google/caja/plugin/caja.js:347: o.HasProperty___ = function (P) { return P in this; }; P -> p http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/domado.js File src/com/google/caja/plugin/domado.js (right): http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/plugin/domado.js#newcode1 src/com/google/caja/plugin/domado.js:1: // Copyright (C) 2008-2011 Google Inc. Is this based on domita.js via svn mv in your local client? When you diff via svn diff --notice_ancestry, do you get smaller diffs? http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/ses/es5shim.js File src/com/google/caja/ses/es5shim.js (right): http://codereview.appspot.com/4535118/diff/17001/src/com/google/caja/ses/es5shim.js#newcode677 src/com/google/caja/ses/es5shim.js:677: !desc.ses_ignoreBug_propertyWillAppearAsOwn) { Could you comment on this? http://codereview.appspot.com/4535118/
