The default bug view has changed. See this FAQ.

Add a JSAPI test for the JSOPTION_VAROBJFIX behavior of JS_EvaluateScript

RESOLVED FIXED in mozilla17

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: bz, Assigned: bz)

Tracking

unspecified
mozilla17
x86
Mac OS X
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:t])

Attachments

(1 attachment)

Because I'm about to write some code that may depend on it.
Created attachment 643603 [details] [diff] [review]
Add a test for the JSOPTION_VAROBJFIX behavior of JS_EvaluateScript.
Attachment #643603 - Flags: review?(jorendorff)
Assignee: general → bzbarsky
Whiteboard: [js:t]
Comment on attachment 643603 [details] [diff] [review]
Add a test for the JSOPTION_VAROBJFIX behavior of JS_EvaluateScript.

Review of attachment 643603 [details] [diff] [review]:
-----------------------------------------------------------------

Nice test. Thanks very much.

::: js/src/jsapi-tests/testJSEvaluateScript.cpp
@@ +15,5 @@
> +
> +    static const char src[] = "var x = 5;";
> +
> +    JS::Value retval;
> +    CHECK(JS_EvaluateScript(cx, obj, src, sizeof(src) - 1, "", 0, &retval));

Nit: `__FILE__, __LINE__,` rather than `"", 0,` please, just for luck.

@@ +33,5 @@
> +
> +    CHECK(JS_EvaluateScript(cx, obj, src2, sizeof(src2) - 1, "", 0, &retval));
> +
> +    hasProp = JS_FALSE;
> +    CHECK(JS_AlreadyHasOwnProperty(cx, obj, "y", &hasProp));

Use plain JS_HasProperty.  The only special thing about JS_AlreadyHasOwnProperty is that it lets you peek at a particular implementation detail (resolve hooks) that isn't relevant here.

@@ +41,5 @@
> +    CHECK(JS_AlreadyHasOwnProperty(cx, global, "y", &hasProp));
> +    CHECK(!hasProp);
> +
> +    // And restore the context options
> +    JS_SetOptions(cx, options);

Oh, don't bother -- for the same reason that you didn't bother deleting that "x" property you created on the global: all this stuff is 100% throwaway. We'll create a fresh cx for the next test.
Attachment #643603 - Flags: review?(jorendorff) → review+
Made those changes, landed http://hg.mozilla.org/integration/mozilla-inbound/rev/f142f32a98a3
Flags: in-testsuite+
Backed out due to bustage

 https://hg.mozilla.org/integration/mozilla-inbound/rev/ad5866c81a03
This patch had nothing to do with the bustage... and it continues after the backout.
Relanded as http://hg.mozilla.org/integration/mozilla-inbound/rev/fb0cac0021c5
https://hg.mozilla.org/mozilla-central/rev/fb0cac0021c5
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.