fix fn.apply() and Array.slice() on IE[678]

this fixes http://code.google.com/p/google-caja/issues/detail?id=1077

fn.apply() has trouble on IE[678] because IE's native apply
doesn't conform to standards.  ES5 says that these
should all be equivalent:
    fn.apply(x)
    fn.apply(x, [])
    fn.apply(x, null)
    fn.apply(x, void 0)
but IE throws a type error for the last two.

there was a previous patch that fixed this problem in
most cases, but it didn't fix the case of valija-mode
    Object.prototype.toString.apply(x)
which is a common technique to get the native type of x.

I think this patch now fixes fn.apply() behavior in
almost all cases.  there are still two deviations from
ES5 behavior, mentioned in domita_test_untrusted.html

1. if fn.apply is the native apply, not a caja wrapper,
then fn.apply(x, null) will throw a type error on IE.
to fix that, fn.apply would have to always be a wrapper
on IE.  I don't think it's worth fixing that, because
it's easy for programs to avoid the problem.  and since
this is an existing browser incompatibility, it's
unlikely that programs rely on that specific behavior.

2. fn.apply(x, 9) is supposed to throw a type error,
but when fn.apply is a caja wrapper, it's sometimes
equivalent to fn.apply(x, undefined).  to fix that,
the fn.apply wrappers that use Array.slice would have
to do a complex typecheck: throw an error unless
args is null or undefined or an array or an array-like
object like "arguments".  I don't think that's worth
it, since I can't think of a plausible reason that
code would rely on getting a type error here.

[MikeSamuel, 2009-10-20 03:59:56]

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;
Please use curlies around if bodies.

http://codereview.appspot.com/135054/diff/1/3#newcode1598
Line 1598: var args = [self].concat(Array.slice(opt_args, 0));
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.

http://codereview.appspot.com/135054/diff/1/3#newcode3030
Line 3030: return toFunc(this).apply(USELESS, opt_args || []);
Is this where you would throw a TypeError if you wanted to be strict re
foo.apply(bar, 9)?

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)');
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);

http://codereview.appspot.com/135054/diff/1/2#newcode1756
Line 1756: //expectFailure(function() { inc.apply({}, 'x'); }, 'apply "x"');
ok

http://codereview.appspot.com/135054/diff/1/2#newcode1765
Line 1765: assertEquals('add(4, 5)', 5, count);
Maybe test with an array like object.

foo.apply({}, { 0: 'zero', 1: 'one', 3: 'three', length: 5 });

[felix8a, 2009-10-20 04:59:09]

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.

[MikeSamuel, 2009-10-21 19:33:53]

http://codereview.appspot.com/135054/diff/1/3
File src/com/google/caja/cajita.js (right):

http://codereview.appspot.com/135054/diff/1/3#newcode1598
Line 1598: var args = [self].concat(Array.slice(opt_args, 0));
> that doesn't work, because opt_args might be an arguments object,
> so we have to call Array.slice anyway.

Really?  I thought apply could take an array like object, like arguments, and so
calling push indirectly would push the arguments onto the end of the single
element array.

$ var arr = [1,2,3]
$ arr.push.apply(arr, [4, 5, 6])
6
$ arr
1,2,3,4,5,6
$ (function () { arr.push.apply(arr, arguments); })(7, 8, 9)
arr
$ 1,2,3,4,5,6,7,8,9

In section 15.3.4.3 Function.prototype.apply (thisArg, argArray), there is the
following language in a number of places:
    Otherwise, if argArray is neither an array nor an
    arguments object (see 10.1.8), a TypeError exception
    is thrown. If argArray is either an array or an
    arguments object,

[felix8a, 2009-10-21 21:24:37]

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#newcode1598
Line 1598: var args = [self].concat(Array.slice(opt_args, 0));
On 2009/10/21 19:33:54, MikeSamuel wrote:
> > that doesn't work, because opt_args might be an arguments object,
> > so we have to call Array.slice anyway.
> 
> Really?  I thought apply could take an array like object, like arguments, and
so
> calling push indirectly would push the arguments onto the end of the single
> element array.

oh, right.  arguments does works with apply.  I was thinking of somewhere
else where I accidentally had foo.concat(args), which behaves differently
when args is an array and when args is an arguments object.

ok.

[MikeSamuel, 2009-11-05 03:52:35]

LGTM

[felix8a, 2009-11-05 04:56:47]

@r3833