Closed Bug 534382 Opened 15 years ago Closed 15 years ago

TB stores new fetched e-mails in Local Folders-1 after upgrade from tb2 to tb3(deferred_to_account is deleted, and the account/server number is used by "Smart Folders" of Tb3. Somehow directory for "Smart Folders" becomes "Local Folders-1")

Categories

(MailNews Core :: Backend, defect)

1.9.1 Branch
x86
Windows XP
defect
Not set
critical

Tracking

(blocking-thunderbird3.0 .1+, thunderbird3.0 .1-fixed)

RESOLVED FIXED
Thunderbird 3.1a1
Tracking Status
blocking-thunderbird3.0 --- .1+
thunderbird3.0 --- .1-fixed

People

(Reporter: exceptione, Assigned: Bienvenu)

References

Details

(Keywords: dataloss, fixed-seamonkey2.0.3, Whiteboard: [landed on trunk])

Attachments

(3 files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; nl-NL; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 FirePHP/0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; nl-NL; rv:1.9.1.5) Gecko/20091204 Thunderbird/3.0

I've upgraded tb2 to tb3. My old e-mail is in Local Folders. However, if I download new e-mails, they are stored in Local Folders-1. It's like tb tries to create a map Local Folders, and windows does so by suffixing it with "-1", the default behaviour in windows when you create a duplicate folder/file.

So new e-mails don't show up, because tb reads from Local folders, and writes to Local folders-1.

I've openened a thread (http://forums.mozillazine.org/viewtopic.php?f=39&t=1640085) but sofar nobody has seen this behaviour I think.

I tried to delete Local Folders-1. This does not help, since when I fetch new e-mails, Local Folders-1 is created again and the new e-mails are stored there.


Reproducible: Always




The only anti-virus software I have is ESET nod32.
Mail folder path for an account is set at:
  Tools/Account Settings/Server Settings
     Local Directory:
> Saved in next entry. Can be viewd by Config Editor.
> N == pointed by mail.account.accountA.server for "Local Folders" account.
>   mail.server.serverN.directory
>   mail.server.serverN.directory-rel
What path is set for "Local Folders"?

If you created or had directory named "Local Folders" before account setup of Tb, Tb creates "Local Folders-1" and uses it for psedo account of "Local Folders" upon first account definition.
How did you create profile and setup accounts?
Possibly a variant of phenomenon of bug 505465.
Is there multiple mail.account.accountN.server entries which point same serverX?
If yes, keep backup of prefs.js, and attach all next entries in your prefs.js, please.
> mail.accountmanager.accounts
> mail.account.accountN.server
@WADA      2009-12-12 21:56:15 PST 

It seems like smart mail-boxes are located in Local Folders-1, and local folders in Local Folders. Here some info from my config editor

> mail.server.server1.directory;profielen\Standaardgebruiker\Mail\Local Folders
> mail.server.server1.name;Lokale mappen
> mail.server.server1.hostname;Local Folders

> mail.server.server2.directory;\profielen\Standaardgebruiker\Mail\Local Folders-1
> mail.server.server2.hidden;true
> mail.server.server2.hostname;smart mailboxes

(Note that "Lokale mappen" means "Local folders".)


> Is there multiple mail.account.accountN.server entries which point same
> serverX?
AFAIK this nis not the case. The only strange thing is that — unlike the other mail.account.accountN 's — there is is no mail.account.account1.identities for mail.account.acount1.

> mail.account.account1.server;server1
> mail.accountmanager.localfoldersserver;server1

Thus server1 is a local folder server and has no identity.

