https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/apitaming/google.visualization.policyFactory.js
File src/com/google/caja/apitaming/google.visualization.policyFactory.js
(right):
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/apitaming/google.visualization.policyFactory.js#newcode419
src/com/google/caja/apitaming/google.visualization.policyFactory.js:419:
return [ args[0].apply(Object.freeze({}), []) ];
Revert this if you agree with my other comments.
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/ses-frame-group.js
File src/com/google/caja/plugin/ses-frame-group.js (right):
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/ses-frame-group.js#newcode114
src/com/google/caja/plugin/ses-frame-group.js:114:
Object.freeze({USELESS: 'USELESS'}),
I think this should use a singleton USELESS, because constructing a
frozen object is more expensive than we want for a single function call.
The rationale explained elsewhere should also be explained here.
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-membrane.js
File src/com/google/caja/plugin/taming-membrane.js (right):
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-membrane.js#newcode40
src/com/google/caja/plugin/taming-membrane.js:40: function
directConstructor(obj) {
Maybe document this function, or at least what BASE_OBJECT_CONSTRUCTOR
means?
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-membrane.js#newcode94
src/com/google/caja/plugin/taming-membrane.js:94: // then check for
enumerability?
This looks equivalent to Object.keys.
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-membrane.js#newcode448
src/com/google/caja/plugin/taming-membrane.js:448: // CAUTION: It is
ESSENTIAL that we pass USELESS, not (void 0), when
Suggest moving this comment to the definition(s) of USELESS.
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-membrane.js#newcode465
src/com/google/caja/plugin/taming-membrane.js:465: var t = function (_)
{
should be function(var_args) (no space, that particular arg name), yes?
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-schema.js
File src/com/google/caja/plugin/taming-schema.js (left):
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-schema.js#oldcode111
src/com/google/caja/plugin/taming-schema.js:111: if
(privilegedAccess.isDefinedInCajaFrame(f)) {
Why is this constraint no longer needed?
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-schema.js
File src/com/google/caja/plugin/taming-schema.js (right):
https://codereview.appspot.com/14605043/diff/1/src/com/google/caja/plugin/taming-schema.js#newcode202
src/com/google/caja/plugin/taming-schema.js:202: return p(self,
advice.apply((void 0), [f, self, args]));
Does this (and the 2 below) even need to be apply instead of a plain
function call?
https://codereview.appspot.com/14605043/diff/1/tests/com/google/caja/plugin/test-defensible-objects.js
File tests/com/google/caja/plugin/test-defensible-objects.js (right):
https://codereview.appspot.com/14605043/diff/1/tests/com/google/caja/plugin/test-defensible-objects.js#newcode38
tests/com/google/caja/plugin/test-defensible-objects.js:38:
assertEquals('USELESS', this.USELESS);
Ew. I'd rather see there be a singleton USELESS. User code might find a
need to reliably recognize it; in fact, if any such uses already exist,
removing caja.USELESS is a breaking change.
https://codereview.appspot.com/14605043/
--
---
You received this message because you are subscribed to the Google Groups "Google Caja Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
