fix for #42105.

when creating a news account, don't set the username to be the username
part of the email address (from the wizard).

this fixes bug #154213, a problem with the account picker in the subscribe dialog
cause by the bogus username

also, some code cleanup

r/sr=bienvenu

mailnews/base/prefs/resources/content/AccountWizard.js

@@ -348,12 +348,16 @@ function PageDataToAccountData(pageData, accountData)
 // (but don't fill in any fields, that's for finishAccount()
 function createAccount(accountData)
 {
-
     var server = accountData.incomingServer;
-    dump("am.createIncomingServer(" + server.username + "," +
+    
+    // for news, username is always null
+    var username = (server.type == "nntp") ? null : server.username;
+
+    dump("am.createIncomingServer(" + username + "," +
                                       server.hostName + "," +
                                       server.type + ")\n");
-    var server = am.createIncomingServer(server.username,
+    
+    var server = am.createIncomingServer(username,
                                          server.hostName,
                                          server.type);
     
@@ -375,8 +379,8 @@ function createAccount(accountData)
 
 // given an accountData structure, copy the data into the
 // given account, incoming server, and so forth
-function finishAccount(account, accountData) {
-
+function finishAccount(account, accountData) 
+{
     if (accountData.incomingServer) {
 
         var destServer = account.incomingServer;
@@ -496,7 +500,7 @@ function copyObjectToInterface(dest, src) {
         catch (ex) {
             dump("Error copying the " +
                  i + " attribute: " + ex + "\n");
-            dump("(This is ok if this is a ServerType-* attribute)\n");
+            dump("[This is ok if this is a ServerType-* attribute, or if this is a readonly attribute (like receiptHeaderType)]\n");
         }
     }
 }
@@ -842,11 +846,17 @@ function PrefillAccountForIsp(ispName)
 // - anything else?
 function FixupAccountDataForIsp(accountData)
 {
+    // no fixup for news
+    // setting the username does bad things
+    // see bugs #42105 and #154213
+    if (accountData.incomingServer.type == "nntp")
+      return;
+
     var email = accountData.identity.email;
     var username;
 
     if (email) {
-		username = getUsernameFromEmail(email);
+      username = getUsernameFromEmail(email);
     }
     
     // fix up the username

mailnews/base/src/nsMessengerMigrator.cpp

@@ -2323,7 +2323,8 @@ nsMessengerMigrator::MigrateNewsAccount(nsIMsgIdentity *identity, const char *ho
 
     // create the server
 	nsCOMPtr<nsIMsgIncomingServer> server;
-    rv = accountManager->CreateIncomingServer(nsnull, hostname.get(), "nntp",
+    // for news, username is always null
+    rv = accountManager->CreateIncomingServer(nsnull /* username */, hostname.get(), "nntp",
                               getter_AddRefs(server));
     NS_ENSURE_SUCCESS(rv,rv);
  

mailnews/base/src/nsMsgAccount.cpp

@@ -88,9 +88,10 @@ nsMsgAccount::Init()
 }
 
 nsresult
-nsMsgAccount::getPrefService() {
-
-  if (m_prefs) return NS_OK;
+nsMsgAccount::getPrefService() 
+{
+  if (m_prefs) 
+    return NS_OK;
   
   return nsServiceManager::GetService(kPrefServiceCID,
                                       NS_GET_IID(nsIPref),
@@ -102,17 +103,14 @@ nsMsgAccount::GetIncomingServer(nsIMsgIncomingServer * *aIncomingServer)
 {
   NS_ENSURE_ARG_POINTER(aIncomingServer);
 
-  nsresult rv = NS_OK;
   // create the incoming server lazily
   if (!m_incomingServer) {
     // ignore the error (and return null), but it's still bad so assert
-    rv = createIncomingServer();
+    nsresult rv = createIncomingServer();
     NS_ASSERTION(NS_SUCCEEDED(rv), "couldn't lazily create the server\n");
   }
   
-  
-  *aIncomingServer = m_incomingServer;
-  NS_IF_ADDREF(*aIncomingServer);
+  NS_IF_ADDREF(*aIncomingServer = m_incomingServer);
 
   return NS_OK;
 }

mailnews/base/src/nsMsgAccountManager.cpp

@@ -426,16 +426,13 @@ nsMsgAccountManager::CreateIncomingServer(const char* username,
                                           nsIMsgIncomingServer **_retval)
 {
   NS_ENSURE_ARG_POINTER(_retval);
-  nsresult rv;
 
-  rv = LoadAccounts();
+  nsresult rv = LoadAccounts();
   if (NS_FAILED(rv)) return rv;
 
   nsCAutoString key;
   getUniqueKey(SERVER_PREFIX, &m_incomingServers, key);
-  rv = createKeyedServer(key.get(), username, hostname, type, _retval);
-
-  return rv;
+  return createKeyedServer(key.get(), username, hostname, type, _retval);
 }
 
 nsresult

mailnews/base/util/nsMsgIdentity.cpp

@@ -644,9 +644,9 @@ NS_IMETHODIMP
 nsMsgIdentity::GetRequestReturnReceipt(PRBool *aVal)
 {
   NS_ENSURE_ARG_POINTER(aVal);
-  nsresult rv;
+
   PRBool useCustomPrefs = PR_FALSE;
-  rv = GetBoolAttribute("use_custom_prefs", &useCustomPrefs);
+  nsresult rv = GetBoolAttribute("use_custom_prefs", &useCustomPrefs);
   NS_ENSURE_SUCCESS(rv, rv);
   if (useCustomPrefs)
   {
@@ -664,9 +664,9 @@ NS_IMETHODIMP
 nsMsgIdentity::GetReceiptHeaderType(PRInt32 *aType)
 {
   NS_ENSURE_ARG_POINTER(aType);
-  nsresult rv;
+
   PRBool useCustomPrefs = PR_FALSE;
-  rv = GetBoolAttribute("use_custom_prefs", &useCustomPrefs);
+  nsresult rv = GetBoolAttribute("use_custom_prefs", &useCustomPrefs);
   NS_ENSURE_SUCCESS(rv, rv);
   if (useCustomPrefs)
   {

mailnews/news/src/nsNntpService.cpp

@@ -979,12 +979,12 @@ nsNntpService::ConstructNntpUrl(const char *urlString, nsIUrlListener *aUrlListe
 }
 
 nsresult
-nsNntpService::CreateNewsAccount(const char *username, const char *hostname, PRBool isSecure, PRInt32 port, nsIMsgIncomingServer **server)
+nsNntpService::CreateNewsAccount(const char *aHostname, PRBool aIsSecure, PRInt32 aPort, nsIMsgIncomingServer **aServer)
 {
-	nsresult rv;
-	// username can be null.
-	if (!hostname || !server) return NS_ERROR_NULL_POINTER;
-	
+    NS_ENSURE_ARG_POINTER(aHostname);
+    NS_ENSURE_ARG_POINTER(aServer);
+
+    nsresult rv;	
 	nsCOMPtr <nsIMsgAccountManager> accountManager = do_GetService(NS_MSGACCOUNTMANAGER_CONTRACTID, &rv);
     NS_ENSURE_SUCCESS(rv,rv);
 
@@ -992,13 +992,14 @@ nsNntpService::CreateNewsAccount(const char *username, const char *hostname, PRB
 	rv = accountManager->CreateAccount(getter_AddRefs(account));
 	if (NS_FAILED(rv)) return rv;
 
-	rv = accountManager->CreateIncomingServer(username, hostname, "nntp", server);
+    // for news, username is always null
+	rv = accountManager->CreateIncomingServer(nsnull /* username */, aHostname, "nntp", aServer);
 	if (NS_FAILED(rv)) return rv;
 
-	rv = (*server)->SetIsSecure(isSecure);
+	rv = (*aServer)->SetIsSecure(aIsSecure);
 	if (NS_FAILED(rv)) return rv;
 	
-	rv = (*server)->SetPort(port);
+	rv = (*aServer)->SetPort(aPort);
 	if (NS_FAILED(rv)) return rv;
 
 	nsCOMPtr <nsIMsgIdentity> identity;
@@ -1011,11 +1012,11 @@ nsNntpService::CreateNewsAccount(const char *username, const char *hostname, PRB
     NS_ENSURE_SUCCESS(rv,rv);
 
 	// the identity isn't filled in, so it is not valid.
-	rv = (*server)->SetValid(PR_FALSE);
+	rv = (*aServer)->SetValid(PR_FALSE);
 	if (NS_FAILED(rv)) return rv;
 
 	// hook them together
-	rv = account->SetIncomingServer(*server);
+	rv = account->SetIncomingServer(*aServer);
 	if (NS_FAILED(rv)) return rv;
 	rv = account->AddIdentity(identity);
 	if (NS_FAILED(rv)) return rv;
@@ -1031,14 +1032,12 @@ nsresult
 nsNntpService::GetProtocolForUri(nsIURI *aUri, nsIMsgWindow *aMsgWindow, nsINNTPProtocol **aProtocol)
 {
   nsCAutoString hostName;
-  nsCAutoString userPass;
   nsCAutoString scheme;
   nsCAutoString path;
   PRInt32 port = 0;
   nsresult rv;
   
   rv = aUri->GetAsciiHost(hostName);
-  rv = aUri->GetUserPass(userPass);
   rv = aUri->GetScheme(scheme);
   rv = aUri->GetPort(&port);
   rv = aUri->GetPath(path);
@@ -1082,7 +1081,7 @@ nsNntpService::GetProtocolForUri(nsIURI *aUri, nsIMsgWindow *aMsgWindow, nsINNTP
   //
   // xxx todo what if we have two servers on the same host, but different ports?
   // or no port, but isSecure (snews:// vs news://) is different?
-  rv = accountManager->FindServer(userPass.get(),
+  rv = accountManager->FindServer("",
                                 hostName.get(),
                                 "nntp",
                                 getter_AddRefs(server));
@@ -1126,7 +1125,7 @@ nsNntpService::GetProtocolForUri(nsIURI *aUri, nsIMsgWindow *aMsgWindow, nsINNTP
               port = SECURE_NEWS_PORT;
           }
 	  }
-	  rv = CreateNewsAccount(userPass.get(),hostName.get(),isSecure,port,getter_AddRefs(server));
+	  rv = CreateNewsAccount(hostName.get(), isSecure, port, getter_AddRefs(server));
   }
    
   if (NS_FAILED(rv)) return rv;
@@ -1196,7 +1195,7 @@ nsNntpService::RunNewsUrl(nsIURI * aUri, nsIMsgWindow *aMsgWindow, nsISupports *
 
 NS_IMETHODIMP nsNntpService::GetNewNews(nsINntpIncomingServer *nntpServer, const char *uri, PRBool aGetOld, nsIUrlListener * aUrlListener, nsIMsgWindow *aMsgWindow, nsIURI **_retval)
 {
-  if (!uri) return NS_ERROR_NULL_POINTER;
+  NS_ENSURE_ARG_POINTER(uri);
 
   NS_LOCK_INSTANCE();
   nsresult rv = NS_OK;
@@ -1224,8 +1223,7 @@ NS_IMETHODIMP nsNntpService::GetNewNews(nsINntpIncomingServer *nntpServer, const
     rv = RunNewsUrl(aUrl, aMsgWindow, nsnull);  
 	
     if (_retval) {
-      *_retval = aUrl;
-      NS_IF_ADDREF(*_retval);
+      NS_IF_ADDREF(*_retval = aUrl);
     }
   }
   else {

mailnews/news/src/nsNntpService.h

@@ -91,7 +91,7 @@ protected:
   nsresult CreateMessageIDURL(nsIMsgFolder *folder, nsMsgKey key, char **url);
   // a convience routine used to put together news urls
   nsresult ConstructNntpUrl(const char * urlString, nsIUrlListener *aUrlListener,  nsIMsgWindow * aMsgWindow, const char *originalMessageUri, PRInt32 action, nsIURI ** aUrl);
-  nsresult CreateNewsAccount(const char *username, const char *hostname, PRBool isSecure, PRInt32 port, nsIMsgIncomingServer **server);
+  nsresult CreateNewsAccount(const char *aHostname, PRBool aIsSecure, PRInt32 aPort, nsIMsgIncomingServer **aServer);
   nsresult GetProtocolForUri(nsIURI *aUri, nsIMsgWindow *aMsgWindow, nsINNTPProtocol **aProtocol);
   // a convience routine to run news urls
   nsresult RunNewsUrl (nsIURI * aUrl, nsIMsgWindow *aMsgWindow, nsISupports * aConsumer);