Bob Clary [:bc] (inactive) Reporter
	Description • 17 years ago

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 ?

	Brendan Eich [:brendan]
	Comment 1 • 17 years ago

(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

	Brendan Eich [:brendan]
	Comment 2 • 17 years ago

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

/be

	Bob Clary [:bc] (inactive) Reporter
	Comment 3 • 17 years ago

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

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 4 • 17 years ago

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.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 5 • 17 years ago

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

	Brendan Eich [:brendan]
	Comment 6 • 17 years ago

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

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 7 • 17 years ago

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

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 8 • 17 years ago

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

	Johnny Stenback (:jst)
	Comment 9 • 17 years ago

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

Looks good.

	Johnny Stenback (:jst)
	Comment 10 • 17 years ago

Fix checked in.

	Bob Clary [:bc] (inactive) Reporter
	Comment 11 • 17 years ago

verified