Closed Bug 408473 Opened 17 years ago Closed 16 years ago

Strict warnings in calendar-dnd-listener.js [redeclaration of function ...]

Categories

(Calendar :: General, defect)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: ssitter, Assigned: gekacheka)

Details

Attachments

(1 file, 1 obsolete file)

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.12pre) Gecko/2007121501 Calendar/0.8pre

Start Sunbird or Thunderbird with Lightning:

Warning: redeclaration of function calendarViewDNDObserver
Source File: chrome://calendar/content/calendar-dnd-listener.js
Line: 478, Column: 4
Source Code:
var calendarViewDNDObserver = new calendarViewDNDObserver();

Warning: redeclaration of function calendarMailButtonDNDObserver
Source File: chrome://calendar/content/calendar-dnd-listener.js
Line: 479, Column: 4
Source Code:
var calendarMailButtonDNDObserver = new calendarMailButtonDNDObserver();

Warning: redeclaration of function calendarCalendarButtonDNDObserver
Source File: chrome://calendar/content/calendar-dnd-listener.js
Line: 480, Column: 4
Source Code:
var calendarCalendarButtonDNDObserver = new calendarCalendarButtonDNDObserver();

Warning: redeclaration of function calendarTaskButtonDNDObserver
Source File: chrome://calendar/content/calendar-dnd-listener.js
Line: 481, Column: 4
Source Code:
var calendarTaskButtonDNDObserver = new calendarTaskButtonDNDObserver();
Another warning after dropping an .ics file into Sunbird:

Warning: assignment to undeclared variable channel
Source File: chrome://calendar/content/calendar-dnd-listener.js
Line: 121
(patch -l -p 1 -i file.patch)

For developers who keep
  user_pref("javascript.options.strict", true);
warning is due to vars with same name as constructor functions,
added by patch for bug 388018.

To differentiate them, this patch capitalizes the constructor functions
(following the Java convention of capitalizing class names).

Also addressed the missing 'var' to declare channel (comment 1).
Attachment #295630 - Flags: review?(michael.buettner)
Flags: wanted-calendar0.8?
Attachment #295630 - Attachment description: v1 patch: capitalize constructor → v1 patch: capitalize constructors
Comment on attachment 295630 [details] [diff] [review]
v1 patch: capitalize constructors

Sorry, can't do the capitalizing. No other classes are capitalized like that. I'd suggest to rename the variables.
Attachment #295630 - Flags: review?(michael.buettner) → review-
> ... No other classes are capitalized like that. 

Most Javascript constructor names are capitalized.  For example, 
  grep 'new ' mozilla/calendar/base/content/*
and you'll see
  ... new Object();
  ... new Array();
  ... new Date();
  ... new RegExp(...);
  ... new FlavourSet();
  ... new OpCompleteListener(...);
one was not:
  ... new dataMigrator(...);

The capitalize-constructors convention distinguishes functions that are constructors (to be called with 'new', not like a normal function).
Just a small selection of initial-small-caps objects:
calAlarmServiceObserver
calAlarmMonitor
calAlarmService
calAttachment
calAttendee
calCalendarManager
calCalendarSearchListener
calCalendarSearchService
Assignee: nobody → gekacheka
(patch -l -p 1 -i file.patch)

Following mvl's examples, differentiate constructor from singletons by shortening constructor prefix from 'calendar' to 'cal'.
Attachment #295630 - Attachment is obsolete: true
Attachment #295646 - Flags: review?(michael.buettner)
Flags: wanted-calendar0.8? → wanted-calendar0.8+
Status: NEW → ASSIGNED
OS: Windows 2000 → All
Hardware: PC → All
Version: Mozilla 1.8 Branch → unspecified
Comment on attachment 295646 [details] [diff] [review]
v2 patch: shorten constructor prefix to lowercase 'cal'

Thanks for taking care. r=mickey.
Attachment #295646 - Flags: review?(michael.buettner) → review+
patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.8
Verified using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.13pre) Gecko/20080213 Lightning/0.8pre (2008021319) Thunderbird/2.0.0.13pre.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.