If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Strict warnings should only be enabled for chrome by default (in debug builds)

RESOLVED FIXED in Firefox 3 alpha7

Status

()

Firefox
General
P2
normal
RESOLVED FIXED
10 years ago
6 years ago

People

(Reporter: Jesse Ruderman, Assigned: mano)

Tracking

({regression})

Trunk
Firefox 3 alpha7
regression
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

10 years ago
Bug 383365 recently enabled strict warnings in debug builds by default, for both chrome and content.

Seeing a zillion strict warnings every time I load almost any web page is annoying.  Strict warnings should only be enabled by default (in debug builds) for chrome.
Bug 248528, comment 12 suggests a solution.
(Reporter)

Updated

10 years ago
Depends on: 248528
You break it, you buy it. Please fix this soon, it's a noise tax on every developer using a DEBUG build.

/be
Assignee: nobody → mano
Created attachment 271584 [details] [diff] [review]
patch
Attachment #271584 - Flags: superreview?(brendan)
Attachment #271584 - Flags: review?(brendan)
Severity: minor → normal
Status: NEW → ASSIGNED
OS: Mac OS X → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → Firefox 3 M7
Possibly nsContentUtils::IsCallerChrome?
Er, ignore me, never mind, forgot what this bug was about.
Comment on attachment 271584 [details] [diff] [review]
patch

>+#ifdef DEBUG
>+  // In debug builds, warnings are always enabled in chrome context
>+  // Note this callback is also called from context's InitClasses thus we don't
>+  // need to enable this directly from InitContext
>+  if ((newDefaultJSOptions & JSOPTION_STRICT) == 0) {
>+    nsIScriptGlobalObject *global = context->GetGlobalObject();
>+    if (global) {
>+      nsresult rv;
>+      nsCOMPtr<nsIDOMChromeWindow> chromeWindow(do_QueryInterface(global, &rv));
>+      if (NS_SUCCEEDED(rv))

Would prefer the one-argument form of do_QueryInterface and a null test of chromeWindow to the rv block local being passed and tested. Simpler source (don't care about compiled code here, but that may be shorter too).

Ok with me as SR given the above nit picked, asking jst to review.

/be
Attachment #271584 - Flags: superreview?(brendan)
Attachment #271584 - Flags: superreview+
Attachment #271584 - Flags: review?(jst)
Attachment #271584 - Flags: review?(brendan)
Comment on attachment 271584 [details] [diff] [review]
patch

+    nsIScriptGlobalObject *global = context->GetGlobalObject();
+    if (global) {
+      nsresult rv;
+      nsCOMPtr<nsIDOMChromeWindow> chromeWindow(do_QueryInterface(global, &rv));
+      if (NS_SUCCEEDED(rv))

Yeah, check chromeWindow here and don't bother with the nsresult, and also remove the if (global) check as do_QueryInterface() does that check for you.

r=jst with that.
Attachment #271584 - Flags: review?(jst) → review+
Created attachment 271634 [details] [diff] [review]
as checked in

mozilla/browser/app/profile/firefox.js 1.186
mozilla/dom/src/base/nsJSEnvironment.cpp 1.331
Attachment #271584 - Attachment is obsolete: true
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Duplicate of this bug: 248528
Flags: in-testsuite-

Updated

6 years ago
Depends on: 737022
You need to log in before you can comment on or make changes to this bug.