updated snapshot
http://codereview.appspot.com/135054/diff/1/3
File src/com/google/caja/cajita.js (right):
http://codereview.appspot.com/135054/diff/1/3#newcode98
Line 98: if (arguments.length < 3) end = self.length;
On 2009/10/20 03:59:56, MikeSamuel wrote:
Please use curlies around if bodies.
ok
http://codereview.appspot.com/135054/diff/1/3#newcode1598
Line 1598: var args = [self].concat(Array.slice(opt_args, 0));
On 2009/10/20 03:59:56, MikeSamuel wrote:
How about
var args = [self];
if (opt_args !== void 0 && opt_args !== null) {
args.push.apply(args, opt_args);
}
?
It avoids an unnecessary array creation, and the apply will do the
same
TypeError checks for opt_args === 9.
that doesn't work, because opt_args might be an arguments object,
so we have to call Array.slice anyway.
http://codereview.appspot.com/135054/diff/1/3#newcode3030
Line 3030: return toFunc(this).apply(USELESS, opt_args || []);
On 2009/10/20 03:59:56, MikeSamuel wrote:
Is this where you would throw a TypeError if you wanted to be strict
re
foo.apply(bar, 9)?
this is one of the places. it also has to be done in
tameXo4a, tameInnocent, PseudoFunction, and maybe
in valija-cajita.js, and I might have forgotten a
place or two.
I guess it's plausible to factor all that out into a common
___.apply() function and use that consistently instead of
native apply, but I'm wary of the added runtime cost.
http://codereview.appspot.com/135054/diff/1/2
File tests/com/google/caja/plugin/domita_test_untrusted.html (right):
http://codereview.appspot.com/135054/diff/1/2#newcode1723
Line 1723: assertArray([], Array.slice([9], void 0, 0), '([9], void 0,
0)');
On 2009/10/20 03:59:56, MikeSamuel wrote:
How about a quick check of provided bounds:
assetArray([2,3,4], Array.slice([0,1,2,3,4,5,6], 2, 5);
assetArray([2,3,void 0], Array.slice([0,1,2,3], 2, 5);
ok. though the second case is supposed to return [2,3],
(and does).
http://codereview.appspot.com/135054/diff/1/2#newcode1765
Line 1765: assertEquals('add(4, 5)', 5, count);
On 2009/10/20 03:59:56, MikeSamuel wrote:
Maybe test with an array like object.
foo.apply({}, { 0: 'zero', 1: 'one', 3: 'three', length: 5 });
ES5 says that throws TypeError, and it does in the browsers I tried.
in caja, it throws in cajita, but succeeds in valija.
I'll add it to the list of things-that-should-throw-but-don't.
http://codereview.appspot.com/135054
