60649 - javascript strict warnings in AccountWizard.js

Status: RESOLVED FIXED

(MailNews Core :: Account Manager, defect)

Description

[Henrik Gemal]

chrome://messenger/content/AccountWizard.js line 62: trailing comma is not legal
 in ECMA-262 object initializers

JavaScript strict warning:
chrome://messenger/content/AccountWizard.js line 321: redeclaration of var 
server

JavaScript strict warning:
chrome://messenger/content/AccountWizard.js line 484: function 
verifyLocalFoldersAccount does not always return a value

JavaScript strict warning:
chrome://messenger/content/AccountWizard.js line 486: function 
verifyLocalFoldersAccount does not always return a value

JavaScript strict warning:
chrome://messenger/content/AccountWizard.js line 91: assignment to undeclared 
variable wizardManager

JavaScript strict warning:
chrome://messenger/content/AccountWizard.js line 679: reference to undefined 
property pageData.accounttype

Comment 1

[(not reading, please use [e-mail] instead)]

mass re-assign of account manager bugs to racham.

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

When you fix this, can someone verify, thanks!

Comment 3

[Håkan Waara]

I found that on this line:
http://lxr.mozilla.org/seamonkey/source/mailnews/base/prefs/resources/content/AccountWizard.js#358
the variable 'server' is created two times, and assigned different things. This
might be very bad.

Comment 4

[Henrik Gemal]

adding steps to reproduce:

1) Set user_pref("javascript.options.strict", true); in your prefs.js file
2) Edit -> "Mail/News Account Settings" -> New Account

Comment 5

[Henrik Gemal]

If I produces patched for this bug can I then trust that someone will help me to
get r, sr and checkin? I produced quite a few patches that just still sits
around and are now out of date...:(

Comment 6

[[e-mail] (gone - use [e-mail] instead)]

function createAccount(accountData)
{

    var server = accountData.incomingServer;
    dump("am.createIncomingServer(" + server.username + "," +
                                      server.hostName + "," +
                                      server.type + ")\n");
    var server = am.createIncomingServer(server.username,
                                         server.hostName,
                                         server.type);

Yeah, that is strange code, indeed.  Bhuvan, can you add insight?

Comment 7

[(not reading, please use [e-mail] instead)]

mass re-assign.

Comment 8

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b4) Gecko/20050728 SeaMonkey/1.0a]
(nightly) (W98SE)

Opening the account creation wizard:
{{
Warning: function setupCopiesAndFoldersServer does not always return a value
Source File: chrome://messenger/content/AccountWizard.js
Line: 631, Column: 14
Source Code:
        return;

Warning: function setupCopiesAndFoldersServer does not always return a value
Source File: chrome://messenger/content/AccountWizard.js
Line: 642, Column: 16
Source Code:
    return false;

Warning: function setupCopiesAndFoldersServer does not always return a value
Source File: chrome://messenger/content/AccountWizard.js
Line: 644, Column: 13
Source Code:
  return true;
}}

Trivial fix should be
{{
-631         return;
+631         return false;
}}
I guess.

Comment 9

[Serge Gautherie (:sgautherie)]

(In reply to comment #8)
> [Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b4) Gecko/20050728
> SeaMonkey/1.0a]
> (nightly) (W98SE)
> 
> Opening the account creation wizard:
> {{
> Warning: function setupCopiesAndFoldersServer does not always return a value

This is still present in
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.0.2) Gecko/20060408 SeaMonkey/1.0.1] (nightly) (W98SE)
.

Comment 10

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)
> chrome://messenger/content/AccountWizard.js line 62: trailing comma is not
> legal
>  in ECMA-262 object initializers

Fixed by
"1.96	racham%netscape.com	2002-02-06 00:18	 	Fixing bug 101873. Adapting new <wizard/> for mailnews accountwizard. This fix introduces accessiblity via proper key handling and provides platform specific appearance. r=ducarroz, sr=mscott"

> JavaScript strict warning:
> chrome://messenger/content/AccountWizard.js line 321: redeclaration of var 
> server

