Last Comment Bug 256447 - In <mailWindowOverlay.js>, "Warning: redeclaration of var i" and "Warning: assignment to undeclared variable pop3Server"
: In <mailWindowOverlay.js>, "Warning: redeclaration of var i" and "Warning: as...
Status: RESOLVED FIXED
: fixed-seamonkey1.1, fixed1.8.1.2, regression
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Message Display (show other bugs)
: Trunk
: All All
: -- minor (vote)
: mozilla1.8beta1
Assigned To: Serge Gautherie (:sgautherie)
:
:
Mentors:
: 325098 (view as bug list)
Depends on: 260447 266066
Blocks: 303545
  Show dependency treegraph
 
Reported: 2004-08-21 14:32 PDT by Serge Gautherie (:sgautherie)
Modified: 2007-10-27 10:41 PDT (History)
9 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
(Av1) <mailwindowoverlay.js> (SeaMonkey part) (3.38 KB, patch)
2004-08-21 15:09 PDT, Serge Gautherie (:sgautherie)
mozilla: review+
mozilla: superreview+
Details | Diff | Splinter Review
(Av2) <mailWindowOverlay.js> (SeaMonkey part) [Checked in: Comment 13] (4.34 KB, patch)
2004-11-16 23:52 PST, Serge Gautherie (:sgautherie)
mozilla: review+
mozilla: superreview+
Details | Diff | Splinter Review
(Bv1-TB) <mailWindowOverlay.js> [Checkin: Comment 33] (4.28 KB, patch)
2005-01-05 16:06 PST, Serge Gautherie (:sgautherie)
mozilla: review+
mscott: superreview+
dveditz: approval1.8.1.2+
dveditz: approval1.8.1.4-
Details | Diff | Splinter Review
(Cv1) <mailWindowOverlay.js> (3.97 KB, patch)
2005-03-10 10:15 PST, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
(Cv1a) <mailWindowOverlay.js> [Checkins: Comment 28 & 35 & 38] (3.28 KB, patch)
2006-04-06 16:40 PDT, Serge Gautherie (:sgautherie)
mozilla: review+
mscott: superreview+
dveditz: approval1.8.1.1-
jaymoz: approval1.8.1.2+
kairo: approval‑seamonkey1.1+
Details | Diff | Splinter Review

Description Serge Gautherie (:sgautherie) 2004-08-21 14:32:51 PDT
[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a3) Gecko/20040817] (release) (W98SE)

{{
Warning: redeclaration of var i
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 769, Column: 17
Source Code:
        for (var i = 0; i < pop3DownloadServersArray.length; i++)
}}

when opening MailNews.
Comment 1 Serge Gautherie (:sgautherie) 2004-08-21 14:35:43 PDT
There is another warning:
{{
Warning: assignment to undeclared variable pop3Server
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 746
}}
Comment 2 Serge Gautherie (:sgautherie) 2004-08-21 15:09:23 PDT
Created attachment 156703 [details] [diff] [review]
(Av1) <mailwindowoverlay.js> (SeaMonkey part)

2 warning fixes, plus 2 little "optimizations".
Comment 3 neil@parkwaycc.co.uk 2004-08-21 15:53:48 PDT
Comment on attachment 156703 [details] [diff] [review]
(Av1) <mailwindowoverlay.js> (SeaMonkey part)

I shouldn't review this because I don't have any deferred pop3 accounts, try
bienvenu as it's his code. He might give you sr too as it's a small patch, if
not you can ask me after you have his review.

That said, in my opinion you should leave the indenting at 4 spaces, and the
only "optimization" I think you should keep is the [index].AppendElement one.
Comment 4 Serge Gautherie (:sgautherie) 2004-08-21 16:07:39 PDT
Comment on attachment 156703 [details] [diff] [review]
(Av1) <mailwindowoverlay.js> (SeaMonkey part)

