Last Comment Bug 875675 - prefs.js entry of mail.account.accountX.identities and mail.account.accountX.server is not removed by account deletion
: prefs.js entry of mail.account.accountX.identities and mail.account.accountX....
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Account Manager (show other bugs)
: 17
: All All
: -- minor (vote)
: Thunderbird 24.0
Assigned To: :aceman
:
Mentors:
Depends on: 880602
Blocks:
  Show dependency treegraph
 
Reported: 2013-05-23 23:52 PDT by WADA
Modified: 2014-10-06 03:43 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (1.53 KB, patch)
2013-06-08 15:15 PDT, :aceman
neil: review+
Details | Diff | Review

Description WADA 2013-05-23 23:52:37 PDT
prefs.js entry of mail.account.accountX.identities and mail.account.accountX.server is not removed by account deletion.

[Build ID]
- Thunderbird 17.0.6 (Application Build ID : 20130509153421)
- Thunderbird 24.0a1 (Application Build ID : 20130523030940)

[Steps to reproduce]

(1) Create new Tb profile, start  Tb
(2) At first start, cancel account creation.
    Via Toolbar Customize menu, show Menu Bar for ease of test.
    View/Folders/Unified,
    in order to force "server1" for smart mailboxes account,
    in order to avoid disturbing of account#/server# assignment.

    mail.accountmanager.accounts = account1
    mail.accountmanager.defaultaccount = account1
    mail.account.lastKey = 1
    - mail.account.account1.server = server1
      mail.server.server1.type     = none (smart mailboxes)

(3) Create dummy POP3 account or dummy news account.
    Local Folders is added, and created accout is added.

    mail.accountmanager.accounts = account1,account2,account3
    mail.accountmanager.defaultaccount = account1
    mail.accountmanager.localfoldersserver = server2
    mail.account.lastKey = 3   <= incremented
    - mail.account.account1.server = server1
      mail.server.server1.type = none (smart mailboxes)
    (following is added)
    - mail.account.account2.server = server2
      mail.server.server2.type = none (Local Folders)
    - mail.account.account3.identities = idX, idY, ..., idZ
      mail.account.account3.server = server3
      mail.identity.id?.useremail = ???@?.?.?
      mail.server.server3.type    = none  (pop3 or news)
    If POP3, and if the POP3 account is set as "Default account",
    mail.accountmanager.defaultaccount = account3

(4) Delete dummy POP3 account or dummy news account.
    mail.server.server3.xxx, and mail.identity.id?.xxx is removed,
    and account is removed from mail.accountmanager.accounts.
    However, mail.account.account3.identities = idX, idY, ..., idZ
         and mail.account.account3.server     = server3
    is not removed.

    mail.accountmanager.accounts = account1,account2
    mail.accountmanager.defaultaccount = account1
         (if default account is deleted, reset to first account)
    mail.accountmanager.localfoldersserver = server2
    mail.account.lastKey = 3   <= unchanged
    - mail.account.account1.server = server1
      mail.server.server1.type = none (smart mailboxes)
    - mail.account.account2.server = server2
      mail.server.server2.type = none (Local Folders)
    (following is not removed)
    - mail.account.account3.identities = idX, idY, ..., idZ
      mail.account.account3.server     = server3

(5) Create dummy POP3 account or dummy news account.
    Because mail.account.lastKey + 1 is used,
    accont4 is normally created and added.
    Because mail.server.server3.xxx and mail.identity.id?.useremail
    doesn't exist, these are normally created and used.

    mail.accountmanager.accounts = account1,account2,account4
    mail.accountmanager.defaultaccount = account1
    mail.accountmanager.localfoldersserver = server2
    mail.account.lastKey = 4   <= incremented
    - mail.account.account1.server = server1
      mail.server.server1.type = none (smart mailboxes)
    - mail.account.account2.server = server2
      mail.server.server2.type = none (Local Folders)
    (following still remains)
    - mail.account.account3.identities = idX, idY, ..., idZ
      mail.account.account3.server     = server3
    (following is added)
    - mail.account.account4.identities = idX, idY, ..., idZ
      mail.account.account4.server     = server3
      mail.identity.id?.useremail = ???@?.?.?
      mail.server.server3.type    = none  (pop3 or news)
    If POP3, and if the POP3 account is set as "Default account",
    mail.accountmanager.defaultaccount = account4

