306617 - The generic function dispatcher should imitate Function.call

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Blake Kaplan (:mrbkap) (inactive)]

Problems with the latest checkin.
-- null shouldn't throw.
-- this may be miscomputed.

Shaver's patch would have done better, except that we still wouldn't have the
desired compatibility. Patch in a minute.

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Use ComputeThis.

Use ComputeThis for total compatibility and safety!

Comment 2

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 194470 [details] [diff] [review]
Use ComputeThis.


>+    /*
>+     * Follow Function.prototype.apply and .call by using the global object as
>+     * the 'this' param if no args.Match Function.protype.call.
>+     */

Random garbage at the end of the comment?

Gotta run, but I'll review for real when I get back.

Comment 3

[Brendan Eich [:brendan]]

Comment on attachment 194470 [details] [diff] [review]
Use ComputeThis.

>+    /*
>+     * We know that argv[0] is valid because JS_DefineFunctions, which is our
>+     * only (indirect) referrer, defined us as requiring at least one argument
>+     * (notice how it passes fs->nargs + 1 as the next-to-last argument to
>+     * JS_DefineFunction).
>+     */
>+    if (JSVAL_IS_PRIMITIVE(argv[0])) {
>         /*
>          * Make sure that this is an object, as required by the generic
>          * functions.

s/an object/& or null/

>+
>+    /*
>+     * Follow Function.prototype.apply and .call by using the global object as
>+     * the 'this' param if no args.Match Function.protype.call.

Trailing junk starting at "Match".

Thanks for fixing this, shaver and I owe you.

/be

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 194470 [details] [diff] [review]
Use ComputeThis.

sr+thanks=shaver

Comment 5

[Brendan Eich [:brendan]]

This needs to go in ASAP.

/be

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into MOZILLA_1_8_BRANCH and trunk.