David:
Neil wrote
{{
... in my opinion you should leave the indenting at 4 spaces, and the
only "optimization" I think you should keep is the [index].AppendElement one.
}}
If you (likely) confirm this, I'll prepare a new patch for checkin...
Comment 5 David :Bienvenu 2004-10-04 08:00:00 PDT
Comment on attachment 156703 [details] [diff] [review]
(Av1) <mailwindowoverlay.js> (SeaMonkey part)

can you just remove this, instead of commenting out the already commented out
line?

+/*
+	 else
+	 {
+	   //dump(currentServer.serverURI + "...skipping, already opened\n");
+	 }
+*/

If you want, I can make that change when I check in...
Comment 6 Serge Gautherie (:sgautherie) 2004-10-04 08:08:20 PDT
Comment on attachment 156703 [details] [diff] [review]
(Av1) <mailwindowoverlay.js> (SeaMonkey part)

(In reply to comment #5)
> (From update of attachment 156703 [details] [diff] [review])
> can you just remove this, instead of commenting out the already commented out
> line?

That was the idea ;-)

> If you want, I can make that change when I check in...

Your Edit+Checkin would be fine with me,
(unless you prefer that I edit it myself, which I could do later today.)
Comment 7 Serge Gautherie (:sgautherie) 2004-11-16 22:44:34 PST
'blocking1.8a5=?': No big deal, but we need an Edit+Checkin only, see comment 6.
Comment 8 Serge Gautherie (:sgautherie) 2004-11-16 23:51:47 PST
New additional warnings:
{{
Warning: redeclaration of var i
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 1969, Column: 13
Source Code:
    for (var i = 0; i < pop3DownloadServersArray.length; i++)

Warning: reference to undefined property defaultServer.msgFolder
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 749

Warning: assignment to undeclared variable pop3Server
Source File: chrome://messenger/content/mailWindowOverlay.js
Line: 1919
}}

|Warning: reference to undefined property defaultServer.msgFolder|:
I don't know how to fix this one: helpwanted.
Comment 9 Serge Gautherie (:sgautherie) 2004-11-16 23:52:58 PST
Created attachment 166208 [details] [diff] [review]
(Av2) <mailWindowOverlay.js> (SeaMonkey part)
[Checked in: Comment 13]
Comment 10 Serge Gautherie (:sgautherie) 2004-11-16 23:53:55 PST
Comment on attachment 166208 [details] [diff] [review]
(Av2) <mailWindowOverlay.js> (SeaMonkey part)
[Checked in: Comment 13]

Can you check it in too ?
Comment 11 Serge Gautherie (:sgautherie) 2004-12-19 18:02:16 PST
David:
Would you have time to r+sr this more than 1 month old patch ? Thanks.
Comment 12 Serge Gautherie (:sgautherie) 2004-12-19 18:07:38 PST
+ (K) Regression: see bug 270343 comment 6.
Comment 13 Serge Gautherie (:sgautherie) 2005-01-05 15:39:47 PST
Comment on attachment 166208 [details] [diff] [review]
(Av2) <mailWindowOverlay.js> (SeaMonkey part)
[Checked in: Comment 13]


Check in: { 2005-01-04 16:51	neil%parkwaycc.co.uk	mozilla/ mailnews/
base/ resources/ content/ mailWindowOverlay.js	     1.212   9/13    Bug 256477
Fix JavaScript strict warnings p=gautheri@noos.fr r+sr=bienvenu }
(nit: wrong bug number ;-<)
Comment 14 Serge Gautherie (:sgautherie) 2005-01-05 16:06:48 PST
Created attachment 170411 [details] [diff] [review]
(Bv1-TB) <mailWindowOverlay.js>
[Checkin: Comment 33]

Same as Av2-SM.