Problem like "duplicate serverX entry use" was resolved by above mechanism.
However, if creation/deletion of account is repeated, garbage of mail.account.accountX.identities and mail.account.accountX.server increases infinitely.

"Removal of mail.account.accountX.identities/server upon account deletion" may search entries from mail.account.lastKey.
Following is perhaps needed.
(1) Correctly remove mail.account.accountX.identities/server.
(2) Decrease mail.account.lastKey, when account of largest number is deleted.
Comment 1 :aceman 2013-05-24 00:01:31 PDT
(In reply to WADA from comment #0)
> (2) Decrease mail.account.lastKey, when account of largest number is deleted.

We intentionally keep the lastKey ever increasing so that account keys are never repeated. See bug 485839 for details.
Comment 2 WADA 2013-05-24 01:06:20 PDT
(In reply to :aceman from comment #1)

I didn't know final solution of bug 485839 was good practice of "never resuse accountKey".
It looks Account Manager simply forget to remove mail.account.accountX.identities and mail.account.accountX.server.

Another issue in account deletion what is seen in above test.
  When default account is deleted,
  mail.accountmanager.defaultaccount is set to first account.
  Note: In older Tb, delete of default account was rejected
        by graying-out of "Remove account" menu.
This produces status of "non-pop3/no-imap account is set as default account".
- Already known problem.
  When default account is deleted, or when news account only is defined,
  Local Folders account or news account is set as default account.
  Because Local Folders doesn't have associated identity,
  and because identity for news account is not always setup well for
  mail sending, some features(e.g. reply by filter) doesn't work as
  expected, and it produces user's confusion.
- Phenomenon observed in above test.
  Even though hidden "smart mailboxes", it's set as default account
  by Tb when user deleted default account.

"Fall back of default account upon account deletion" should be;
(1) First non Global-Inbox-use pop3 account or imap account.
(2) First Global-Inbox-use pop3 account.
(3) If no pop3/imap account, "Local Folders" account.
Comment 3 :aceman 2013-05-24 02:13:24 PDT
Ok, but that is another bug.
In this one I look at the removal of the prefs.
Comment 4 WADA 2013-06-06 01:14:06 PDT
Memo for default account issue.
An example where first account is set as default account.
> http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgAccountManager.cpp#758
Comment 5 :aceman 2013-06-06 01:24:29 PDT
(In reply to WADA from comment #2)
> Another issue in account deletion what is seen in above test.
>   When default account is deleted,
>   mail.accountmanager.defaultaccount is set to first account.
>   Note: In older Tb, delete of default account was rejected
>         by graying-out of "Remove account" menu.
> This produces status of "non-pop3/no-imap account is set as default account".
> - Already known problem.
>   When default account is deleted, or when news account only is defined,
>   Local Folders account or news account is set as default account.
>   Because Local Folders doesn't have associated identity,
>   and because identity for news account is not always setup well for
>   mail sending, some features(e.g. reply by filter) doesn't work as
>   expected, and it produces user's confusion.

If the default account is deleted, first account that can be set as default (server attribute) is set as default. So there should not be a problem here. Can you describe steps to reproduce when an account is set as default that is Local folders, but there is another free POP3/IMAP account that should be selected instead?

> "Fall back of default account upon account deletion" should be;
> (1) First non Global-Inbox-use pop3 account or imap account.
> (2) First Global-Inbox-use pop3 account.
> (3) If no pop3/imap account, "Local Folders" account.

This could be done if needed.

(In reply to WADA from comment #4)
> Memo for default account issue.
> An example where first account is set as default account.
> > http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgAccountManager.cpp#758

This is only partially true. Later in the code, it is also checked if the chosen account can be set as default. Local folders does not satisfy that so it is not chosen. Only if Local folders is the last account remaining, it is chosen as default.
Comment 6 WADA 2013-06-06 16:10:50 PDT
(In reply to :aceman from comment #5)
> Can you describe steps to reproduce when an account is set as default that is Local folders, 
> but there is another free POP3/IMAP account that should be selected instead?

To force "Local Folders"==server1, "first created account != pop3/imap" is needed, because current Tb uses server1=pop3 or imap account, server2=Lcal Folders, upon first account creation.
(1) Create News&Blog account => Local Folders==server1
(2) Create any number of accounts, Delete all accounts
    => default server==server1==Local Folders
(3) Create account(s)

Steps I wrote in comment #1 is method to force "smart mailboxes"==server1 by entering "View/Folders/Unified" before first account creation. Following is formal version of it.
(1) Create pop3 or imap account => account2=server2=Local Folders
(2) Delete account => account1/server1 is deleted. server1 is free
(3) View/Folders/Unified => account3=server1=="smart mailboxes"
                            account2=server2=="Local Folders"
(4) Create account(s)
Comment 7 WADA 2013-06-07 00:52:17 PDT
FYI.
Bug 880464 existed. It looks that RSS(Blog&News Feeds) is currently "account who can be default account", as pop3/imap/movemail account is so.
Default account setting after account deletion should be;
(1) Currently defaulted pop3/imap/movemail account
(2) First pop3/imap/movemail account
(3) If no account of type (1), Local Folders.
(4) If no "Local Folders" account, defaultaccount=null or account0,
    or terminate Tb with error message of "prefs.js is broken".
This should be invoked upon deletion of other than "currently defaulted account" too, because other than (1), including "smart mailboxes", "Blog&News Feeds", "Local Folders", nntp account, may be already set as default account by older Tb.
Comment 8 :aceman 2013-06-08 15:15:10 PDT
Created attachment 760153 [details] [diff] [review]
patch

This fixes the original problem of not clearing out the prefs. RemoveBranch() fails on nullptr argument.
Comment 9 neil@parkwaycc.co.uk 2013-06-12 15:36:08 PDT
Comment on attachment 760153 [details] [diff] [review]
patch

Oops. Sorry about that. (Do we need to uplift this anywhere?)
Comment 10 :aceman 2013-06-12 15:40:32 PDT
Probably not. It leaves unused prefs but otherwise should be harmless.
Comment 11 Ryan VanderMeulen [:RyanVM] 2013-06-13 06:38:09 PDT
https://hg.mozilla.org/comm-central/rev/b1fd2d8e66d2
Comment 12 David.P 2014-10-06 01:18:28 PDT
This bug has fu**ed up my account configuration.

Please, what has to be removed manually from prefs.js in order to ERADICATE a certain account from there?

Is removing everything starting with:

- mail.server.server
- mail.identity.id

...enough, or do I have to remove anything else?
Comment 13 :aceman 2014-10-06 03:27:09 PDT
Also try mail.account.accountX .
But this bug should be fixed since TB24. So what is your TB version that you see the problem? And how did it break your config?
Comment 14 David.P 2014-10-06 03:30:38 PDT
Well I have "masses" of old account stuff in the prefs.js. Somehow it got broken such that accounts can't be deleted nor added anymore from the account manager.

So I remove everything starting with:

- mail.server.server
- mail.identity.id
- mail.account.account

...for any given account?
Comment 15 :aceman 2014-10-06 03:39:34 PDT
Yes, in mail.account.accountX prefs you'll see which identities (e.g. idZ) and server (serverY) belong to that account. Then clear out the lines of mail.server.serverY.* corresponding to that server ID. And mail.identity.idZ.*.

Also check the value of mail.account.lastKey pref. That one says which account ID is the last one used. Next created account ID will be mail.account.lastKey+1 . If the value seems bogus (lower than ids of already created accounts), you may try to increase it.
Comment 16 David.P 2014-10-06 03:43:04 PDT
OK thank you very much, I'll do that.

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