248528 - JS strict warnings should be on (at least for chrome) in debug builds

Status: RESOLVED DUPLICATE of bug 385872

(Core :: DOM: Core & HTML, defect)

Description

[Dan Mosedale (:dmosedale, :dmose)]

In order to help keep the code clean.  There may be some other things that need
to happen first, such as tweaking venkman.

Comment 1

[gekacheka]

 ------- Bug 249012 Comment #29 From Darin Fisher  2005-02-23 21:01 PST  -------

what would it take to get JS strict warnings enabled by default in debug builds?
 or would that just be too painful?


------- Bug 249012 Comment #30 From Benjamin Smedberg [:bs] (formerly
[e-mail]) 2005-02-23 21:11 PST -------

Just an #ifdef in all.js -- very simple, in fact. I don't know the pain level.

Comment 2

[Steffen Wilberg]

We need to change this to true:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/modules/libpref/src/init/all.js&rev=3.567&mark=433#433

But #ifdef DEBUG doesn't work, it's not defined in debug builds. What kind of
Makefile magic is required here?

I'd also suggest to set javascript.options.showInConsole to true in Firefox
debug builds:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/app/profile/firefox.js&rev=1.37&mark=217#217

Comment 3

[Darin Fisher]

Have you tried NS_DEBUG ?

Comment 4

[Steffen Wilberg]

NS_DEBUG doesn't work, neither does MOZ_DEBUG.

Comment 5

[Daniel C]

Attachment: Set showInConsole to true for Firefox debug builds and strict to true for all debug builds

all-thunderbird.js already has showInConsole set to true. I didn't change Sunbird or others, but I can add those if needed. This patch is untested, but both changed files have other preprocessing instructions in, so I can't think why it wouldn't work.

Comment 6

[Alex Vincent [:WeirdAl]]

I'm really not so sure this is such a hot idea.  Bear in mind there are 20 times as many strict warnings from web pages as there are from chrome.  As we haven't yet fixed the bug to give error reporting more details yet, I can't say this is really worth it at this point.

Comment 7

[Benjamin Smedberg]

Comment on attachment 207896 [details] [diff] [review]
Set showInConsole to true for Firefox debug builds and strict to true for all debug builds

I agree with AJ: showInConsole is good, options.strict is not because it makes browsing web content ridiculously painful.

Comment 8

[Daniel C]

Attachment: Set showInConsole to true for Firefox debug builds

Yeah, I've changed my mind too, strict isn't useful really, and produces far too many messages.

Comment 9

[Brendan Eich [:brendan]]

(In reply to comment #8)
> Created an attachment (id=208020) [edit]
> Set showInConsole to true for Firefox debug builds
> 
> Yeah, I've changed my mind too, strict isn't useful really, and produces far
> too many messages.

Maybe we really just want chrome windows (not content windows) to enable the JSOPTION_STRICT option on their JSContexts.  Then no web page JS fluff will be reported as lint.  Why not do that?

/be

Comment 10

[Daniel C]

Thanks benjamin. Why exactly is superreview not needed here? I'm not fully familiar with the process...

Also, can someone check this in for me please?

Comment 11

[Daniel C]

Oh. I just tested this, and it doesn't appear to work. I'm not sure why, but I guess I should have believed comment 2...

I built a Firefox debug build, went to about:config and looked for javascript.options.showInConsole and it was set to false.

Comment 12

[Brendan Eich [:brendan]]

(In reply to comment #9)

> Maybe we really just want chrome windows (not content windows) to enable the
> JSOPTION_STRICT option on their JSContexts.  Then no web page JS fluff will be
> reported as lint.  Why not do that?

This is trivial to do, btw.  Did anyone figure out why the existing code to preset JSOPTION_STRICT #ifdef DEBUG isn't working?

http://lxr.mozilla.org/mozilla/source/dom/src/base/nsJSEnvironment.cpp#695

Perhaps because of the way the pref-changed callback is used to bootstrap itself?

Anyway, see

http://lxr.mozilla.org/mozilla/source/dom/src/base/nsJSEnvironment.cpp#1612

where JSOPTION_XML is set by default for chrome windows.  If we just bitwise-or JSOPTION_STRICT here, no #ifdefs, then that ought to do the trick.  I hope.

/be

Comment 13

[Daniel C]

Comment on attachment 208020 [details] [diff] [review]
Set showInConsole to true for Firefox debug builds

Benjamin, do you know why comment 11 and the first part of comment 12 are true (#ifdef DEBUG not working in both cases)?

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Attachment: set showInConsole to true for Firefox debug builds

I don't know of a nicer way to do this, since just adding the -DDEBUG=$(MOZ_DEBUG) makes "#ifdef DEBUG" true, regardless of MOZ_DEBUG's value ("" or "1").

Comment 15

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 221935 [details] [diff] [review]
set showInConsole to true for Firefox debug builds

Actually, I split the showInConsole app setting to bug 337875, since this one is about strict warnings.

Comment 17

[:Gavin Sharp [email: [e-mail]]]

Bug 383365 is Firefox-specific, this bug wasn't, so not really a dupe.

Comment 18

[Asaf Romano (gone)]

Given the fix, duping again.