Open Bug 946876 Opened 11 years ago Updated 2 years ago

Can't edit account settings after update to version 24, folder location error

Categories

(MailNews Core :: Account Manager, defect)

All
Windows 7
defect

Tracking

(Not tracked)

People

(Reporter: standard8, Assigned: aceman)

References

()

Details

(Keywords: regression, Whiteboard: [tracking TB31])

This is a clone for the full fix for bug 921371

+++ This bug was initially created as a clone of Bug #921371 +++

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Firefox/24.0 (Beta/Release)
Build ID: 20130910160258

Steps to reproduce:

after upgrade from Thunderbird version 17.08 to version 24  I could not edit my accountsettings anymore.


Actual results:

 I got a pop-up screen saying " "Het lokale map-pad "C:\......\TB\Profiles\xxxxxxxx.default\Mail\Local Folders-2" is geschikt voor berichten opslag. Kies een ander pad."
I was not able anymore to edit any of the items in the Accountsettings screen as this pop-up showed every time.
someone else had/has the same problem :
http://www.mozbrowser.nl/forum/viewtopic.php?f=3&t=22587
but it was not resolved.
I added a new account, same result. And i could not delete any of the accounts.


Expected results:

I should be able to edit my account settings. after I rolled back to 17.08 all worked fine. ( running on windows 7 64bit with all the latest MS updates)
Copying bug 921371 comment 40 as it contains info on how to attempt the permanent fix:
For all involved and mainly the great supporters (like Lee and Eckard) at
http://forums.mozillazine.org/viewtopic.php?f=39&t=2750681

The problem the users are seeing is this:
Due to some users using strange directories for their mail storage (e.g. the whole profile) and that colliding with the files in there that TB needed to use for its proper operation, in bug 750781 (and another one) I tried to blacklist some unsafe directories. The blacklist can be seen on top of the file http://hg.mozilla.org/comm-central/file/e57fef637964/mailnews/base/prefs/content/AccountManager.js and it amounts to this on Windows:
ProfD = C:\Documents and Settings\<User>\Application Data\Mozilla\Thunderbird\Profiles\12345678.default
GreD = C:\Program Files\Mozilla Thunderbird
CurProcD = C:\Program Files\Mozilla Thunderbird
TmpD = C:\Documents and Settings\User\Local Settings\Temp
SysD = C:\WINDOWS\system32
WinD = C:\WINDOWS
ProgF = C:\Program Files

You can't have the local directory point to a subdirectory of any of these directories but also not to a directory above it (e.g. C:\). There is one exception to this rule:
You can have the local directory pointing to under "ProfD = C:\Documents and Settings\User\Application Data\Mozilla\Firefox\Profiles\12345678.default" as that is the default setting. But it is allowed only for some predefined subdirectories. The catch is, the list of allowed directories is taken from each account's server type using the GetDefaultLocalPath function. And that function actually consults the "mail.root.<servertype>" prefs (see e.g. http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Service.cpp#525). The idea being that e.g. for POP3 this returns C:\Documents and Settings\<User>\Application Data\Mozilla\Thunderbird\Profiles\12345678.default\Mail. That is true by default, but seemingly not when the value of the pref is already generated like this, stored in prefs.js and then the user moves the whole profile to another place, or upgrades to Win Vista+ and just copies the profile from "C:\Documents and Settings\User\" to "C:\Users\" (creating a new profile from scratch would work fine). So if that pref now contains some invalid/unusable directory TB only allows this invalid directory to be used.
So even if you have your Local directory now set to the valid and fine "C:\Users\<user>\AppData\Mozilla\Thunderbird\Profiles\12345678.default" you may still get the warning.

So in this bug I lessen the impact of the warning (it will still allow you to proceed in the account manager) and then we will think out how to solve this. I didn't want to hardcode the allowed subdirs ("Mail", "ImapMail", "News") in the check function that is why I used GetDefaultLocalPath. But if that becomes unusable.

My idea would be to silently reset the value of the pref mail.root.pop3 if it does not match the mail.root.pop3-rel. Imagine this:
mail.root.pop3 = C:\Documents and Settings\<User>\Application Data\Mozilla\Thunderbird\Profiles\12345678.default\Mail
mail.root.pop3-rel = [ProfD]/Mail