I don't use TB: Could you test/review/check in this patch ? Thanks.
Comment 15 Serge Gautherie (:sgautherie) 2005-02-07 14:32:32 PST
(In reply to comment #8)
> |Warning: reference to undefined property defaultServer.msgFolder|:

Fixed by bug 270343 patch Cv1.
Comment 16 Serge Gautherie (:sgautherie) 2005-02-07 14:35:24 PST
Comment on attachment 170411 [details] [diff] [review]
(Bv1-TB) <mailWindowOverlay.js>
[Checkin: Comment 33]


No review from <mscott@mozilla.org> since "2005-01-05" :-(
Comment 17 Serge Gautherie (:sgautherie) 2005-03-10 09:36:00 PST
Comment on attachment 170411 [details] [diff] [review]
(Bv1-TB) <mailWindowOverlay.js>
[Checkin: Comment 33]

[Mozilla Thunderbird, version 1.0.1 (20050309)] (nightly) (W98SE)

No review from <bienvenu@nventure.com> aither since "2005-02-07" :-(

Could you (super-)review/check in this patch ? Thanks.
Comment 18 Serge Gautherie (:sgautherie) 2005-03-10 09:38:50 PST
(In reply to comment #17)
> (From update of attachment 170411 [details] [diff] [review] [edit])
> [Mozilla Thunderbird, version 1.0.1 (20050309)] (nightly) (W98SE)

Brendan, here too, I'm surprised not to get the warning about |pop3Server|...
Comment 19 Brendan Eich [:brendan] 2005-03-10 10:13:42 PST
Is the question "why doesn't the undeclared global variable set at
http://lxr.mozilla.org/mozilla/source/mail/base/content/mailWindowOverlay.js#1988
in CoalesceGetMsgsForPop3ServersByDestFolder cause a strict warning?"

I don't know.  It will warn only the first time per chrome window, after which
the global var exists and reassignment to it won't cause any warning.  Someone
needs to debug.

Serge, I know I've said this before: don't nominate warning fixes and cleanups
at the last minute for releases, especially for security releases.  Fix them in
alpha milestones only.

/be
Comment 20 Serge Gautherie (:sgautherie) 2005-03-10 10:15:01 PST
Created attachment 177026 [details] [diff] [review]
(Cv1) <mailWindowOverlay.js>

[Mozilla Thunderbird, version 1.0.1 (20050309)] (nightly) (W98SE)

1 fix, plus a little more.

Could you (super-)review/check in this patch ? Thanks.
Comment 21 Serge Gautherie (:sgautherie) 2005-03-10 10:15:37 PST
Comment on attachment 177026 [details] [diff] [review]
(Cv1) <mailWindowOverlay.js>

This one came from
{{
1.29	scott%scott-macgregor.org	2003-11-30 12:36		Add a
try/catch around a method call in HandleMDNResponse to catch a weird JS error I
have not tracked down yet.
}}
Comment 22 Serge Gautherie (:sgautherie) 2005-03-10 10:29:09 PST
(In reply to comment #19)
> Is the question "why doesn't the undeclared global variable set at
> http://lxr.mozilla.org/mozilla/source/mail/base/content/mailWindowOverlay.js#1988
> in CoalesceGetMsgsForPop3ServersByDestFolder cause a strict warning?"

Yes.

> I don't know.  It will warn only the first time per chrome window, after which
> the global var exists and reassignment to it won't cause any warning.  Someone
> needs to debug.

I'm unskilled for this :-<
Comment 23 Serge Gautherie (:sgautherie) 2006-04-06 16:40:49 PDT
Created attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]

Cv1, unbitrotted.

(Hoping to get a r/sr before next year: still wondering how others achieve to fix the same bugs I already have patch waiting :-/)
Comment 24 Wayne Mery (:wsmwk, NI for questions) 2006-11-11 19:17:53 PST
Serge, how does this relate to bug 260447?  you set the dependency
Could David do the r and mscott the sr?
Comment 25 Serge Gautherie (:sgautherie) 2006-11-11 21:13:35 PST
(In reply to comment #24)
> Serge, how does this relate to bug 260447?  you set the dependency

Looking back at this after so long:
that bug seems related to, at least, comment 15 and bug 270343 comment 20;
it's where (some of) the regression/warnings comes from.

> Could David do the r and mscott the sr?

Yes, any r/sr-er would have my thanks...
Comment 26 David :Bienvenu 2006-11-12 04:46:34 PST
Comment on attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]

is this the right patch? I don't see how it could have to do with any warnings...
Comment 27 Serge Gautherie (:sgautherie) 2006-11-12 18:18:21 PST
Comment on attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]

(In reply to comment #26)
> is this the right patch?

Yes:
in the Cv1 patch, the fix was
[
-  } catch (ex) { return 0;}
+  } catch (ex) { return; }
]
but in Cv1a only the little rewrite part stands: no more warning issue.
Comment 28 Serge Gautherie (:sgautherie) 2006-11-27 00:38:58 PST
Comment on attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]


Checkin: {
2006-11-26 11:27	bugzilla%standard8.demon.co.uk
}

'approval1.8.1.1=?': (Thunderbird only)
'approval-seamonkey1.1=?':
Trivial U.I. code cleanup, no risk.
(To stay in sync, before other potentialy upcoming patches.)
Comment 29 Robert Kaiser 2006-11-27 03:46:15 PST
Comment on attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]

a=me for SeaMonkey 1.1 (mailnews/ part)
Comment 30 Daniel Veditz [:dveditz] 2006-11-28 15:25:38 PST
Comment on attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]

approved for 1.8 branch, a=mscott (via PM)
Comment 31 Serge Gautherie (:sgautherie) 2006-12-03 10:00:55 PST
*** Bug 325098 has been marked as a duplicate of this bug. ***
Comment 32 Daniel Veditz [:dveditz] 2006-12-04 10:49:50 PST
Comment on attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]

missed 1.8.1.1, removing approval. Will try to get it next time.
Comment 33 Serge Gautherie (:sgautherie) 2006-12-04 15:13:36 PST
Comment on attachment 170411 [details] [diff] [review]
(Bv1-TB) <mailWindowOverlay.js>
[Checkin: Comment 33]


Checkin: {
2006-12-04 09:09	bugzilla%standard8.demon.co.uk 	mozilla/mail/base/content/mailWindowOverlay.js 	1.153
}
Comment 34 Serge Gautherie (:sgautherie) 2006-12-04 15:50:32 PST
Comment on attachment 170411 [details] [diff] [review]
(Bv1-TB) <mailWindowOverlay.js>
[Checkin: Comment 33]


'approval1.8.1.2=?': (Thunderbird only)
Trivial U.I. code cleanup, no risk.
(Sync' SM->TB.)
Comment 35 Serge Gautherie (:sgautherie) 2006-12-22 15:23:18 PST
Comment on attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]


Checkin (/MailNews part): {
2006-12-22 07:13	neil%parkwaycc.co.uk 	mozilla/mailnews/base/resources/content/mailWindowOverlay.js 	1.222.2.20 	MOZILLA_1_8_BRANCH
}
Comment 36 Jay Patel [:jay] 2006-12-29 14:34:20 PST
Comment on attachment 217503 [details] [diff] [review]
(Cv1a) <mailWindowOverlay.js>
[Checkins: Comment 28 & 35 & 38]

