Closed Bug 50645 Opened 24 years ago Closed 24 years ago

"javascript.options.strict" should be true

Categories

(SeaMonkey :: General, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bugzilla, Assigned: dveditz)

Details

Attachments

(1 file)

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).
A question of the browser embedding of the engine, not the engine itself. Reassigning to Browser-General for proper consideration -
Assignee: rogerl → asa
Component: Javascript Engine → Browser-General
QA Contact: pschwartau → doronr
See also the related bug 50291 -
adding brendan. looking for more input.
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
So who should it be reassigned or cc to?
Ben, can you look at this or bounce to someone else in gramps's group? Thanks, /be
Assignee: asa → ben
This may be mine, not that it really matters who does it. So, Brendan, are you officially approving this change?
For DEBUG builds, a=brendan@mozilla.org /be
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.
Assignee: ben → dveditz
Attached patch diff of proposed fix (deleted) — Splinter Review
Thanks, good idea -- how about using #else so mDefaultJSOptions is set to 0 only if DEBUG isn't defined? Do that and r,a=brendan@mozilla.org. /be
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.
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.
Status: NEW → ASSIGNED
Waterson's on the road (back Thursday, IIRC), maybe he should get this bug temporarily, then back to dveditz to do the deed? /be
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.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Thanx for fixing this... A lot of javascript warnings has already been fixed!
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: