63484 - Error vs warning

Status: VERIFIED FIXED

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

Description

[Henrik Gemal]

I'm not sure this is a bug or not, but if you have js strict option on and 
encounter a strict warning you'll get in the console:

JavaScript strict warning:
http://blablabla.blabla/ line 548: assignment to undeclared variable itag
Error was suppressed by event handler

Should it say:
Warning was suppressed by event handler or should it say nothing at all, since 
it's only a warning?

Comment 1

[Phil Schwartau]

cc'ing Brendan for his opinion on this -

Comment 2

[Brendan Eich [:brendan]]

jst, can you just add a test !(report->flags & JSREPORT_WARNING) to the
condition around line 171 of dom/src/base/nsJSEnvironment.cpp?

/be

Comment 3

[Phil Schwartau]

Setting default component -

Comment 4

[Johnny Stenback (:jst)]

Brendan, how about this in stead?

Index: dom/src/base/nsJSEnvironment.cpp
===================================================================
RCS file: /cvsroot/mozilla/dom/src/base/nsJSEnvironment.cpp,v
retrieving revision 1.122
diff -u -r1.122 nsJSEnvironment.cpp
--- nsJSEnvironment.cpp 2000/12/07 02:08:21     1.122
+++ nsJSEnvironment.cpp 2000/12/25 00:20:39
@@ -96,7 +96,7 @@
       //send error event first, then proceed
       nsCOMPtr<nsIDocShell> docShell;
       globalObject->GetDocShell(getter_AddRefs(docShell));
-      if (docShell) {
+      if (docShell && !JSREPORT_IS_STRICT(report->flags)) {
         static PRInt32 errorDepth = 0; // Recursion prevention
         errorDepth++;

This makes mozilla *not* call the onerror handler at all for strict warnings,
which IMO is the right thing to do, also see bug 63672 (also fixed by the above
patch).

Comment 5

[Brendan Eich [:brendan]]

The issue isn't just strictness.  If I set javascript.options.werror, I want all
warnings (strict or not) to be treated as errors, which then must be handled by
onerror.  So I think you want JSREPORT_IS_WARNING, not JSREPORT_IS_STRICT (and
thanks for reminding me about those macros -- wonder why they don't take a
reportp arg instead of report->flags -- cc'ing mccabe).

/be

Comment 6

[Johnny Stenback (:jst)]

Brendan, sr=? Pollmann sez r=pollmann.

Index: dom/src/base/nsJSEnvironment.cpp
===================================================================
RCS file: /cvsroot/mozilla/dom/src/base/nsJSEnvironment.cpp,v
retrieving revision 1.126
diff -u -r1.126 nsJSEnvironment.cpp
--- nsJSEnvironment.cpp 2001/02/02 06:05:17     1.126
+++ nsJSEnvironment.cpp 2001/02/02 06:53:26
@@ -180,7 +180,7 @@
   error.AppendWithConversion(": ");
   error.Append(NS_REINTERPRET_CAST(const PRUnichar*, report->ucmessage));
   error.AppendWithConversion("\n");
-  if (status != nsEventStatus_eIgnore)
+  if (status != nsEventStatus_eIgnore && !JSREPORT_IS_WARNING(report->flags))
     error.AppendWithConversion("Error was suppressed by event handler\n");

   char *errorStr = error.ToNewCString();

Comment 7

[Brendan Eich [:brendan]]

sr=[e-mail]

Comment 8

[Johnny Stenback (:jst)]

Fix checked in.

Comment 9

[Prashant Desale]

Verified with 2001-02-06-08-Mtrunk