50645 - "javascript.options.strict" should be true

Status: VERIFIED FIXED

(SeaMonkey :: General, defect, P3)

Description

[Henrik Gemal]

The pref "javascript.options.strict" should be true while Mozilla 1 and Netscape 
6 is still under development. This would benefit a lot of developers into fixing 
loads of javascript bugs.
The pref could then be turned of when we ship Mozilla 1.0 and Netscape 6.0, so 
that the average user dont see all these developer bugs(warnings).

Comment 1

[Phil Schwartau]

A question of the browser embedding of the engine, not the engine itself.
Reassigning to Browser-General for proper consideration -

Comment 2

[Phil Schwartau]

See also the related bug 50291 -

Comment 3

[Asa Dotzler [:asa]]

adding brendan.  looking for more input.

Comment 4

[Brendan Eich [:brendan]]

Developers want this on, generally -- end users do not.  Thoughts: have it on 
for DEBUG builds; have it on by default in Mozilla binaries but not in Netscape 
commercial builds; have it off in Mozilla milestone builds too (treat them like 
commercial builds).

This bug should go to someone who can preset the pref.  I'm not sure who that is 
right now, possibly it's a don melton group thang.

/be

Comment 5

[Henrik Gemal]

So who should it be reassigned or cc to?

Comment 6

[Brendan Eich [:brendan]]

Ben, can you look at this or bounce to someone else in gramps's group?  Thanks,

/be

Comment 7

[Daniel Veditz [:dveditz]]

This may be mine, not that it really matters who does it. So, Brendan, are you 
officially approving this change?

Comment 8

[Brendan Eich [:brendan]]

For DEBUG builds, a=[e-mail]

/be

Comment 9

[Daniel Veditz [:dveditz]]

Easy enough to set a default pref differently between Mozilla and N6, but not 
as easy to differentiate debug and release XP (on Win and Unix I guess it would 
be easy enough to echo the pref onto the end of all.js)

Instead, how 'bout skipping the pref and setting the default in 
nsJSEnvironment.cpp.  If someone has set their own pref it'll overwrite this 
value. Will attach patch.

Comment 10

[Daniel Veditz [:dveditz]]

Attachment: diff of proposed fix

Comment 11

[Brendan Eich [:brendan]]

Thanks, good idea -- how about using #else so mDefaultJSOptions is set to 0 only
if DEBUG isn't defined?  Do that and r,a=[e-mail].

/be

Comment 12

[Daniel Veditz [:dveditz]]

That was my first thought. I changed to this because setting it twice in 
DEBUG isn't going to hurt anything and it made the non-DEBUG code easier to 
read.  But I'll happily switch it back.

I was afflicted with jillions of new asserts last night and couldn't tell if it 
was related to the jillions of new javascript strict warnings. Need to make 
sure this is not the cause of those asserts before checking in.

Comment 13

[Daniel Veditz [:dveditz]]

The assertions are definitely due to the strict option. There is no way I'm 
checking this in, I'd get lynched.

The assertion is at mozilla/rdf/content/src/nsXULPrototypeDocument.cpp line 
523, looks like nsXULPDGlobalObject::GetDocShell() was not intended to be 
reached (hard to tell with the helpful message "waaah!"). CC'ing waterson who 
appears to have written this code.

Looks like NS_ScriptErrorReporter() handles the lack of a docshell just fine, 
maybe the NS_NOTREACHED() could be changed to a NS_WARNING() if it's important 
at all.

Comment 14

[Brendan Eich [:brendan]]

Waterson's on the road (back Thursday, IIRC), maybe he should get this bug 
temporarily, then back to dveditz to do the deed?

/be

Comment 15

[Daniel Veditz [:dveditz]]

Fix checked in. In debug builds (not optimized) javascript strict will be on. A 
pref will override this, so developers not working on js who are hugely annoyed 
by the thousands of new warnings can set javascript.options.strict to false.

People developing chrome using optimized/release builds will have to manually 
set their own pref to get strict mode, but that seemed a better choice than 
having to remember to unset the option in all.js for milestones.

waterson,brendan: I changed the NS_NOTREACHED() to a NS_WARNING() in 
rdf/content/src/nsXULPrototypeDocument.cpp to avoid a lynching. The two of you 
can argue whether nsJSEnvironment's error reporter shouldn't be trying to call 
this, if this function should be implemented after all, or if this is a symptom 
of something really screwed up that needs to be tracked down and fixed.

Comment 16

[Henrik Gemal]

Thanx for fixing this...
A lot of javascript warnings has already been fixed!