[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8a5) Gecko/20041116] (nightly) (W98SE) {{ Warning: redeclaration of var msgFolder Source File: chrome://messenger/content/msgMail3PaneWindow.js Line: 277, Column: 19 Source Code: var msgFolder = folder.QueryInterface(Components.interfaces.nsIMsgFolder); Warning: reference to undefined property defaultServer.msgFolder Source File: chrome://messenger/content/msgMail3PaneWindow.js Line: 803 }} Steps: 1. Open MailNews.

|Warning: reference to undefined property defaultServer.msgFolder| I don't know how to fix this one: helpwanted !

Please, check line 206 too: | var lastMessageLoaded = msgFolder.lastMessageLoaded;| I wonder where this |msgFolder| was/is supposed to come from, or maybe(!?) why this |if| block is not included at the end of the previous one...

(In reply to comment #2) >Please, check line 206 too: >| var lastMessageLoaded = msgFolder.lastMessageLoaded;| >I wonder where this |msgFolder| was/is supposed to come from, >or maybe(!?) why this |if| block is not included at the end of the previous >one... Very interesting... I suggest the safest course of action would be to declare and initialize msgFolder around line 130 so it's always available.

Av1, with comment 3 suggestion(s). Neil: *It's more line 140 than 130, isn't it ? *Do you know how to fix the other warning ?

I must have some changes which disrupt my line numbers. At least the first part of your patch looks ok, I haven't read the rest yet. As for the other warning I'm not sure what conditions you need to trigger it.

(In reply to comment #5) > As for the other warning I'm not sure what conditions you need to trigger it. It's triggered by the following line |user_pref("mail.server.server1.login_at_startup", true);|. If I change the value to |false| the warning goes away. NB: Same condition for bug 256447 warning which appears right after this one. [Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.5) Gecko/20041217] (release) (W98SE) No bug(s): + (K) Regression.

I guess that the Pop3 check starts before the UI has fully initialized !?

Weird. I saw this once, and then it went away again...

Comment on attachment 168070 [details] [diff] [review] (Av1b-MAS) <msgMail3PaneWindow.js> David: You can do the review too, if you have time... Thanks.

Comment on attachment 168070 [details] [diff] [review] (Av1b-MAS) <msgMail3PaneWindow.js> > var rerootingFolder = (uri == gCurrentFolderToReroot); >+ var msgFolder = folder.QueryInterface(Components.interfaces.nsIMsgFolder); > if (rerootingFolder) { Nit: keep the var rerootingFolder next to the if (rerootingFolder) it looks neater that way.

Av1b-MAS, with comment 10 suggestion(s). Keeping {{ (Av1b-MAS) <msgMail3PaneWindow.js> patch 2004-12-06 15:55 PST 3.82 KB neil.parkwaycc.co.uk: review+ }} David: Could you sr this for v1.8a6 ?

Comment on attachment 170624 [details] [diff] [review] (Av1c-MAS) <msgMail3PaneWindow.js> [Checked in: Comment 16] thx, sr=bienvenu, but I don't see any reason to put it in 1.8a6...

Comment on attachment 170624 [details] [diff] [review] (Av1c-MAS) <msgMail3PaneWindow.js> [Checked in: Comment 16] (In reply to comment #12) > (From update of attachment 170624 [details] [diff] [review] [edit]) > thx, sr=bienvenu, but I don't see any reason to put it in 1.8a6... To fix comment 0 (warning in console) and comment 3 ("undefined" var usage) !? 'approval1.8a6=?': Trivial U.I. code cleanup, no risk.

"Same" as Av1c-MAS, but for ThunderBird. I don't use TB: Could you test/review/check in this patch ? Thanks.

Comment on attachment 170624 [details] [diff] [review] (Av1c-MAS) <msgMail3PaneWindow.js> [Checked in: Comment 16] a=asa for checkin to 1.8a6

Comment on attachment 170624 [details] [diff] [review] (Av1c-MAS) <msgMail3PaneWindow.js> [Checked in: Comment 16] Check in: { 2005-01-09 14:13 neil%parkwaycc.co.uk mozilla/ mailnews/ base/ resources/ content/ msgMail3PaneWindow.js 1.274 }

Comment on attachment 170628 [details] [diff] [review] (Bv1-TB) <msgMail3PaneWindow.js> [Checked in: Comment 19] 'approval1.8a6=?': Trivial U.I. code cleanup, no risk. This one should not affect v1.8a6, and it's MAS counterpart has already been checked in: it can't hurt to stay in sync..

Comment on attachment 170628 [details] [diff] [review] (Bv1-TB) <msgMail3PaneWindow.js> [Checked in: Comment 19] too late for non-critical fixes. please hold until we open for 1.8 Beta. thanks.

Comment on attachment 170628 [details] [diff] [review] (Bv1-TB) <msgMail3PaneWindow.js> [Checked in: Comment 19] Check in: { 1.58 neil%parkwaycc.co.uk 2005-01-14 05:07 }

The defaultServer.msgFolder problem, is a regression from bug 260447 - msgFolder is not a property of defaultServer, perhaps you meant rootFolder instead? >+ if (defaultServer && defaultServer.equals(currentServer) && >+ !defaultServer.isDeferredTo && >+ defaultServer.msgFolder == defaultServer.rootMsgFolder)

That was actually quoted from mailWindowOverlay.js so in fact we have that error in both Suite and Thunderbird's versions of the two files.

Comment on attachment 173646 [details] [diff] [review] (Cv1) <mailWindowOverlay.js> [Checked in: Comment 24] I'll take that as a "yes" ;-) I think there's still a case in the two copies of msgMail3PaneWindow.js as mentioned in comment 0 and comment 1.

Comment on attachment 173646 [details] [diff] [review] (Cv1) <mailWindowOverlay.js> [Checked in: Comment 24] Check in: { 2005-02-07 10:34 bienvenu%nventure.com }

Using Cv1 as a template... I don't use TB: Could you test/(super-)review/check in this patch ? Thanks.

fixing the warning breaks the downoading of pop3 mail on startup - since the bogus code before always returned false for one of the conditions, I've removed the whole if clause to get back to where we were before the warning was fixed.

Comment on attachment 173763 [details] [diff] [review] fix regression introduced by fixing warning. [Checked in: Comment 27] Check in (without the |-w|): { 2005-02-08 10:25 bienvenu%nventure.com mozilla/ mailnews/ base/ resources/ content/ mailWindowOverlay.js 1.214 }

(In reply to comment #27) > (From update of attachment 173763 [details] [diff] [review] [edit]) > > Check in (without the |-w|): { 2005-02-08 10:25 bienvenu%nventure.com > mozilla/ mailnews/ base/ resources/ content/ mailWindowOverlay.js 1.214 } With the mailnews/ part too.

Comment on attachment 173679 [details] [diff] [review] (Dv1) <msgMail3PaneWindow.js> [Checkin: See comment 35] [Mozilla Thunderbird, version 1.0.1 (20050309)] (nightly) (W98SE) No review from <bienvenu@nventure.com> since "2005-02-07" :-( Could you review/check in this patch ? Thanks.

Comment on attachment 170628 [details] [diff] [review] (Bv1-TB) <msgMail3PaneWindow.js> [Checked in: Comment 19] [Mozilla Thunderbird, version 1.0.1 (20050309)] (nightly) (W98SE) Welcomed on branch too.

Comment on attachment 170628 [details] [diff] [review] (Bv1-TB) <msgMail3PaneWindow.js> [Checked in: Comment 19] please don't nominate JS warnings for .0.1. This is a security respin intended for critical security bugs and not JS warnings.

Serge, in line with what Scott said, please don't nominate javascript warnings for inclusion in any release. If you can get them in during the open checkin periods, that's great. If not, then please wait until the next open period.

Comment on attachment 173679 [details] [diff] [review] (Dv1) <msgMail3PaneWindow.js> [Checkin: See comment 35] [Mozilla Thunderbird, version 1.0.1 (20050309)] (nightly) (W98SE) I'm puzzled by the fact that TB doesn't complain with either |msgFolder| or |rootFolder|... I can't tell which one it best.

Comment on attachment 173679 [details] [diff] [review] (Dv1) <msgMail3PaneWindow.js> [Checkin: See comment 35] I think this patch is obsolete now. Looks like it's been checked in already.

Comment on attachment 173679 [details] [diff] [review] (Dv1) <msgMail3PaneWindow.js> [Checkin: See comment 35] (In reply to comment #34) > (From update of attachment 173679 [details] [diff] [review] [edit]) > I think this patch is obsolete now. Looks like it's been checked in already. Right: /mail/ part was later included in bug 270743; /mailnews/ part was later included in bug 304176. As a sidenote, I'm still "sad" that so many of my patches simply rot in BugZilla, while other people (you and Neil in this case) 1) have to double my work (monthes later), 2) are able to get their patchs in :-/

(In reply to comment #35) > (From update of attachment 173679 [details] [diff] [review] [edit]) > (In reply to comment #34) > > (From update of attachment 173679 [details] [diff] [review] [edit] [edit]) > > I think this patch is obsolete now. Looks like it's been checked in already. > > Right: > /mail/ part was later included in bug 270743; > /mailnews/ part was later included in bug 304176. > > As a sidenote, > I'm still "sad" that so many of my patches simply rot in BugZilla, > while other people (you and Neil in this case) 1) have to double my work > (monthes later), 2) are able to get their patchs in :-/ maybe we can get you focused on something that's going to have more impact that these JS warnings that we don't even see. They are so low on our list that they tend to be more of a nuisance to have to deal with. And in one case, one of them snuck in and caused a really bad regression in Thunderbird 1.5. That would be a win win for everyone if we could do that :)

(In reply to comment #36) > maybe we can get you focused on something that's going to have more impact that > these JS warnings that we don't even see. They are so low on our list that they > tend to be more of a nuisance to have to deal with. And in one case, one of > them snuck in and caused a really bad regression in Thunderbird 1.5. That would > be a win win for everyone if we could do that :) At least, I have stopped preparing patches (for such JS warning bugs)...

(In reply to comment #36) >And in one case, one of them snuck in and caused a really bad regression Well in this case I'm blaming David for forgetting to set javascript.options.strict to true and thus catching his error :-P

(In reply to comment #35) >/mail/ part was later included in bug 270743; >/mailnews/ part was later included in bug 304176. Indeed, these regressions might well have been avoided had people paid more attention to JavaScript warnings in the first place.