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

Status: VERIFIED FIXED

(Calendar :: General, defect)

Description

[Stefan Sitter [:ssitter]]

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();

Comment 1

[Stefan Sitter [:ssitter]]

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

Comment 2

[gekacheka]

Attachment: v1 patch: capitalize constructors

(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).

Comment 3

[Michiel van Leeuwen (email: mvl+moz@)]

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.

Comment 4

[gekacheka]

> ... 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).

Comment 5

[Michiel van Leeuwen (email: mvl+moz@)]

Just a small selection of initial-small-caps objects:
calAlarmServiceObserver
calAlarmMonitor
calAlarmService
calAttachment
calAttendee
calCalendarManager
calCalendarSearchListener
calCalendarSearchService

Comment 6

[gekacheka]

Attachment: v2 patch: shorten constructor prefix to lowercase 'cal'

(patch -l -p 1 -i file.patch)

Following mvl's examples, differentiate constructor from singletons by shortening constructor prefix from 'calendar' to 'cal'.

Comment 7

[Michael Büttner (no reviews TFN)]

Comment on attachment 295646 [details] [diff] [review]
v2 patch: shorten constructor prefix to lowercase 'cal'

Thanks for taking care. r=mickey.

Comment 8

[Michael Büttner (no reviews TFN)]

patch checked in on trunk and MOZILLA_1_8_BRANCH

-> FIXED

Comment 9

[Martin Schröder [:mschroeder]]

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.