Closed Bug 321413 Opened 14 years ago Closed 14 years ago

gDebugEnabled is not defined

Categories

(Calendar :: Sunbird Only, defect, major)

x86
Windows XP
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: robin.edrenius, Assigned: robin.edrenius)

References

Details

(Keywords: regression)

Attachments

(2 files)

Can't open Extension- or thememanager since gDebugEnabled is not defined.

Error: gDebugEnabled is not defined
Source File: chrome://calendar/content/applicationUtil.js
Line: 395
This patch defines gDebugEnabled
Assignee: mostafah → robin.edrenius
Status: NEW → ASSIGNED
Attachment #206757 - Flags: first-review?(jminta)
Comment on attachment 206757 [details] [diff] [review]
definde gDebugEnabled

http://lxr.mozilla.org/mozilla/search?string=gDebugEnabled

gDebugEnabled was originally defined in weekView.js and it's only real function is to hide some dump() spew.  Given that the only people that will really ever see these dumps know how to hack the code and remove them if they want, I'd rather just see gDebugEnabled disappear entirely.

Note: This is slightly a stylistic thing.  I'm personally against global variables in almost all cases.  They have the minor performance hit of increasing our scope list  for each variable/function name lookup and little use (perhaps for caching XPCOM services they may be useful).  If someone has a good argument for keeping this variable around, I'll be happy to entertain it.  You should at least define gDebugEnabled to 'false' in that case though.
Attachment #206757 - Flags: first-review?(jminta) → first-review-
Upping severity.  It blocks anyone from working on extensions or themes.

A better fix might be to remove the debug() and debugVar() calls, they probably shouldn't be in checked-in code.
Severity: normal → major
(patch -l -p 2 -i file.patch)

approach 2: remove calls to debug (from active files)
Attachment #206768 - Flags: first-review?(jminta)
Comment on attachment 206768 [details] [diff] [review]
approach 2: remove debug calls from applicationUtils, eventDialog, calAlarmMonitor

Looks good.  Thanks for the patch.  We might want to look into getting most of these functions out of try/catch too, since if they throw it's probably a serious problem.
Attachment #206768 - Flags: first-review?(jminta) → first-review+
patch checked in.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.