459405 - Math property flags regressed by bug 376957

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Robert Sayre]

js> var Math = "foo";
typein:8: TypeError: redeclaration of const Math

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Patch

(In reply to bug 465443 comment #19)
> Uh, Math should *not* be const in the global object (not have JSPROP_READONLY).
> Waldo, I thought you backed that out -- it landed as part of the misbegotten
> attempt to lock down standard constructors. Please file and fix ASAP!

I did a little archaeology; I reviewed a patch to make the E4X names mutable again (bug 407323), but I wasn't involved in the reversion in the general case (bug 409252).  A double-check against the original patches says this is the only forgotten re-addition of mutability.  Luckily, people aren't doing stupid things with Math that they did with the other bindings, or we'd have had more bugs filed on this.

Probably want to fix this in 1.9.0.x as well since it exists there...

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 350562 [details] [diff] [review]
Patch

>From: Jeff Walden <[e-mail]>
>
>Bug 459405 - Math property flags regressed by bug 376957.  NOT REVIEWED YET
>
>diff --git a/js/src/jsmath.cpp b/js/src/jsmath.cpp
>--- a/js/src/jsmath.cpp
>+++ b/js/src/jsmath.cpp
>@@ -709,7 +709,8 @@ js_InitMathClass(JSContext *cx, JSObject
>     if (!Math)
>         return NULL;
>     if (!JS_DefineProperty(cx, obj, js_Math_str, OBJECT_TO_JSVAL(Math),
>-                           JS_PropertyStub, JS_PropertyStub, 0))
>+                           JS_PropertyStub, JS_PropertyStub,
>+                           JSPROP_READONLY | JSPROP_PERMANENT))
>         return NULL;

Thanks. Could you also brace the return since the condition is multiline? House style rule not observed in patch for bug 451154, no big deal but may as well fix it now.

/be

Comment 3

[Robert Sayre]

can't really justify blocking, since no one noticed in Fx3. but the patch is here and approved.

Comment 4

[Daniel Veditz [:dveditz]]

echoing comment 3 for the 1.9.0 branch. Will look at the approval request after this has landed on mozilla-central for some nightly testing

Comment 5

[Jeff Walden [:Waldo]]

Landed in 1.9.1:

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/d0a0b167f79b

Comment 6

[Samuel Sidler (old account; do not CC)]

Are you planning on landing this on trunk? It needs to bake a little longer on 1.9.1 before we'll consider it for 1.9.0.

Comment 7

[Jeff Walden [:Waldo]]

I was planning on someone doing a TM merge to m-c that would also take care of that.  That will probably happen fairly soon, depending on m-c openness; I don't think there's such a rush for this to make it into any particular branch release that we need to pull it out of the merge process just yet.

Comment 8

[Jeff Walden [:Waldo]]

...and it just got merged to m-c, so marking fixed finally:

http://hg.mozilla.org/mozilla-central/rev/fb20756f1a0c

Comment 9

[Daniel Veditz [:dveditz]]

Comment on attachment 350562 [details] [diff] [review]
Patch

Approved for 1.9.0.6, a=dveditz for release-drivers.

Comment 10

[Jeff Walden [:Waldo]]

Fixed in 1.9.0.6 as well.  For QA, to verify the fix is correct, load the following URL in a page:

javascript:var%20Math=5;Math

You should see a page containing the number 5, not a blank page.

Comment 11

[Al Billings [:abillings - ex-MoCo]]

Verified for 1.9.0.6 with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.0.6pre) Gecko/2009010504 GranParadiso/3.0.6pre. Checked with the javascript shell too.

Comment 12

[Bob Clary [:bc] (inactive)]

http://hg.mozilla.org/mozilla-central/rev/d269b60ff845
/cvsroot/mozilla/js/tests/ecma_3/Object/regress-459405.js,v  <--  regress-459405.js
initial revision: 1.1

Comment 13

[Bob Clary [:bc] (inactive)]

v 1.9.1, 1.9.2