417387 - js1_5/Regress/regress-68498-001.js FAIL browser only

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Bob Clary [:bc] (inactive)]

BUGNUMBER: 68498
STATUS: Testing that variable statement outside any eval creates a DontDelete property of the global object
FAILED! expected: [reported from test()] Expected value 'false', Actual value 'true' 

regressed approximately sometime between 2008-02-12 04:00 (nightly) and 2008-02-13 00:00

<http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=mozilla%2Fjs%2Fsrc&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-12+04%3A00%3A00&maxdate=2008-02-13+00%3A00%3A00&cvsroot=%2Fcvsroot>

bug 417033 ?

Comment 1

[Brendan Eich [:brendan]]

(In reply to comment #0)
> BUGNUMBER: 68498
> bug 417033 ?

No, that was a DEBUG-only change.

All of these tests are passing for me:

./js1_5/Regress/regress-68498-001.js
./js1_5/Regress/regress-68498-002.js
./js1_5/Regress/regress-68498-003.js
./js1_5/Regress/regress-68498-004.js

What am I missing?

/be

Comment 2

[Brendan Eich [:brendan]]

Passing in the shell, I should say. Are you seeing browser-only failures?

/be

Comment 3

[Bob Clary [:bc] (inactive)]

yes, see summary ^^^, try the url ^^^

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

This is fallout from bug 416931. Given a global variable |var v|, setting |v| will now hit the JS_DefineProperty at the end of nsWindowSH::SetProperty that re-defines |v| with only JSPROP_ENUMERATE, throwing out the fact that |v| was originally (and should continue to be) JSPROP_PERMANENT.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: So, this fixes it

This fix ensures that we propagate any interesting attributes onto our new property.

Comment 6

[Brendan Eich [:brendan]]

Comment on attachment 303482 [details] [diff] [review]
So, this fixes it

>+                            attrs & (JSPROP_ENUMERATE |
>+                                     JSPROP_PERMANENT |
>+                                     JSPROP_EXPORTED |
>+                                     JSPROP_SHARED));

Have my doubts about EXPORTED and SHARED -- can you defend them?

/be

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

That was more of a "they are there" moment. I'll remove them.

Comment 8

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Updated

This also doesn't lose exceptions thrown from under ::JS_DefineProperty and doesn't return a JSBool from a function that returns nsresult!

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 303549 [details] [diff] [review]
Updated

Looks good.

Comment 10

[Johnny Stenback (:jst)]

Fix checked in.

Comment 11

[Bob Clary [:bc] (inactive)]

verified