This is blamed to
"1.40 	alecf%netscape.com	2000-02-07 21:56	 	update account wizard to run and fill in the first "invalid" account and start adding framework for pre-filled account data (such as from ISPs)
r=sspitzer"

Not sure what happened (or not) in bug 219509, but this was never fixed;
even if the JS Engine does not report this (type of) warning nowadays.

> verifyLocalFoldersAccount does not always return a value

|verifyLocalFoldersAccount| was the previous name of |setupCopiesAndFoldersServer()| (code),
which was fixed by
"1.128	bugzilla%standard8.demon.co.uk	2005-08-27 10:48	 	Fix bug 216245 subscribe/account wizard for newsgroups forgets account name. r=mnyromyr,sr=bienvenu"
and
"1.127.8.1	bugzilla%standard8.demon.co.uk	2006-04-14 01:57	 	Bug 216245 Subscribe/account wizard for newsgroups forgets account name. r=mnyromyr,sr=bienvenu for main patch, r/sr=bienvenu for regression fix (included in checkin) a=mscott for both patches"

> JavaScript strict warning:
> chrome://messenger/content/AccountWizard.js line 91: assignment to undeclared 
> variable wizardManager

Fixed by
"1.96	racham%netscape.com	2002-02-06 00:18	 	Fixing bug 101873. Adapting new <wizard/> for mailnews accountwizard. This fix introduces accessiblity via proper key handling and provides platform specific appearance. r=ducarroz, sr=mscott"

> JavaScript strict warning:
> chrome://messenger/content/AccountWizard.js line 679: reference to undefined 
> property pageData.accounttype

Fixed by
"1.96	racham%netscape.com	2002-02-06 00:18	 	Fixing bug 101873. Adapting new <wizard/> for mailnews accountwizard. This fix introduces accessiblity via proper key handling and provides platform specific appearance. r=ducarroz, sr=mscott"

Comment 11

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) <AccountWizard.js>

(In reply to comment #10)
> This is blamed to
> "1.40   alecf%netscape.com      2000-02-07 21:56

The second |var| was already there;
the first one was added, without removing the second one.
To reuse the variable is what is actually wanted.

Comment 12

[David :Bienvenu]

Comment on attachment 332627 [details] [diff] [review]
(Av1) <AccountWizard.js>

I think server means two separate things here, so one of the vars should probably change its name.

Comment 13

[Serge Gautherie (:sgautherie)]

(In reply to comment #12)
> (From update of attachment 332627 [details] [diff] [review])
> I think server means two separate things here, so one of the vars should
> probably change its name.

What about my comment 11 ?
See
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=AccountWizard.js&branch=&root=/cvsroot&subdir=mozilla/mailnews/base/prefs/resources/content&command=DIFF_FRAMESET&rev1=1.39&rev2=1.40

Comment 14

[Serge Gautherie (:sgautherie)]

Attachment: (Av1a) <AccountWizard.js>

Av1, with additional documentation and some rewrites.

(In reply to comment #12)
> I think server means two separate things here, so one of the vars should
> probably change its name.

Actually, iiuc, |server| does mean the "same" thing all along...

Comment 15

[Serge Gautherie (:sgautherie)]

David, ping ?

Comment 16

[Serge Gautherie (:sgautherie)]

bienvenu, ping for r+sr.

Comment 17

[Robert Kaiser]

This bug is open but targeted for seamonkey2.0a1, which has been released a long time ago. Please set the target milestone to an appropriate value, "---" if it has no specific target.

Comment 18

[David :Bienvenu]

Comment on attachment 345834 [details] [diff] [review]
(Av1a) <AccountWizard.js>

bit-rotted, sorry.

Comment 19

[Serge Gautherie (:sgautherie)]

Attachment: (Av1b) Remove duplicated |var|, Add documentation, Little code reformatting [Checkin: Comment 20]

Av1a, with comment 18 suggestion(s):
the file just moved one directory up.

Comment 20

[Serge Gautherie (:sgautherie)]

Comment on attachment 426806 [details] [diff] [review]
(Av1b) Remove duplicated |var|, Add documentation, Little code reformatting
[Checkin: Comment 20]


http://hg.mozilla.org/comm-central/rev/2a58f4a3497b