Do you get an «Aha-Erlebnis» yet?
(I need to add that in the previous post I've stripped the common prefix of  mail.server.serverN.directories for brevity, this is not important)
Smart Mailboxes are supposed to into the "smart mailboxes" sub-directory of <profiledir>/Mail, not Local folders.
So I can change the value exactly to this in config editor? All lowercase? 

> mail.server.server2.directory;profielen\Standaardgebruiker\Mail\Local Folders\smart mailboxes

Or do I need to take additional care.

Very strange that Tb decided to move the smart folders to such a strange place then. This is a migration bug I suppose.
I had the same problem and made two observations (all options translated by me as I am using the German TB3):

(1) The archive folder setting for all mail accounts that were set up to use the "Local Folders" was set to "Grouped Folders" which was not a valid option when opening the drop-down menu. Because of this the content pane (right side of account settings dialog) kept freezing upon entering the "Copies & Folders" section of the affected accounts.
Fix: I reset the archive folder to "Local Folders".

(2) The advanced server settings (Account Settings -> <Account> -> Server Settings -> Advanced) for those accounts seem to have not been set correctly when migrating from TB2 to TB3. The accounts in question were set to "separate inbox for this account". Although this option was set the checkbox for "include this account for retrieving messages" was not deactivated like it should be. So that setting might have been damaged/not set/whatsoever by the migration.
Fix: I set the option to "global inbox (local account)" like it was in TB2 and now all mail for those accounts arrives in the "Local Folders" as it used to.

Hope this helps.
Unfortunately I did not backup the prefs.js before changing the advanced settings so I do not know what was set before my changes which could maybe help in tracking down the cause of this issue.

Now I have a bunch of mail in the "Local Folders-1"-directory and not showing up in the global inbox. Any suggestions for putting them into the right place again (messages are not stored on the server anymore)?
this is datalossy.
taking stab at better component
Severity: major → critical
blocking-thunderbird3.0: --- → ?
Component: General → Backend
Keywords: dataloss
Product: Thunderbird → MailNews Core
QA Contact: general → backend
Version: unspecified → 1.9.1 Branch
see http://getsatisfaction.com/mozilla_messaging/topics/messages_disappear_when_moving_to_a_local_folder for some more people w/ the same issue.
blocking-thunderbird3.0: ? → .1+
probable fix coming in bug 505465
bug 505465 is about having two identical accounts, e.g.,

"mail.account.acount1.server", "server1"
"mail.account.acount2.server", "server1"

In this bug, there is a server1 and a server2.
Exception e, were you using the German localization as well? the key is to figure out why smart mailboxes ended up in local folders...
(In reply to comment #3)
> mail.accountmanager.localfoldersserver;server1
> mail.server.server1.directory;profielen\Standaardgebruiker\Mail\Local Folders
> mail.server.server1.hostname;Local Folders
> mail.server.server2.directory;\profielen\Standaardgebruiker\Mail\Local Folders-1
> mail.server.server2.hidden;true
> mail.server.server2.hostname;smart mailboxes

Tb3 uses "...\Mail\Local Folders" for "Local Folders", and "...\Mail\Local Folders-1" for "Smart Folders", according to your prefs.js.

(In reply to comment #0)
> I tried to delete Local Folders-1.
> This does not help, since when I fetch new e-mails, Local Folders-1 is created again

It's normal, because server2(Smart Folders) uses the directory.

> and the new e-mails are stored there.

Exception e, do you use "Global Inbox"? If yes, check next, please. Points server1?
> mail.server.serverX.deferred_to_account

Exception e, do you have filter rules which move mails to other mail folder?
If yes, check msgFilterRules.dat content. Do you see string of "Folders-1" or "smart" it?

Before your upgrade from Tb2 to Tb3, did you use the profile by Tb3.0rcx, Tb3.0bx, Tb3.0ax, or Tb3 nightly build?
If profile is used by Tb2 after used by Tb3, entry for "Smart Folders" created by Tb3 is displayed as ordinal account by Tb2(hidden is not supported by Tb2).
Exception e, did you touch or use "Smart Folders" by Tb2?
(In reply to comment #12)
> Exception e, were you using the German localization as well? the key is to
> figure out why smart mailboxes ended up in local folders...

No, I did use the Dutch localization.
For "Local Folders-1".

Smart folders definition in prefs.js. 
> user_pref("mail.server.server3.hidden", true);
> user_pref("mail.server.server3.hostname", "smart mailboxes");
> user_pref("mail.server.server3.name", "Smart Folders");
> user_pref("mail.server.server3.type", "none");
> user_pref("mail.server.server3.userName", "nobody");
I changed above to:
> user_pref("mail.server.server3.hidden", true); => I deleted
> user_pref("mail.server.server3.hostname", "Local Folders");
> user_pref("mail.server.server3.name", "XSmart Folders");
This definition is same as "Local Folders", except mail.server.serverX.name.
Tb2 doesn't display server3, because duped hostname/username.
Tb3 changed back to original by restart.
> user_pref("mail.server.server3.hidden", true); <= added by Tb3
> user_pref("mail.server.server3.hostname", "smart mailboxes"); <= replaced
> user_pref("mail.server.server3.name", "Smart Folders"); <= replaced
I changed server3 to server4, and changed hostname, name, and deleted hidden.
Tb3 used server4 for Smart Folders and changed back to original by restart.

There is no mail.accountmanager.localfoldersserver like one for Smart Folders.
How Tb3 selects serverX for "Smart Folders"?
Tb3.0 does do next or similar? (to remove garbage by old nightlies)
  Tb3.0 selects serverX of next as server for Smart Folders.
    hostname="Local Folders" or "smart mailboxes"
    userName=nobody
    type=none
    If hostname="Local Folders", serverX not pointed by localfoldersserver.
  If hostname="Local Folders", replace it by "smart mailboxes". 
  hidden is added, and name is set to "Smart Folders".

If old nightly set hostname="Local Folders", and if directory-rel/directory was not written in the past, directory of "Local Folders-1" could be created and used.
(In reply to comment #13)
> (In reply to comment #3)
> > mail.accountmanager.localfoldersserver;server1
> > mail.server.server1.directory;profielen\Standaardgebruiker\Mail\Local Folders
> > mail.server.server1.hostname;Local Folders
> > mail.server.server2.directory;\profielen\Standaardgebruiker\Mail\Local Folders-1
> > mail.server.server2.hidden;true
> > mail.server.server2.hostname;smart mailboxes
> 
> Tb3 uses "...\Mail\Local Folders" for "Local Folders", and "...\Mail\Local
> Folders-1" for "Smart Folders", according to your prefs.js.
 
Yes, I know, I suppose this is were the trounble starts.


> (In reply to comment #0)
> > I tried to delete Local Folders-1.
> > This does not help, since when I fetch new e-mails, Local Folders-1 is created again
> 
> It's normal, because server2(Smart Folders) uses the directory.
> 
> > and the new e-mails are stored there.
> 
> Exception e, do you use "Global Inbox"? If yes, check next, please. Points
> server1?
> > mail.server.serverX.deferred_to_account
> 

The accounts use their own Inbox, according to the "Account settings" pane. But there are two non-default strings associated indeed:

mail.server.server3.deferred_to_account;account2
mail.server.server4.deferred_to_account;account2





> Exception e, do you have filter rules which move mails to other mail folder?
> If yes, check msgFilterRules.dat content. Do you see string of "Folders-1" or
> "smart" it?
> 
Yes, I have filter rules! 

 \profielen\Standaardgebruiker\Mail\Local Folders\msgFilterRules.dat

points for example to mailbox://nobody@Local%20Folders/Inbox/MyAccount

But there is also a 
 \profielen\Standaardgebruiker\Mail\Local Folders-1\msgFilterRules.dat

which does contain nothing more then

> version="9"
> logging="no"



> Before your upgrade from Tb2 to Tb3, did you use the profile by Tb3.0rcx,
> Tb3.0bx, Tb3.0ax, or Tb3 nightly build?

I never had run tb3 before. I just upgraded from tb2 to tb3.

> If profile is used by Tb2 after used by Tb3, entry for "Smart Folders" created
> by Tb3 is displayed as ordinal account by Tb2(hidden is not supported by Tb2).
> Exception e, did you touch or use "Smart Folders" by Tb2?

So, I would never be able to touch smart folders in tb2, since tb2 was removed before the first run of tb3.


Additionaly, this might be of interest: in account settings there is a display bug. I can acces all account settings from the left pane, but when I click on "Copies and folders" this page is displayed, but sticks there. I can click on other items in the left pane. They are visually marked as selected, but the right pane doesn't update; you still see the copies and folder settings there. This holds for every account.
(In reply to comment #16)
> there are two non-default strings associated indeed:
> mail.server.server3.deferred_to_account;account2
> mail.server.server4.deferred_to_account;account2

It's the reason why mail data was written in ...\Mail\Local Folders-1.
For future analysis, keep bacup op prefs.js, please.
Next?
  1. server2 existed. 2. server3/server4 were defered to server2.
  3. server2 was deleted. 4. server2 was used for "Smart Folders".
Or Tb3 permits deferering to "Smart Folders"?

Manual recovery.
(1) Directory name for "Smart Folders".
(1-1) Delete next two entries. 
> mail.server.server2.directory
> mail.server.server2.directory-rel
(1-2) Rename "...\Mail\smart mailboxes", "...\Mail\smart mailboxes-1", 
    "...\Mail\smart mailboxes-2", ... to apropriate name.
(2) recovery of deferred_to_account : points appropriate server.
(3) recovery of mails in "...\Mail\Local-Folders-1"
(3-1) Move "...\Local-Folders-1" to "...\Local Folders\Local-Folders-1".
(3-2) Rename "...\Local Folders\Local-Folders-1" to "...\Local Folders\Local-Folders-1.sbd".
(3-3) Create a file of "...\Local Folders\Local-Folders-1" (null file)
(4) Restart Tb, check account settings, folder content, and terminate Tb.
(5) Check prefs.js content.
(In reply to comment #17)
> (In reply to comment #16)
> > there are two non-default strings associated indeed:
> > mail.server.server3.deferred_to_account;account2
> > mail.server.server4.deferred_to_account;account2
> 
> It's the reason why mail data was written in ...\Mail\Local Folders-1.
> For future analysis, keep bacup op prefs.js, please.
> Next?
>   1. server2 existed. 2. server3/server4 were defered to server2.
>   3. server2 was deleted. 4. server2 was used for "Smart Folders".

Thx, WADA, that makes sense. Basically, we're not cleaning up prefs sufficiently when removing accounts/servers. No, TB3 does not permit deferring to Smart Folders.

I think the solution would have 3 parts:

1. Trying to repair already broken profiles
2. Cleaning up prefs correctly when removing accounts/servers (I thought we did that, however)
3. When adding a new account&server, make sure there aren't stale prefs that affect the account&server.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → bienvenu
(In reply to comment #16)
> Additionaly, this might be of interest: in account settings there is a display
> bug. I can acces all account settings from the left pane, but when I click on
> "Copies and folders" this page is displayed, but sticks there. I can click on
> other items in the left pane. They are visually marked as selected, but the
> right pane doesn't update; you still see the copies and folder settings there.
> This holds for every account.

Read bug 535183, please.
(In reply to comment #18)
> >   1. server2 existed. 2. server3/server4 were defered to server2.
> >   3. server2 was deleted. 4. server2 was used for "Smart Folders".
> that makes sense. Basically, we're not cleaning up prefs
> sufficiently when removing accounts/servers.

Does Tb2(1,3) falls back to mail.accountmanager.localfoldersserver(server1) if server2 which is pointed by mail.server.server3.deferred_to_account doesn't exist?
If so, user usually is not aware of inconsistency, because mails to account3/account4 is downloaded in Inbox of "Local Folders" after deletion of account2. It's same as server3.deferred_to_account=server1 unless account2/server2 is defined again.
If not, I think user usually realizes that no mail arrives to mail address assiged to account3/account4 for long time during Tb2 use, then bugs are opened.
Summary: TB stores new fetched e-mails in Local Folders-1 after upgrade from tb2 to tb3 → TB stores new fetched e-mails in Local Folders-1 after upgrade from tb2 to tb3(deferred_to_account is deleted, and the account/server number is used by "Smart Folders" of Tb3. Somehow directory for "Smart Folders" becomes "Local Folders-1")
(For Local Folders-1)
When garbage of next exists(account2 is deleted. No other xxx.server2.xxx exists), Tb3 used this garbage for "Smart Folders"(account2/server2).
> user_pref("mail.server.server2.directory", "C:\\...\\Mail\\Local Folders");
> (directory-rel is generate based on this setting)

Because garbage of mail.server.server2.hostname="Local Folders" for "Smart Folders" can't produce directory of "...\Local Folders-1" for "Smart Folders", I guess next.
 (a) mail.server.server2.directory=...\\Local Folders-1(and/or directory-rel)
     was generated in the past,
 (b) and garbage remaind, and re-used by "Smart Folders" of Tb3.
A way to force (a) is set mail.server.server2.hostname="Local Folders" and delete mail.server.server2.directory and mail.server.server2.directory-rel for an account.

Anyway, upon new serverX creation, all exsistent mail.server.serverX.xxx should be discarded.
WADA, I don't see any code that falls back to the local folders server if the deferred to account doesn't exist. I'll have to try it in 2.0, I guess.

Exception E, do you have your bad prefs.js so I can see it in its entirety? Or have you fixed it already?
I tried setting this up with 2.0 by hand - if I defer to an account that doesn't exist, get new mail simply doesn't do anything. But I'm reasonably sure from the evidence that it must have worked somehow. I'm sure that the smart mailboxes account was added, and used a previously deleted account, and inherited its prefs.
Even the 2.0 code cleared out prefs when accounts were deleted, so I still don't see how the 2.0 -> 3.0 transition caused this problem. I'd really like to understand this before working on the prevention and repair steps...
prefs.js, anonimized. All preferences are left intact, but some non-critical values like user names are anonimized consistently.
(In reply to comment #22)
> Exception E, do you have your bad prefs.js so I can see it in its entirety? Or
> have you fixed it already?

I didn't dare to touch my settings yet, so I haven't fixed it yet. I have attached per your request my prefs.js
Exception E, thx very much, that was very helpful. Here's how you can fix your profile (after first backing it up, of course):

1. Tools | options | advanced, general - config editor
2. Type deferred_to_account
3. For the two matches that defer to account2, right click each line, pick modify, and change to account1.
4. Restart

I'm starting to get an idea of what might be happening here. I tweaked a 2.0 profile to have two identical local folder accounts, account1 and account2. 2.0 is perfectly happy with this. I then discovered that when I added a new pop3 account, and made it use the global inbox, it deferred to account2. My suspicion is that 3.0 was somewhat unhappy with the duplicate account, and expressed its displeasure in such a way that the smart folders account it added used account2.
Yes, TB 3 tries to remove accounts that fail to create their server when loaded, and duplicate accounts with the exact same server fail because creating a server that already exists fails. This was an attempt clean up the duplicate accounts that show up in the folder pane due to de-rdfication, but it didn't pay any attention to whether other accounts deferred to the account/server we were trying to remove. And indeed, it can't know that until all the accounts have been loaded.

I'm probably going to need to extend the patch in bug 505465 since its closely related, and has a unit test. I'll probably use this bug for a patch to repair the corruption, along with a unit test to show the repair working.
I'm still missing a piece of the puzzle, which is why we created a new local folders-1 directory. When I try this, the failure that causes us to remove the second local folders account also causes us to fail to remove the server, so we're left with a server pointing at the correct local folders directory, and we don't try to create a new one.
Ugh, I'm going to inch out a bit further on the limb and assume the Local Folders-1 thing happened with TB 2, and TB 3 came along and essentially removed the wrong account. If that's the case, then the people seeing this bug have been having their e-mail delivered to the local folders-1 directory all along. If that's the case, switching the deferred to account via the config editor to the local folders account would only show newly downloaded mail, and the tb 2 downloaded mail would still be in the local folders-1 directory. It would be helpful if those experiencing this bug could check the respective inboxes in local folders and local folders-1 and tell me which kind of messages were in each. Thx!
(In reply to comment #30)
> If that's the case, switching the deferred to account via the config editor to
> the local folders account would only show newly downloaded mail, and the tb 2
> downloaded mail would still be in the local folders-1 directory.

That is not the case at least on my side. The file Inbox in "Local Folders" contains every mail from TB 1.5 (1.5.0.4 Windows/20060516, actually also mail imported from another client before TB) to now. The Inbox in "Local Folder-1" contains only a few mails that were retrieved by TB 3 using the bad settings.
But the "Local Folders-1" directory seems to have been around there for some time as you suggested. The file system dates its creation time to 21 Feb 2008 but the Inbox file inside of it was created on 8 Dec 2009 (day of my upgrade to TB 3).

I also tried to reproduce this bug using a clean TB 2 (2.0.0.23) installation, setting up my accounts and upgrading to TB 3. It did not show up. That is why I included the version number of my first TB installation above. I suspect that the problem could have been introduced by an even older version of TB by using some setting that was tolerated by TB 2 but not TB 3. I did not have the time to test it yet.
Thx for the confirmation, Dirk. I tried it here earlier and found that 2.0 would store the mail in Local Folders, even though account2 was using server2, and server2 pointed at Local Folders-1, and the pop3 accounts were deferring to account2. So 3.0 needs to do the same somehow.
Exception e(bug opener):

Can you check next.
 - Creation date of directory of "...\Local Folders-1"
 - Creation date of file of "...\Local Folders-1\Inbox"
 - First line of file of "...\Local Folders-1\Inbox"
   "From ..." and "..." part is local time of mail download without tome-zone.

Anyway, please keep backup of prefs.js and "...\Local Folders-1\Inbox" for future problem analysis.
You can execute manual recovery after keeping backup.
(In reply to comment #23)
> I tried setting this up with 2.0 by hand - if I defer to an account that
> doesn't exist, get new mail simply doesn't do anything. 
> But I'm reasonably sure from the evidence that it must have worked somehow.

Bug 303542 may explain it.
  - Tb2 doesn't display duplicated account(same hostname/userName).
    Tb2's folder selection.
      deferred_to_account=account2 => account2/server2
      => hostname=Local Folders/userName=none
      => search server for it => use "...\Local Folders" of server1
  - Tb3 displays duplicaed account(same hostname/userName).
    Tb3's folder selection.
      See server2 correctly from account2/server2

> I'm sure that the smart mailboxes account was added, and used a previously deleted account, and inherited its prefs.

If account2 was deleted, I don't think mails for deferred_to_account=account2 can't go "...\Local Folders". If it's true, use of account2/server2 by "Smart Folders" may be next phenomenon.
  Tb3 reuses account2/server2 for "Smart Folders", if server2 has
  hostname=Local Folders/userName=none and is not pointed by
  localfoldersserver.
Tb3 didn't reuse server2(and account2 too), if account2 exists ant it points server2 of hostname=Local Folders/userName=nobody(it was nobody, not none).

To Exception e(bug opener):
Did you delete account via UI or delete entries of prefs.js in the past?
If yes, you defined account after your last deletion of account?
(if you re-defined, account2(use serverX) should exist in your prefs.js, and Tb3 won't use account2 for "Smart Folders".)
Tb3 reused server2/server2, when account2 and server2 is copy of "Local Folders" account(account2.server=server2 only, no account2.identitities, xxx.server1.xxx was copied to xxx.server2.xxx).
Note: check of comment #35 was done with account2/server2 for dummy POP3 account. (I probably forgot to change type to "none".)
Attached patch proposed fixSplinter Review
This fixes it so we don't remove duplicate accounts (which caused the bug in the first place - I've been working on a separate patch to remove unused accounts and I'll need to modify it to make sure it deals with the local folders-1 issue), and fixes broken deferred to accounts. I need to try to write a unit test for this, and think about a way to recover mail that's been delivered to the hidden inbox.

If someone wants to write an extension to repair broken profiles, what that extension would probably do is iterate over pop3 incoming servers, do the same check this patch does, i.e., that the deferred to server is not hidden, and do the same kind of repair (set the deferred to account to the local folders server account).
I could observe deletion of accounts who point to servers of same properties as "Local Folders", and re-use of first server of same properties as "Local Folders" which is not pointed by localfoldersserver by "Smart Folders".

1. Folder view=All Folders, terminate Tb
2. Craft prefs.js
   accounts=account1,accountX,accountY,accountZ 
   localfoldersserver=server1
   account1.server=server1
   accountX.identities=idP
   accountX.server=serverX
   accountY.server=serverY
   accountZ.server=serverZ
   server.server1.xxx=for Local Folders
   server.serverX.xxx=copied from server1(directory-rel/directory is deleted)
   server.serverY.xxx=copied from server1(directory-rel/directory is deleted)
   server.serverZ.xxx=copied from server1(directory-rel/directory is deleted)
3. Restart Tb, All Folders is displayed, terminate Tb
   next entrie was deleted(points serverN of same properties as server1.
     accountX.identities=idP
     accountX.server=serverX
     accountY.server=serverY
     accountZ.server=serverZ
4. Restart Tb, go to "Smart Folders", terminate Tb
   serverX was used for "Smart Folders", and property values were changed to
   one for "Smart Folders".

Following is my understanding. David, is it not wrong?

Above deletion of accounts/reuse of server seems to be recovery from garbages for "Smart Folders" by old Tb3 nightly builds.
If user already has accountX which points serverX of same property as "Local Folders", serverX is used for "Smart Folders" by Tb3 due to above.
And, if user sets accountX as deferred_to_account of some accounts, problem of this bug occurs, because deferred_to_account is not cared upon account deletion and Tb3 uses directory specified in server which is pointed by account, as initially designed.
Directory name used by "Smart Folders" depends on when directory was created and savd in directory-rel/directory.
Tb2 saved mail data of such accounts(points same hostname/userName as server1) to directory pointed by server1, so problem wasn't exposed during Tb2 era.

Question:
When/by Whom/How were such accounts generated at bug opener's environment.
That's basically right, except that I don't think the issue is specific to TB 3 nightly builds. TB 2 would create the garbage local folders-1 account, and newly added pop3 servers would defer to it, but everything worked anyway. Then TB 3 final came out, and tried to clean up the bogus accounts, and then re-used one for the smart folders account.
Attached patch unit testSplinter Review
Attachment #418721 - Flags: review?(bwinton)
Attachment #418568 - Flags: superreview?(bugzilla)
Attachment #418568 - Flags: review?(bwinton)
Why account2's mail went to directory for server1 while Tb2 was used;  
(1) deferred_to_account=account2 -> account2.server=server2
    -> server2.hostname="Local Folders", server2.userName="nobody" 
(2) As seen in mail.identity.idX.drafts_folder etc. internal path for mail box
    doesn't have information of account2, server2.
      mailbox://nobody@Local%20Folders/Inbox
    => server entry of hostname="Local Folders", userName="nobody" is serached,
       and server1.directory-rel is used.
(3) After upgrade to Tb3, account2 is deleted, account2/server2 is used for
   "Smart Folders", and it becomes server2.hostname != server1.hostname, then
    server2.directory-rel(Local Folders-1) is used by Tb3 for accounts of
    deferred_to_account=account2.  

David, is it right?

If yes, similar problem can occur on Tb3, if multiple serverX of same hostname/userName exist(i.e. Bug 303542 happened.)
And, above can explain phenomenon on Tb3 after Bug 303542 occurs.

Tb2 didn't display accounts of same hostname/userName, but Tb3 exposes same accounts of hostname/userName.
Server setting is not obtained by accountA.server=serverB only. Server setting is searched based on hostname value and userName value which is obtained by accoutA.server=serverB. 
Thus, same definition data is displayed for multiple accounts(same hostname/userName accounts) at Account Settings and same folders is displayed at folder pane by Tb3 if Bug 303542 happens.
Comment on attachment 418721 [details] [diff] [review]
unit test

>diff --git a/mailnews/base/test

It looked like every line in this file ended in ^M.  Is that something we care about?  Will hg handle it for us when it gets checked in?

>+function run_test()
[…]
>+  prefs.setCharPref("mail.accountmanager.accounts", "account1,account2,account4,account5");

This line is over 80 characters, but I don't think we care.

>+  // Set the default account to one we're going to get rid of. The account manager
>+  // should recover relatively gracefully.

The first line is over 80 characters, and since the comment continues on the second line, we should probably re-wrap, but it doesn't look to me like we do get rid of any accounts, so perhaps we need a different comment here entirely.

>+  prefs.setCharPref("mail.accountmanager.defaultaccount", "account1");
>+
>+  // This will force the load of the accounts setup above.
>+  do_check_eq(am.accounts.Count(), 3); // hidden account not included
>+  let server4 = am.getAccount("account4").incomingServer
>+                  .QueryInterface(Ci.nsIPop3IncomingServer);
>+  do_check_eq(server4.deferredToAccount, "account1");
>+  let server5 = am.getAccount("account5").incomingServer
>+                  .QueryInterface(Ci.nsIPop3IncomingServer);
>+  do_check_eq(server5.deferredToAccount, "account1");

I don't see what the difference is between the server4 test and the server5 test.  Are we testing the same thing twice, and if so, do we want to?

And should we check that we write out the cleaned-up prefs?  (Or do we write out the cleaned-up prefs?)

Finally, this test seems to fail for me, so, uh, thumbs up?  ;)

I'm going to give it an r=me since the changes I'm asking for are minor.

Thanks,
Blake.
Attachment #418721 - Flags: review?(bwinton) → review+
Comment on attachment 418568 [details] [diff] [review]
proposed fix

>+++ b/mailnews/base/src/nsMsgAccountManager.cpp
>@@ -1354,19 +1354,16 @@ nsMsgAccountManager::LoadAccounts()
>     nsCOMPtr<nsIMsgIncomingServer> server;
>     account->GetIncomingServer(getter_AddRefs(server));
>-    // if no server, we should get rid of the account.
>-    if (!server)
>-      RemoveAccount(account);

Why aren't we removing the account here anymore?  The test still seems to pass when I don't delete these lines, which leads me to think that we need another test.  ;)

>+++ b/mailnews/local/src/nsPop3IncomingServer.cpp
>@@ -128,17 +128,51 @@ NS_IMPL_SERVERPREF_INT(nsPop3IncomingSer
>+  // We need to repair broken profiles that defer to hidden or invalid servers,
>+  // so find out if the deferred to account has a valid non-hidden server, and
>+  // if not, defer to the local folders inbox.

If the deferred to account is invalid, we go to the account for the Local Folders server, not the defaultaccount?

>+    nsCOMPtr<nsIMsgAccount> account;
>+    rv = acctMgr->GetAccount(aRetVal, getter_AddRefs(account));

I'm a little worried about what happens if rv is false here.  Are we guaranteed to get back a null account in that case?

>+    if (account)
>+    {
>+      nsCOMPtr<nsIMsgIncomingServer> server;
>+      account->GetIncomingServer(getter_AddRefs(server));

What would happen if this server deferred to yet another server?
(And while I'm asking, what would happen if a server deferred to itself, or if two deferred to each other in a loop?)

>+      // Can't call SetDeferredToAccount because it calls GetDeferredToAccount.
>+      return SetCharValue("deferred_to_account", aRetVal);

SetDeferredToAccount seems to do a whole lot of other stuff.  Maybe we should extract an internal method which both of these functions can call.

Later,
Blake.
Attachment #418568 - Flags: review?(bwinton) → review-
(In reply to comment #43)

thx for looking at the patch.

> Why aren't we removing the account here anymore? 

We remove those lines because they caused this bug. I need to rework the patch in the other bug about cleaning up duplicate accounts to handle the case of a duplicate account being deferred to. That's bug 505465

 The test still seems to pass
> when I don't delete these lines, which leads me to think that we need another
> test.  ;)
The test is for testing the repair of already corrupted profiles. I do need an other test to make sure we don't corrupt profiles, but that will go in bug 505465

> 
> >+++ b/mailnews/local/src/nsPop3IncomingServer.cpp
> >@@ -128,17 +128,51 @@ NS_IMPL_SERVERPREF_INT(nsPop3IncomingSer
> >+  // We need to repair broken profiles that defer to hidden or invalid servers,
> >+  // so find out if the deferred to account has a valid non-hidden server, and
> >+  // if not, defer to the local folders inbox.
> 
> If the deferred to account is invalid, we go to the account for the Local
> Folders server, not the defaultaccount?
Correct - the default account could be an imap account, for example. Since we don't know what the deferred to account was, we have to guess, and local folders is correct 99.9% of the time.
> 
> >+    nsCOMPtr<nsIMsgAccount> account;
> >+    rv = acctMgr->GetAccount(aRetVal, getter_AddRefs(account));
> 
> I'm a little worried about what happens if rv is false here.  Are we guaranteed
> to get back a null account in that case?

I think so, yes, but I can check.
> 
> >+    if (account)
> >+    {
> >+      nsCOMPtr<nsIMsgIncomingServer> server;
> >+      account->GetIncomingServer(getter_AddRefs(server));
> 
> What would happen if this server deferred to yet another server?
> (And while I'm asking, what would happen if a server deferred to itself, or if
> two deferred to each other in a loop?)

We don't know what the account was deferred to, that account has been crunched and replaced with a different account (usually, the smart mailboxes account)
> 
> >+      // Can't call SetDeferredToAccount because it calls GetDeferredToAccount.
> >+      return SetCharValue("deferred_to_account", aRetVal);
> 
> SetDeferredToAccount seems to do a whole lot of other stuff.  Maybe we should
> extract an internal method which both of these functions can call.

It does, but we don't need any of it for the repair case, I don't believe.
(In reply to comment #44)
> > Why aren't we removing the account here anymore? 
> We remove those lines because they caused this bug.

I think re-use of serverX entry of hostname="Local Folders"/userName=nobody/type=none(it's probably recovery from old nightly's bug) is another cause of this bug.
After restart("All Folders" view after restart) and remove of accountX who points serverX of same properties as one for "Local Folders", I added accountX=serverX via Config Editor. After change to "Smart Folders" view, Tb used the accountX.server=serverX, and replaced properties of serverX by one for "Smart Folders".  
David, what do you think?
(In reply to comment #27)
> Exception E, thx very much, that was very helpful. Here's how you can fix your
> profile (after first backing it up, of course):
> 
> 1. Tools | options | advanced, general - config editor
> 2. Type deferred_to_account
> 3. For the two matches that defer to account2, right click each line, pick
> modify, and change to account1.
> 4. Restart
> 
> I'm starting to get an idea of what might be happening here. I tweaked a 2.0
> profile to have two identical local folder accounts, account1 and account2. 2.0
Bienvenu and Wada: I am looking for a user friendly procedure for users. The closest I can see is comment 27. Is there anything better. Love to work with either of you to come up with a truly friendly workaround for users. Feel free to email me about it roland AT mozillamessaging.com
(In reply to comment #46)

There is no UI-only recovery procedure in this bug's case, and there is no general recovery procedure for this bug's problem.

Recovery procedure of bug opener's case.
This is improved/modified/user-friendly version of procedure in Comment #17. 
(1) By upgrade to Tb3, "use Inbox of Smart Folders as Global Inbox for me"
    was set for two accounts who use Global Inbox.
    (...defferred_to_account=account2 was wrongly set by old Tb,)
    (and "Smart Folders" of Tb3 hijacked account2/server2.      )
    => Sever Settings/Advanced, use Global Inbox(Local Folders account)
(2) "Smart Folders" account uses directory of "...\Local Folders-1".
    => Terminate Tb,
       edit prefs.js, delete ...server2.directory & ...server2.directory-rel,
       and restart Tb.
       As "Smart Folders" can't be accessed via Account Settings, 
       direct editing of prefs.js is required.
(3) Recovery of mails unexpectedly saved in "...\Local Folders-1".
    => Define dummy POP3 account(no Global Inbox use),
       select "...\Local Folders-1" at Server Settings/Local Directory:,
       and restart Tb. (restart is mandatory due to known/very-old bug)
       Mails are displayed in Inbox of the dummy POP3 account.
Comment on attachment 418568 [details] [diff] [review]
proposed fix

I'm not sure there are any actionable items in your review comments - I think the key thing that wasn't made clear when describing the patch is that it only attempts repair when the deferred to account has been lost and replaced with a new account, so we can't know much about the replaced account. It's a one time repair, basically...
Attachment #418568 - Flags: review- → review?(bwinton)
Comment on attachment 418568 [details] [diff] [review]
proposed fix

(In reply to comment #48)
> I'm not sure there are any actionable items in your review comments

No, it was mainly a bunch of questions, which I liked your answers to.  :)

(There were a couple of things which could have lead to extra work, but ended up not.)

r=me.

Thanks,
Blake.
Attachment #418568 - Flags: review?(bwinton) → review+
(In addition to comment #47)

Problem is next?
(1) Duplicated serverX existed(produced by Tb1/Tb1.5/Tb2) who points server of
    hostname="Local Folders",userName=nobody,type=one).
(2) By upgrade to Tb3, duplicated accounts(who points server of
    hostname="Local Folders",userName=nobody,type=one) are deleted.
(3) Garbage of such serverX is not deleted.
(4) By (2), "Smart Folders" uses account2 because account2 doesn't exist.
(5) server.server2 is used for account2 because no account uses server2.
(6) Account creation writes required server2.xxx entries such as hostname.
(7) Due to (3)/(5), other remained server2.xxx entry such as
    server2.directory-rel/server2.directory is reused upon first account
    access, then "Local Folders- 1" is used.

If so, improvement of process at step (3)/(5) will reduce confusion by user & Tb.
Whiteboard: [has patch for sr standard8]
Comment on attachment 418568 [details] [diff] [review]
proposed fix

>+  // We need to repair broken profiles that defer to hidden or invalid servers,
>+  // so find out if the deferred to account has a valid non-hidden server, and
>+  // if not, defer to the local folders inbox.
>+  nsCOMPtr<nsIMsgAccountManager> acctMgr =
>+                      do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID);
>+  PRBool invalidAccount = PR_TRUE;
>+  if (acctMgr)
>+  {

nit: invalidAccount can be declared inside the if.

sr=Standard8
Attachment #418568 - Flags: superreview?(bugzilla) → superreview+
Whiteboard: [has patch for sr standard8]
fixed on trunk.

I'll try to find a way to recover local folder-1 inboxes, but that will be in a follow-on bug.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1a1
Attachment #418568 - Flags: approval-thunderbird3.0.1?
there's an xpcshell test for this, but this seems important enough to have a litmus test as well...
Flags: in-testsuite+
Flags: in-litmus?
Blocks: 538414
Whiteboard: [landed on trunk]
Attachment #418568 - Flags: approval-thunderbird3.0.1? → approval-thunderbird3.0.1+
fixed for 3.01
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: