Bugzilla

Review of attachment 9061433 [details] [diff] [review]:
-----------------------------------------------------------------

Probably needs some fixes and doesn't build. But after fixes, it seems to work. But haven't tried to add a new account. Will wait for next patch to try that.

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +448,5 @@
>      // From when we first create the account until we have created some folders,
>      // we can change the store type.
>      (*_retval)->SetBoolValue("canChangeStoreType", true);
> +
> +    // create our default clientid

This occurs for every new account, right? Or is it only accounts that use IMAP? At this point we don't know if the server supports clientid, right?

@@ +450,5 @@
>      (*_retval)->SetBoolValue("canChangeStoreType", true);
> +
> +    // create our default clientid
> +    nsCString clientId;
> +    // Dan: Should we be checking if the clientid is already set?

Remove comment. It appears that check for already set is being done.

@@ +469,5 @@
> +        if (NS_FAILED(rv) || clientId.IsEmpty()) {
> +            return NS_ERROR_FAILURE;
> +        }
> +      }
> +      (*_retval)->SetCharValue("clientid", clientId);

Is this saving clientid to mail.server.serverX.clientid or to mail.smtpserver.smtpX.clientid. My guess is the former.

::: mailnews/imap/public/nsIImapIncomingServer.idl
@@ +52,5 @@
>  
> +  /**
> +   * clientid if required
> +   */
> +  attribute ACString clientid;

Is this used somewhere?

::: mailnews/imap/src/nsImapIncomingServer.cpp
@@ +2052,5 @@
> +  // If the pref value is already empty, ClearUserPref will return
> +  // NS_ERROR_UNEXPECTED, so don't check the rv here.
> +  mPrefBranch->ClearUserPref("clientid");
> +  return NS_OK;
> +}

Is this ever called with an empty aClientid string?

::: mailnews/imap/src/nsImapProtocol.cpp
@@ +942,1 @@
>        }

Maybe above needs 
#include "nsIUUIDGenerator.h"

@@ +6105,5 @@
> +  nsCString command(GetServerCommandTag());
> +  command += " CLIENTID TB-UUID ";
> +  command += m_clientId;
> +  command += CRLF;
> +  nresult rv = SendDataParseIMAPandCheckForNewMail(command.get(), nullptr);

nsresult rv = ...

@@ +8948,5 @@
>      }
>    }
>  
> +  // We check for clientid support and whether tls/ssl running
> +  // by checking the server capability flags for CLIENTID and 

blank at EOL

Review of attachment 9061433 [details] [diff] [review]:
-----------------------------------------------------------------

Probably needs some fixes and doesn't build. But after fixes, it seems to work. But haven't tried to add a new account. Will wait for next patch to try that.

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +448,5 @@
>      // From when we first create the account until we have created some folders,
>      // we can change the store type.
>      (*_retval)->SetBoolValue("canChangeStoreType", true);
> +
> +    // create our default clientid

This occurs for every new account, right? Or is it only accounts that use IMAP? At this point we don't know if the server supports clientid, right?

@@ +450,5 @@
>      (*_retval)->SetBoolValue("canChangeStoreType", true);
> +
> +    // create our default clientid
> +    nsCString clientId;
> +    // Dan: Should we be checking if the clientid is already set?

Remove comment. It appears that check for already set is being done.

@@ +469,5 @@
> +        if (NS_FAILED(rv) || clientId.IsEmpty()) {
> +            return NS_ERROR_FAILURE;
> +        }
> +      }
> +      (*_retval)->SetCharValue("clientid", clientId);

Is this saving clientid to mail.server.serverX.clientid or to mail.smtpserver.smtpX.clientid. My guess is the former.

::: mailnews/imap/public/nsIImapIncomingServer.idl
@@ +52,5 @@
>  
> +  /**
> +   * clientid if required
> +   */
> +  attribute ACString clientid;

Is this used somewhere? Edit: Never mind, used to define Get/SetClientid().

::: mailnews/imap/src/nsImapIncomingServer.cpp
@@ +2052,5 @@
> +  // If the pref value is already empty, ClearUserPref will return
> +  // NS_ERROR_UNEXPECTED, so don't check the rv here.
> +  mPrefBranch->ClearUserPref("clientid");
> +  return NS_OK;
> +}

Is this ever called with an empty aClientid string?

::: mailnews/imap/src/nsImapProtocol.cpp
@@ +942,1 @@
>        }

Maybe above needs 
#include "nsIUUIDGenerator.h"

@@ +6105,5 @@
> +  nsCString command(GetServerCommandTag());
> +  command += " CLIENTID TB-UUID ";
> +  command += m_clientId;
> +  command += CRLF;
> +  nresult rv = SendDataParseIMAPandCheckForNewMail(command.get(), nullptr);

nsresult rv = ...

@@ +8948,5 @@
>      }
>    }
>  
> +  // We check for clientid support and whether tls/ssl running
> +  // by checking the server capability flags for CLIENTID and 

blank at EOL