Approved for 1.8 branch, a=jay for drivers.
Comment 37 Jay Patel [:jay] 2007-01-03 16:01:42 PST
Serge:  Could you please get (Bv1-TB) <mailWindowOverlay.js> [Checkin: Comment 33] re-reviewed, it's been a while since that patch was looked at.   If mscott or bienvenu want this other patch, we can take it. 
Comment 38 :Gavin Sharp [email: gavin@gavinsharp.com] 2007-01-09 11:51:24 PST
Already checked in:
mozilla/mail/base/content/mailWindowOverlay.js 1.95.2.49 by bugzilla%standard8.demon.co.uk
Comment 39 Serge Gautherie (:sgautherie) 2007-01-10 02:39:12 PST
(In reply to comment #37)
> Serge:  Could you please get (Bv1-TB) <mailWindowOverlay.js> [Checkin: Comment
> 33] re-reviewed, it's been a while since that patch was looked at.   If mscott

{
bienvenu@nventure.com  	2006-11-12 04:49:53 PST  	  Attachment #170411 [details] [diff]Flag  	review?(bienvenu@nventure.com)  	review+
mscott@mozilla.org  	2006-11-25 17:51:05 PST  	  Attachment #170411 [details] [diff]Flag  	superreview?(mscott@mozilla.org)  	superreview+
}

Is 1(+) month that old ?

> or bienvenu want this other patch, we can take it. 

{
sgautherie.bz@free.fr  	2006-12-04 15:50:32 PST  	  Attachment #170411 [details] [diff]Flag  	  	approval1.8.1.2?
}

What is really missing is the approval, requested 9 days "only" after sr...
Comment 40 Daniel Veditz [:dveditz] 2007-01-10 15:02:51 PST
(In reply to comment #38)
> Already checked in:
> mozilla/mail/base/content/mailWindowOverlay.js 1.95.2.49 by

That's attachment 217503 [details] [diff] [review], I was asking about attachment 170411 [details] [diff] [review] which is in the same file which confuses us as to whether one supersedes the other.
Comment 41 Serge Gautherie (:sgautherie) 2007-01-10 15:52:21 PST
(In reply to comment #40)
> {...} which is in the same file which confuses us as to whether one supersedes the other.

B and C patches are not related to one another, apart from applying to the same file.
Comment 42 Daniel Veditz [:dveditz] 2007-01-16 15:35:22 PST
Comment on attachment 170411 [details] [diff] [review]
(Bv1-TB) <mailWindowOverlay.js>
[Checkin: Comment 33]

approved for 1.8 branch, a=dveditz for drivers
Comment 43 Serge Gautherie (:sgautherie) 2007-03-02 05:31:45 PST
Comment on attachment 170411 [details] [diff] [review]
(Bv1-TB) <mailWindowOverlay.js>
[Checkin: Comment 33]

'approval1.8.1.3=?': (Thunderbird only)
It had 'dveditz: approval1.8.1.2+',
but noone actually checked it in :-/
Comment 44 Henrik Skupin (:whimboo) [away 09/30 - 10/06] 2007-03-23 14:10:43 PDT
Scott or David, is the appoved patch too late for TB2? It also waits for checkin for two months.
Comment 45 Daniel Veditz [:dveditz] 2007-04-09 18:11:42 PDT
Comment on attachment 170411 [details] [diff] [review]
(Bv1-TB) <mailWindowOverlay.js>
[Checkin: Comment 33]

Doesn't meet the 1.8.1.4 criteria, especially since we're not shipping a tbird update (2.0 will be out around then). You can try again later maybe when there is going to be a tbird update.
Comment 46 :Gijs Kruitbosch 2007-04-14 14:58:37 PDT
Removing 'checkin needed' as it is unclear what should (not) be checked in. If the flag is still needed, please add a comment explaining *exactly* which of the patches should be checked in where, and make sure to have the appropriate reviews and/or approvals.

Somewhat related, is there a particular reason this bug is not marked FIXED yet? Given the confusion (in discussion on IRC as well as, cf, comment #37, 39, 40 and 41), it might even be more appropriate to file a separate bug for followup issues.
Comment 47 Serge Gautherie (:sgautherie) 2007-10-27 10:41:28 PDT
Confusion started when Bv1-TB got approval1.8.1.2+, but was not checked in.
Then, it got approval1.8.1.3? unanswered, then approval1.8.1.4-.
Let's say I don't care any longer :-|

R.Fixed.

Note You need to log in before you can comment on or make changes to this bug.