Bug 141951 – Our bizarre behavior on Arguments::defineOwnProperty should be deliberate rather than a spaghetti incident

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Filip Pizlo]

Patch forthcoming.

[Filip Pizlo]

Created attachment 247199 [details]
WRONG PATCH

[Filip Pizlo]

Created attachment 247200 [details]
the patch

[Filip Pizlo]

Comment on attachment 247200 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247200&action=review

> Source/JavaScriptCore/runtime/Arguments.cpp:308
> +                // ignore the request to change enumerability. We appear to have always do so, in

"*We appear to have always done [sic] so"

I changed this locally.

[Filip Pizlo]

Comment on attachment 247200 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247200&action=review

> Source/JavaScriptCore/tests/stress/arguments-bizarre-behaviour-disable-enumerability.js:25
> +function foo(x) {
> +    Object.defineProperty(arguments, 0, {configurable: true, enumerable: true, writable:true, value:42});
> +    return [x, arguments[0], arguments]
> +}
> +
> +var result = foo(1);
> +
> +if (result[0] !== 42)
> +    throw new Error();
> +
> +if (result[1] !== 42)
> +    throw new Error();
> +
> +if (Array.prototype.join.call(result[2], ",") != "42")
> +    throw new Error();
> +
> +var array = [];
> +for (var s in result[2])
> +    array.push(s);
> +
> +if (array.join(",") != "0")
> +    throw new Error();
> +
> +if (Object.keys(result[2]).join(",") != "0")
> +    throw new Error();

This was meant to be a different test, that sets enumerable to false.  I messed up when copying files around...

[Filip Pizlo]

Created attachment 247201 [details]
the patch

[Benjamin Poulain]

Comment on attachment 247201 [details]
the patch

ok

[Filip Pizlo]

Landed in http://trac.webkit.org/changeset/180564