482477 - add a way to have a hidden account

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[David :Bienvenu]

The code in bug 305340 for Smart Mailboxes folder pane mode requires a Smart Mailboxes account - but this account should be hidden from all other UI (e.g., the account manager, the move/copy menu items, etc). I'm thinking of adding a hidden attribute to nsIMsgAccount and making the code that lists accounts ignore the hidden ones, so that you have to find the hidden one(s) by name, which should cause minimal code perturbation (otherwise, I'd need to find all the places that list accounts and teach them to ignore hidden accounts).

Comment 1

[David :Bienvenu]

Attachment: proposed fix

I'm going to run with this a bit, but it seems to be working.

Comment 2

[[e-mail]]

Comment on attachment 367789 [details] [diff] [review]
proposed fix

>+  // enumerate by going through the list of accounts, so that we
>+  // get the order correct
>+  m_incomingServers.Enumerate(hashGetNonHiddenServersToArray,
>+                              (void *)(nsISupportsArray*)servers);
I guess the comment is wrong here, since the old code didn't do this ;-)

>+  // get all servers, including hidden servers.
>+  nsresult GetAllServersInternal(nsISupportsArray **_retval);
Hmm... is it possible to convert the callers to use m_incomingServers?

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 367789 [details] [diff] [review]
proposed fix

In general looks reasonable, but I'd like to see a response to Neil's comments and/or updated patch before reviewing.

Comment 4

[David :Bienvenu]

I think Neil's comments are reasonable - I'll try to do a new patch today.

Comment 5

[David :Bienvenu]

Attachment: fix partially addressing Neil's comments

It turned out to be relatively easy to get rid of one of the uses of GetAllServersInternal but the other one (for saving virtualfolders.dat) was a bit ugly.

I added a call to LoadAccounts to FindServerByURI because the send later stuff was trying to resolve the send later folder uri before the account manager seems to have loaded accounts - I'm not sure why I just ran into that, but it seems like the right thing to do.

Comment 6

[[e-mail]]

Comment on attachment 370488 [details] [diff] [review]
fix partially addressing Neil's comments

>+      nsIOutputStream *outputStream = * (nsIOutputStream **) data;
>+      if (!outputStream)
>       {
>+        nsCOMPtr<nsILocalFile> file;
>+        GetVirtualFoldersFile(file);
>+        rv = NS_NewLocalFileOutputStream(&outputStream,
>+                                         file,
>+                                         PR_CREATE_FILE | PR_WRONLY | PR_TRUNCATE,
>+                                         0664);
>+        NS_ENSURE_SUCCESS(rv, PL_DHASH_STOP);
>+        * (nsIOutputStream **) data = outputStream;
>+        WriteLineToOutputStream("version=", "1", outputStream);
I assume this is better than nsIOutputStream* & or * *outputStream?

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 370488 [details] [diff] [review]
fix partially addressing Neil's comments

>diff --git a/mailnews/base/public/nsIMsgAccountManager.idl b/mailnews/base/public/nsIMsgAccountManager.idl
>--- a/mailnews/base/public/nsIMsgAccountManager.idl
>+++ b/mailnews/base/public/nsIMsgAccountManager.idl
>@@ -102,27 +102,28 @@ interface nsIMsgAccountManager : nsISupp
>   /* account list stuff */
> 
>   /* defaultAccount should always be set if there are any accounts
>    * in the account manager. You can only set the defaultAccount to an
>    * account already in the account manager */
>   attribute nsIMsgAccount defaultAccount;
> 
>   /**
>-   * ordered list of all accounts, by the order they are in the prefs
>+   * ordered list of all accounts, by the order they are in the prefs.
>+   * Accounts with hidden servers are not returned.

nit: as you're adding the . please capitalise the 'ordered'.

Comment 8

[David :Bienvenu]

fix checked in

Comment 9

[David :Bienvenu]

I landed a patch that didn't address Neil's comments - I've fixed that now.