On Win XP both prefs point to the same local directory.
When you now move to Win Vista+ the profile is in "c:\users" which means mail.root.pop3 is an invalid directory but mail.root.pop3-rel now points to (after parsing) C:\Users\<User>\AppData\Mozilla\Thunderbird\Profiles\12345678.default\Mail . In that case we could reset mail.root.pop3 so that it stores the new valid 
C:\Users\<User>\AppData\Mozilla\Thunderbird\Profiles\12345678.default\Mail value.

Of course, to still allow to set his own value of mail.root.pop3* prefs (for legacy reasons) he would need to set both prefs to point to a valid and SAME directory. I think that would be a reasonable requirement. What do you think?
(In reply to :aceman from comment #1)
> My idea would be to silently reset the value of the pref mail.root.pop3 if it does not match the mail.root.pop3-rel

Maybe we could just drop support for the non-relative paths?
Yes, this was discussed in another bug (which I need to find and link here) and I think more people supported the idea of dropping it.
I need to see what the exact logic is (when we use the absolute path and when the relative one). I find rel paths like "[ProfD]../../../../../D:/Mail" awkward when the abs path is plain "D:/Mail". If we only had the rel version, the path will break if we move the profile. If we preferred the abs path, it would still work as whereever the profile is, the data is still untouched on D:/Mail.
(In reply to :aceman from comment #3)
> I need to see what the exact logic is (when we use the absolute path and
> when the relative one).
> If we preferred the abs path, it would still work as whereever the profile is,
> the data is still untouched on D:/Mail.

aceman, do you understand why directory-rel was introduced?

Problem with mail.server.server#.directory only was;
  If Tb's Profile is copied,
  State of "mail directory of an account=[ProfD]Mail/pop3.x.y.z"(directory under profile)
  is broken.

So, to resolve this problem, "relative path from Tb's profile directory" was introduced.
  mail.server.server#.directory-rel=[ProfD]Mail/pop3.x.y.z format(directory under profile)
  mail.server.server#.directory-rel always supersedes mail.server.server#.directory.
And,
(A) For upward compatibility.
    If mail.server.server#.directory only,
    mail.server.server#.directory-rel is generated from mail.server.server#.directory.
(B) For downward compatibility.
    mail.server.server#.directory is always written from mail.server.server#.directory-rel.
By introducing this mail.server.server#.directory-rel, mail data under a Tb's profile   directory can be transferred to other Tb's profile by "simply copy Tb's profile directory".

Trick in "delete all .directory-rel, in order to force Tb to use absolute path in .directory" is (A).
(In reply to :aceman from comment #1)
> My idea would be to silently reset the value of the pref mail.root.pop3
> if it does not match the mail.root.pop3-rel.
It's good idea.
If state of "mail.root.*-rel != mail.root.*" is kept for long time,
(1) downward compatibilty with "Tb who supports mail.root.* but doesn't support mail.root.*-rel" is lost.
(2) if user manually deletes mail.root.*-rel without modifying mail.root.*, "directory in mail.root.* which may be different from user's intention" is used for mail directory upon next mail account creation and "mail.root.*-rel which may be different from user's intention" is created.
(3) If program code including add-on uses mail.root.* wrongly instead of proper mail.root.*-rel, wrong decision is made by the program code.
(4) It produces user's confusion when user sees both mail.root.*-rel and mail.root.* in prefs display via Config Editor or in prefs.js file by Text Editor.

mail.root.* is better replaced by "absolute path obtained from mail.root.*-rel" as soon as posibble, instead of "next account creation time which uses the mail.root.*-rel" only.
If status of "mail.root.* exists but mail.root.*-rel doesn't exist" is detected by someone, mail.root.*-rel is better created immediately.
FYI.
If "mail directory location" is changed from "directory under profile directory" to different "directory outside of Tb's profile directory" by user, mismatch between ".directory-rel in prefs.js" and "directory used before copy of Tb's profile directory" can occur, if user copies Tb's profile directory.

  ProfA       : #1.directory-rel =                         [ProfD]Mail/x.y.z
                #1.directory     = ...\Thunderbird\Profiles\ProfA\Mail\x.y.z
                #2.directory-rel = [ProfD]../../../\DifferentXX/p.q.r
                #2.directory     =              ...\DifferentXX\p.q.r

(1) If profile is copied to same location/level/depth.
    (...\Thunderbird\Profiles\CopyOfProfA)
  CopyOfProfA : #1.directory-rel =                         [ProfD]Mail/x.y.z
                #1.directory     = ...\Thunderbird\Profiles\ProfA\Mail\x.y.z
                   absolute path of [ProfD]Mail/x.y.z = 
                                   ...\Thunderbird\Profiles\CopyOfProfA\Mail\x.y.z
                   #1.directory is changed to this one after restart of Tb.
                #2.directory-rel =  [ProfD]../../../DifferentXX/p.q.r
                #2.directory     =              ...\DifferentXX\p.q.r
                   absolute path of [ProfD]../../../DifferentXX/p.q.r = 
                                                ...\DifferentXX\p.q.r
     Because "relative path of a directory outside of Tb's profile directory
              from Tb's profile directory"
     is not changed by "copy of profile directory to same directory level",
     there is no need to touch pres.js after profile copy,
     as far as profile directory is copied to same directory location/level/depth.

(2)  If profile is copied to deeper directory level.
     (...\Deeper\Thunderbird\Profiles\CopyOfProfA)
  CopyOfProfA : #1.directory-rel =                         [ProfD]Mail/x.y.z
                #1.directory     = ...\Thunderbird\Profiles\ProfA\Mail\x.y.z
                   absolute path of [ProfD]Mail/x.y.z = 
                                   ...\Deeper\Thunderbird\Profiles\CopyOfProfA\Mail\x.y.z
                   #1.directory is changed to this one after restart of Tb.
                #2.directory-rel =  [ProfD]../../../DifferentXX/p.q.r
                #2.directory     =              ...\DifferentXX\p.q.r
                   absolute path of [ProfD]../../../DifferentXX/p.q.r = 
                                                 ...\Deeper\DifferentXX\p.q.r
     Because "relative path of a directory outside of Tb's profile directory
              from Tb's profile"
     is changed by "copy of profile directory to deeper directory level",
     user has to correct mail directory location after profile copy.
     This occurs upon
        Copy profile directory in standard profile location of Tb on Win-XP
          to standard profile location of Tb on Vista, Win7, Win8,
     because Tb's standard profile location on Vista,Win7/8 is one level deeper than XP.
     This is same in "copy of Tb's profile directory to non-standard location".
Mystery in example of "bug opener of bug 921371".
- mail.server.server#.directory-rel = [ProfD]Mail/hostname is kept, correctly, as expected,
. even after profile copy from Win-XP to non XP(or backup on XP, restore on non-XP using
  MozBackup).
  This means problem of (2) didn't occur in his case.
- User says that he did "migration of Tb from XP to non-XP", which requires copy of Tb's
  profile directory from XP to non-XP, but he says that he never touched prefs.js content
  by himself.
- Even though he didn't touch prefs.js by himself,
  mail.root.*-rel points non [ProfD]/Mail, non [ProfD]/ImapMail,
  which may point "old/copied/original profile directory"
  in his prefs.js which is currently used on non-XP.

Bug 921371 comment #61 is for same situation as Bug 921371 comment #0.
But cause may be different.
1. User copied Tb's profile directory from perhaps "non standard profile location" to different place on XP.
   Path=E:\Koen\EmailAndNewsTB\ktanghe
2. mail.server.server#.directory-rel = [ProfD]Mail/pop3.x.y.z is kept, is used,
   as intended, as expected.
3. However, mail.root.* and mail.root.*-rel looks to point copied/original profile directory, or to point location where user intentionally set in the past.
> mail.root.pop3;C:\Documents and Settings\ktanghe\My Documents\EmailAndNewsTB\ktanghe\Mail
> mail.root.pop3-rel;[ProfD]../../../../C:/Documents and Settings/ktanghe/My Documents/EmailAndNewsTB/ktanghe/Mail

I can't imagine other than phenomenon like next in this case, because I can't think pointed location is set by user's intentional change. It looks for me "profile directory location where user copied profile directory in the past".
(A) mail.server.server#.directory-rel support and mail.root.*-rel support was done
    in different Tb release. So, mail.server.server#.directory-rel=[ProfD]Mail and
    "mail.root.* only without mail.root.*-rel" could happen at same time in the past.
    User repeated Tb's profile copy, but didn't modify mail.root.* after profile copy.
    After it, mail.root.*-rel was generated by Tb who supports mail.root.*-rel
    from mail.root.* upon new account creation.
    As the account is already deleted, we can't see "mail directory under My Documents"
    in his prefs.js.
(B) mail.root.*-rel was set to [ProfD]/Mail by Tb who supports mail.root.*-rel.
    But mail.root.* was not updated because new mail account was not created by user.
    mail.root.*-rel was deleted by someone before or when profile directory was copied.
    After it, mail.root.*-rel was generated by Tb who supports mail.root.*-rel
    from mail.root.* upon new mail account creation.
    As the account is already deleted, we can't see "mail directory under My Documents"
    in his prefs.js.
> I didn't want to hardcode the allowed subdirs ("Mail", "ImapMail", "News") in the check function
It's perhaps hard coded(or psuedo hard coded) in Tb already.

> http://mxr.mozilla.org/comm-central/search?string=NS_APP_MAIL_
> http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMailDirServiceDefs.h#6
> 23 #define NS_APP_MAIL_50_DIR                      "MailD"
> 24 #define NS_APP_IMAP_MAIL_50_DIR                 "IMapMD"
> 25 #define NS_APP_NEWS_50_DIR                      "NewsD"
This is defined in nsMailDirServiceDefs.h only, but I think its included by other components.

NS_APP_*_50_DIR is passed to NS_GetPersistentFile() as dirServiceProp by nsMailDirProvider.cpp, nsMovemailService.cpp, nsPop3Service.cpp.
> http://mxr.mozilla.org/comm-central/search?string=PREF_MAIL_ROOT_

NS_GetPersistentFile() searchs mail.root.*-rel, mail.root.*, dirServiceProp, in this order.
> http://mxr.mozilla.org/comm-central/source/mailnews/base/util/nsMsgUtils.cpp#1168
If both mail.root.*-rel and mail.root.* are not found, dirService->Get(dirServiceProp) is called. 
> http://mxr.mozilla.org/comm-central/source/mailnews/base/util/nsMsgUtils.cpp#1212
mail.root.*-rel, mail.root.*, dirServiceProp, obtained directory path, are passed to NS_SetPersistentFile().
This is perhaps reason why string of "Mail" or "ImapMail" or "News" is used when both mail.root.*-rel and mail.root.* is not defined, or why string of "Mail" or "ImapMail" or "News" is set in mail.root.*-rel.

I think you can call dirService->Get(dirServiceProp) with following constant in order to obtain Tb's internal default mail directory location even when mail.root.*-rel(and mail.root.*) is altered by user intentionally.
> 23 #define NS_APP_MAIL_50_DIR                      "MailD"
> 24 #define NS_APP_IMAP_MAIL_50_DIR                 "IMapMD"
> 25 #define NS_APP_NEWS_50_DIR                      "NewsD"
Interesting idea. I think we can access the dirservice from the account manager's Javascript. BUt we would need to hardcode the "MailD", "IMapMD", "NewsD" identifiers. But maybe we can do that for the known server types. In that case we can remove consulting server.defaultLocalPath because we can construct it ourselves (for the known types). The server.defaultLocalPath (which uses the *.root.* prefs) can be modified by the user to actually point into a dangerous folder and we currently accept it. For unknown server types, we would still use server.defaultLocalPath to cover rkent's case.
What do you guys think about that?
Flags: needinfo?(neil)
Flags: needinfo?(iann_bugzilla)
Yes, seems like an interesting idea. It is a shame some of the internal types are also unknown server types, but that is the way it is at the moment.
Flags: needinfo?(iann_bugzilla)
Flags: needinfo?(neil)
Removing myslef on all the bugs I'm cced on. Please NI me if you need something on MailNews Core bugs from me.

Mark, is this still an issue in TB Daily 91.0a1?
If yes, could you provide STR?
Also, what exactly is this about? Can the summary be adjusted to match the problem better?

Flags: needinfo?(standard8)

Sorry, I don't remember enough detail about this, I also didn't file the original and don't use Windows. Reading through the comments on the bugs should help anyone understand what's happening looks like aceman did a good job of documenting things. I really suspect it hasn't been fixed in the meantime, given there was only a partial fix to begin with.

Flags: needinfo?(standard8)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.