[FIXr]JS console not showing any JavaScript strict warnings

VERIFIED FIXED in mozilla1.3alpha


16 years ago
9 years ago


(Reporter: schapel, Assigned: bzbarsky)


({regression, testcase})

regression, testcase

Firefox Tracking Flags

(Not tracked)


(Whiteboard: |javascript.options.strict|, |javascript.options.werror| both broken)


(5 attachments)



16 years ago
Reproducible: Always

Steps to reproduce:
1. Execute any JS function that returns a value on some returns and does not
return a value on other returns.

Expected Results: The JavaScript console contains a warning about the function
not always returning a value.

Actual Results: The JavaScript console does not contain a warning about the

I've tested this with build 2002110204 on Windows 98.

Comment 1

16 years ago
Created attachment 104947 [details]
Testcase that calls a function that does not always return a value


16 years ago
Keywords: mozilla1.3, regression, testcase

Comment 2

16 years ago
Confirming, with strict warnings turned _on_.  Build 2002-11-01-04 on Linux.

Comment 3

16 years ago
Here is a JS shell version of the testcase:

function sometimes(x)
  if (x > 0)
    return x;


If I run this in the JS shell via the -f option, I DO get a warning
if I have the strict option -s turned on:

[ ] ./js -s -f ../../tests/test/test/178062.js
178062.js:9: strict warning: function sometimes does not always return a value:
178062.js:9: strict warning: }
178062.js:9: strict warning: ^

But if I start the JS shell independently, with the -s option set,
and load the testcase interactively, I do not get a warning:

[ ] ./js -s
js> load('../../tests/test/test/178062.js');

I do not know if this bug is one for the JS Engine or for the 
embedder, to capture the warning JS may already be providing.

cc'ing Brendan, Mike, Roger -
Assignee: rogerl → khanson

Comment 4

16 years ago
Created attachment 104962 [details]
JS shell version of testcase

Comment 5

16 years ago
Using Mozilla trunk binary 2002110408 on WinNT.

I am not seeing ANY JS strict warnings in the browser, even when I have
"Show JavaScript strict warnings" checked in Edit > Preferences > Debug.

Will attach an HTML testcase below that should generate five different
JS strict warnings in the JS Console, but is generating none -


16 years ago
Summary: JS console should warn when function does not always return a value → JS console not showing any JavaScript strict warnings

Comment 6

16 years ago
Created attachment 105103 [details]
HTML testcase that should generate 5 distinct JS strict warnings

Comment 7

16 years ago
upgrading severity accordingly.....
Severity: normal → major

Comment 8

16 years ago
Created attachment 105104 [details]
JS shell version of the above testcase

Comment 9

16 years ago
*** Bug 178132 has been marked as a duplicate of this bug. ***

Comment 10

16 years ago
From the duplicate bug 178132, reported by bht@actrix.gen.nz:

> Moz 1.2b
> I think since 1.1, the following options are ignored:

> user_pref("javascript.options.strict", true);
> user_pref("javascript.options.werror", false);

> When I use Netscape 7.0 with the same profile then these work fine.
Whiteboard: |javascript.options.strict|, |javascript.options.werror| both broken

Comment 11

16 years ago
FWIW, my Mozilla trunk binary from 2002-07-01 shows all the strict
warnings, whereas my trunk binary from 2002-08-05 does not.

So a regression between those dates. Again, not sure if this is due to
a bug in JS Engine or in the embedding of JS Engine. I do know that
the current JS shell shows all the strict warnings if I load testcases
via the -s and -f options as described in Comment #3 above -

Comment 12

16 years ago
This doesn't look to be a JS Engine bug; reassigning to
the JavaScript Console component and to danm for a look -
Assignee: khanson → danm
Component: JavaScript Engine → JavaScript Console
Did the pref code change incompatibly?  The nsJSContext::JSOptionChangedCallback
in dom/src/base/nsJSEnvironment.cpp is registered for "javascript.options."
using nsIPref::RegisterCallback.


Comment 14

16 years ago
if the pref code changed and broke this, then it is a bug - but nothing has
changed in that area in quite a while, as far as I know. 

Comment 15

16 years ago
This works in Linux build 2002-07-24-21 and is broken in 2002-07-25-21.  I'm
still coaxing the checkin list out of Bonsai....

Comment 16

16 years ago
More fun.  The warnings "work" in a debug build (requires a reload after the
initial load), but not opt.

The culprit is rev 1.178 of nsJSEnvironment.cpp.  If I switch it back to using
the CID, the warnings return... Why is do_GetService() failing with the contract

Comment 17

16 years ago
Boris: I see warnings in debug builds too. But I can't reproduce your results
with restoring warnings by going back to rev 1.177's kPrefServiceCID.

Comment 18

16 years ago
Hm... odd....

as a note, with the contractid the error I get is NS_ERROR_NO_INTERFACE... The
failing QI is the one in nsComponentManagerImpl::GetServiceByContractID

Comment 19

16 years ago
Scratch comment 17. My mistake. Confirming, then: kPrefServiceCID does restore
warnings to the console in my Windows trunk build (if I remember to turn on the
strict pref!)

Comment 20

16 years ago
Oh, boy.

The old code:

static NS_DEFINE_IID(kPrefServiceCID, NS_PREF_CID);                             

The new code:

nsCOMPtr<nsIPref> prefs(do_GetService(NS_PREFSERVICE_CONTRACTID, &rv));     

If you look at
you see that these CID and contractid belong to _different_ classes and that the
one that implements NS_PREFSERVICE_CONTRACTID does _not_ implement nsIPref. 
alecf, this is all yours.  :)

So possible fixes:

2)  Change to using nsIPrefService instead of nsIPref (preferred, since nsIPref
    is deprecated)

As long as you're at this, fix
as well?

Comment 21

16 years ago
Yes, that's the problem. Nice, Boris! Slightly off topic, I've run into this
problem before, where the debug build has a curious tolerance for slightly wrong
CIDs that the optimized build lacks. Anybody know why?

Comment 22

16 years ago
  Oh. Strict is on by default in debug builds, isn't it? Ignore me once again.
My brain and I, we're taking the rest of the night off. Still, I swear I'm not
making that up about the CIDs.
  Maybe I shouldn't swear.

Comment 23

16 years ago
Created attachment 105154 [details] [diff] [review]
Oh, well

I can't get AddObserver() from inside the constructor to not crash, so let's go
with the simple fix....

Comment 24

16 years ago
I just want to voice my preference for switching to nsIPrefService - its the
right way to go. In addition, you can use nsIPrefBranchInternal to get to the
c-style callbacks mechanism

Comment 25

16 years ago
Comment on attachment 105154 [details] [diff] [review]
Oh, well

however, I just noticed this patch, and it sure is easy. sr=alecf for the
simple fix shown here :)
Attachment #105154 - Flags: superreview+

Comment 26

16 years ago
I agree that we should try to switch to nsIPrefService... but I'd like to land
that simple patch for 1.2, if possible.

Like I said, I tried adding the JSContext as a pref observer in the constructor
and that failed miserably (we're talking crash with gdb just being able to tell
me that Moz had jumped to memory gdb could not read and hence no stack).  I see
no way to get the old-style callbacks from nsIPrefBranchInternal...

So anyone?  r= on that patch?  ;)
Comment on attachment 105154 [details] [diff] [review]
Oh, well

Thanks, bz.  Can we get this into 1.2final?  Pls. mail drivers.

Attachment #105154 - Flags: review+

Comment 28

16 years ago
Comment on attachment 105154 [details] [diff] [review]
Oh, well

a=asa for checkin to the 1.2 branch (on behalf of drivers)
Attachment #105154 - Flags: approval+

Comment 29

16 years ago
I should take this or something...  Waiting for the branch to reopen to check
this in, though...
Assignee: danm → bzbarsky
Priority: -- → P1
Summary: JS console not showing any JavaScript strict warnings → [FIXr]JS console not showing any JavaScript strict warnings
Target Milestone: --- → mozilla1.3alpha

Comment 30

16 years ago
Fixed, trunk and 1.2 branch.
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 31

16 years ago
New bug# for the preferred (comment 20, comment 24, comment 26) fix?

Comment 32

16 years ago
None so far... wanna file it?  ;)

We should actually clean up usage of this nsIPref thing in general... it's only
been deprecated for about 2 years....

Comment 33

16 years ago
Verified FIXED using trunk and branch builds 20021117xx on Windows.
When the |javascript.options.strict| pref is set, the HTML testcase
in Comment #6 now generates all 5 strict warnings in the JS Console -
Product: Core → SeaMonkey


9 years ago
Component: Error Console → Error Console
Product: SeaMonkey → Core Graveyard
You need to log in before you can comment on or make changes to this bug.