41929 - can't have multiple accounts with same pop/imap server and username, even if port is different

Status: NEW

(MailNews Core :: Backend, defect)

Description

[Peter Janes]

BuildID:    2000060720

Attempting to create two IMAP accounts for the same host, but with a different
port, fails silently.

Reproducible: Always

I have an IMAP account (localhost port 143) set up as 'Work'.  I use ssh to
port-forward my home account's port 143 to 1143 on localhost.  If I try to add
another 'Home' IMAP account on localhost, the account is not added (perhaps
because the host and port are identical?).  If I add the account as 'orion',
it's added properly and I can change the port from the advanced options. 
However, access to orion:1143 via ssh forwarding does not work (it requires
'localhost:1143').

Actual Results:  A new entry is placed in prefs.js, but it's not shown in the
sidebar and cannot be edited.

Tried to add 'orion' and then change "mail.server.server4.hostname" in prefs.js
to "localhost"; the 'Home' account then disappeared from the sidebar, although
it was still shown in prefs.js.

Relevant lines from prefs.js:

user_pref("mail.server.server1.directory",
"/home/peterj/.mozilla/mozProfile/ImapMail/server1");
user_pref("mail.server.server1.hostname", "localhost");
user_pref("mail.server.server1.name", "Work");
user_pref("mail.server.server1.type", "imap");
user_pref("mail.server.server1.userName", "peterj");
user_pref("mail.server.server4.directory",
"/home/peterj/.mozilla/mozProfile/ImapMail/orion");
user_pref("mail.server.server4.hostname", "orion");
user_pref("mail.server.server4.max_cached_connections", 5);
user_pref("mail.server.server4.name", "Home");
user_pref("mail.server.server4.port", 1143);
user_pref("mail.server.server4.type", "imap");
user_pref("mail.server.server4.userName", "peterj");

Comment 1

[Alec Flett]

known issue, not sure if we'll ever fix it.

Comment 2

[Alec Flett]

I know this is a hack, but one workaround you can do is refer to localhost by
it's real hostname or something.. i.e. set up the server as localhost and
myhost:1143

Comment 3

[Alec Flett]

oh, and we will be able to fix this if our architecture ever allows us to
eliminate FindServer(), but we're a long way off from that.

Comment 4

[Peter Janes]

Tried that hack, but because I've already got 'Work' set up as localhost, I
can't change it to myhost.  ssh requires localhost:1143, not orion:1143.

Could just re-generate the accounts, I guess.

Comment 5

[Alec Flett]

massive reassign of account manager bugs -> sspitzer
please feel free to put me back on the CC if you have any questions/comments

Comment 6

[(not reading, please use [e-mail] instead)]

passing the buck to racham.

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

This is a big deal for people who get multiple email accounts forwarded to the
same server.  Nominating for mozilla 0.9.

Comment 8

[Joerg Sievers]

Also for NEWS it is very useful to have two different accounts on the same
server because in selecting newsgroups in different categories [e.g. like
'privat' and 'business'].

Comment 9

[Carson Gaspar]

This also fails for multiple accounts on the same server on different ports.
Services are provided on a host:port tuple, not a host. Any architecture that
breaks this is fundamentally broken. This was supposedly fixed as bug 13833 many
moons ago - why is it broken _again_?

Comment 10

[Gervase Markham [:gerv]]

I'm encountering problems with multiple accounts on the same server with
different logins. I have ten IMAP mailboxes; one is home stuff, one is Mozilla
stuff. We really should be able to support that - it's not an uncommon
configuration.

Gerv

Comment 11

[Gary Coady]

*** Bug 106916 has been marked as a duplicate of this bug. ***

Comment 12

[wacheo]

*** Bug 122808 has been marked as a duplicate of this bug. ***

Comment 13

[wacheo]

*** Bug 116949 has been marked as a duplicate of this bug. ***

Comment 14

[wacheo]

*** Bug 159404 has been marked as a duplicate of this bug. ***

Comment 15

[wacheo]

*** Bug 133228 has been marked as a duplicate of this bug. ***

Comment 16

[wacheo]

*** Bug 136933 has been marked as a duplicate of this bug. ***

Comment 17

[Craig Ringer]

A tip for Peter Janes and anybody else having this problem: you can set up your
ssh port forwarding using the name "localhost" or 127.0.0.1 or whatever you
want. The only thing that matters in this setup is the hostnames given to
mozilla. I have just added aliaes (brokenimap1 brokenimap2 etc) to my /etc/hosts
line for "127.0.0.1 localhost". Having done that, I can assign multiple accounts
easily.

Moz really needs to match account conflicts on a "proto://user@host:port" not
just "user@host" or "proto://user@host" basis. The current behavior is badly broken.

Perhaps a relnote mentioning the problem and workaround would be appropriate

Comment 18

[Craig Ringer]

A tip for Peter Janes and anybody else having this problem: you can set up your
ssh port forwarding using the name "localhost" or 127.0.0.1 or whatever you
want. The only thing that matters in this setup is the hostnames given to
mozilla. I have just added aliaes (brokenimap1 brokenimap2 etc) to my /etc/hosts
line for "127.0.0.1 localhost". Having done that, I can assign multiple accounts
easily.

Moz really needs to match account conflicts on a "proto://user@host:port" not
just "user@host" or "proto://user@host" basis. The current behavior is badly broken.

Perhaps a relnote mentioning the problem and workaround would be appropriate

Comment 19

[Boris 'pi' Piwinger]

*** Bug 119900 has been marked as a duplicate of this bug. ***

Comment 20

[Jesse Ruderman]

A related problem was fixed in bug 26768, "Two POP accounts with same server
shouldn't use same password".

See also bug 17289, "[4.x MIGRATION] collision if the user has two imap accounts
on the same server, but different ports".  I think that makes this bug 4xp.

Comment 21

[Matthias Versen [:Matti]]

*** Bug 173268 has been marked as a duplicate of this bug. ***

Comment 22

[R.K.Aa.]

*** Bug 184569 has been marked as a duplicate of this bug. ***

Comment 23

[Matthias Versen [:Matti]]

*** Bug 188406 has been marked as a duplicate of this bug. ***

Comment 24

[Lars O. Grobe]

Hi! I know this is a known problem. But why can I have multiple Accounts on the
same imap server with Mozilla 1.21 on Windows, while on Linux, they are shown as
blank envelopes without the name attached to them? In fact, the account name is
still in the settings, but the mail client becomes unuseable without the account
names if you have more than one account to manage (e.g. we have accounts for
different teams on the server, but all use the same mail client on the same
machine). Thank You, CU, Lars.

Comment 25

[Lars O. Grobe]

(My comment #24) Sorry, it's not the Linux client, but the Mac OS 8/9 version of
Mozilla 1.21 that has problems showing account names.

Comment 26

[Derek Ross]

Attachment: A possible solution: modifiable "From" field in compose.

My two cents, as I also have a need for a fix to this bug (I have a regular
email address, and I also have emails forwarded from one of my domains).  

It would make many people happy if it were easy to alter the "From" field while
sending an email.  I know that it's possible to alter the "Reply-to" field, but
I've heard reports that some email clients ignore the "reply-to" field.

Also, there are situations where it's necessary to give the impression that an
email message is coming "From" a certain domain.

Possibly another option could be made available in the "To/Cc/Bcc/etc" dropdown
while composing an email.  That would be a "From" field.  Possibly the list of
possible addresses accessible to the "From" field could be specified in the
"Account Settings" panel.  That way it makes it more difficult to accidentally
set the "From" field to an unexpected address.

If I could easily set the "From" field, my problem would be solved.  No need to
modify "/etc/hosts", or eliminate "FindServer()", or to provide multiple
identical accounts.  I think that many people would be happy with such a
solution.

Thanks, Derek Ross.

Comment 27

[Craig Ringer]

I'm not sure that really solves the problem.

Unfortunately, I and I expect a number 
of others need to access multiple IMAP accounts on the same server with 
the same username, but on different ports. For example, I have an SSH 
tunnel that forwards requests for a port on my firewall to the IMAP 
server at work behind their firewall. I also have a local IMAP server on 
my firewall that is used to collect mail for several POP3 accounts using 
fetchmail. As you can see, your proposed solution would not fix my 
problem, nor to my mind the central problem behind this bug: you still 
couldn't have multiple accounts on the same host+username but on 
different ports.

Craig Ringer

Comment 28

[Gervase Markham [:gerv]]

I'm sure there is another bug on making the From: field editable.

Gerv

Comment 29

[Gervase Markham [:gerv]]

I am currently suffering from this bug, and using the "multiple aliases"
workaround, so one of my accounts is on "mail.foo.com", another on "web.foo.com"
(which happens to be the same machine) and so on. However, because I use IMAP
over SSL, this has the irritating effect that I have to dismiss a "certificate
mismatch" dialog every time I start mail, because the mail cert is for
mail.foo.com, not web.foo.com. 

This also means I don't have the protection that SSL is supposed to give me,
because I get the aforementioned "someone might be doing a man-in-the-middle
attack" dialog every time, and so ignore it.

I've just tried creating a new profile with two accounts on the same server and,
sure enough, it still doesn't work. Both accounts fail to collect messages.

sspitzer, or anyone: is this bug a deep architectural problem in mail, or quite
easy to fix?

Gerv

Comment 30

[Gervase Markham [:gerv]]

To continue: a bit of searching shows about six other bugs filed, all of which
probably have the same root cause - Mozilla does not distinguish carefully
enough between services which have some configuration data in common, be it
several news servers on the same machine, or two SSL-enabled services (SMTP and
IMAP) on the same machine, or whatever.

Gerv

Comment 31

[Philip White]

*** Bug 131764 has been marked as a duplicate of this bug. ***

Comment 32

[Alfonso Martinez]

*** Bug 201016 has been marked as a duplicate of this bug. ***

Comment 33

[Hugo]

I have read somewhere that there is a workaround for this by manually editing 
the prefs.js file to create more "from" addresses. However, I cannot find this 
anymore. Can anybody help?

Comment 34

[Trevor Nuridin]

My situation is similar and slightly different.
I need to be able to access multiple IMAP accounts on the same server, same
port, same username, same passwords, different email addresses. Why ? I am
accessing a MS exchange server and I am controlling multiple accounts on it, and
some accounts don't have their own 'NT user' attached to them - I am using a
single 'NT user' with several email accounts. These email accounts are NOT
aliases. Alias email works OK, since thats a server side function, but these
other accounts I would prefer not to be aliased. Outlook works fine with this setup.

Comment 35

[(not reading, please use [e-mail] instead)]

mass re-assign.

Comment 36

[Galik]

I fell foul of this bug too. My needs are that I have a single catch-all pop
account that I want to service several family members. In order to get different
email addresses in the From: field I need to set up multiple accounts to the
same pop server. Mozilla flatly refuses to do this :( One solution I thought
about was to have the option to creat a new mail account and set the outgoing
server to *none*. This means Mozilla will never try to collect mail from this
account but does allow sending mail from this account. Another solution would be
to simply allow Mozilla to have several accounts to the same pop server.
Obviously you would need to prevent mozilla from collecting mail redundently.

Comment 37

[timeless]

Comment on attachment 113554 [details]
A possible solution: modifiable "From" field in compose.

i'll minus this. it's confusing since it conflicts with the other from field
also visible in the picture.

as to the last comment if you only need return addresses and not inboxes,
create news server accounts for each name.

Comment 38

[Ross Kendall]

I put a vote on this bug, but what I really think would keep users happy is if
they could manage email aliases for an account.  
(i.e. use different names/email-addresses for an account).

Comment 39

[Michael Richards]

Nobody denies that aliases would be great, and it has been mentioned many times.
But aliases should be viewed as a distinctly different issue then this. I
personally don't care about aliases, but this bug causes me all sort of grief. A
lot of people have the need to access multiple, *different*, accounts in this
manner.

Comment 40

[Bruce Davidson]

*** Bug 217994 has been marked as a duplicate of this bug. ***

Comment 41

[zhuyunchuan]

I think it's useless to check the account, perhaps it's wrong to set the same
account and server. Tools can hint him that he is 'probably' wrong but shouldn't
deny this setting, at least it should take a way to set it.

Comment 42

[Christian Eyrich]

*** Bug 223774 has been marked as a duplicate of this bug. ***

Comment 43

[Carl Seghers]

The solution I propose here is not for the bug as described here, but for bug
122808 that is designated as duplicate of this one.

I have a situation where I have set up an account for each of my e-mail aliases
(@seghers.net) under which I post.  Only one of the accounts actually fetches
the mails and distributes them to the inboxes of the other accounts. The others
have an invalid user specified, so there is no conflict from TB's point of view,
and they also are set up to never request mail themselves.  The From e-mail
address for each account is different, so when I reply from that account's
inbox, my From field is the same as to which the original mail was addressed. 

The only glitch is that when I want to get new mails, I have to first click the
receiving account and then ask for new mail.  If I stay inside the other's
account inbox (or other folder) I am asked the password for the invalid user,
which is not what I want ofcourse.

So, to make everything work seamlessly, I think it should be possible to specify
 the same server, user and port for different accounts, but be given the option
to designate one account as the master, and the others as slaves.  Requesting
mail for the slave accounts would simply pass the request to the master account,
which distributes the incoming mail over the slave accounts.

Should be a relatively easy thing to do, no?

Comment 44

[timeless]

*** Bug 234749 has been marked as a duplicate of this bug. ***

Comment 45

[Kevin Teuscher]

Attachment: Add capability to find a server by URL

Here is the first part of a possible solution.
This patch adds the possiblity of using a given url to find the server. 
Included is the capability to match on the port number also.  A value of "0" is
used for the port if any port is acceptable.

I have discussed this with bienvenu briefly.

The second half of this patch would be to convert the users of FindServer over
to using FindServerByUrl

Please comment.

Kevin

Comment 46

[David :Bienvenu]

Kevin, that looks reasonable so far...I'll review the whole patch when it's done.

Comment 47

[Stephan Petrowitsch]

After a complete new-installation and -configuration it works now well. So I
can´t say what the mistake was, because the configuration before was exactly the
same. 

Comment 48

[Kevin Teuscher]

Attachment: Full patch for backend changes

Full patch

Comment 49

[Kevin Teuscher]

Attachment: same patch using diff -w

same patch without whitespace changes (there were lots of tabs in
nsLocalUtils.cpp and nsLocalMailFolder.cpp)

Comment 50

[David :Bienvenu]

thx, Kevin, looks good in general.

Can you remove the commented out code?

+/*
     // extract the user name and host name information...
     nsresult rv = mailnewsUrl->GetAsciiHost(hostName);
     if (NS_FAILED(rv)) return rv;
@@ -2642,6 +2645,7 @@
 
     if (!userPass.IsEmpty())
       NS_UnescapeURL(userPass); // hopefully we're not unescaping ':' or nasty
control chars
+*/


Also, why do you call SetScheme on the url's? I would have thought the scheme
would get parsed out by the uri parser...

Comment 51

[Kevin Teuscher]

(In reply to comment #50)
> thx, Kevin, looks good in general.
> 
> Can you remove the commented out code?
> 
> +/*
>      // extract the user name and host name information...
>      nsresult rv = mailnewsUrl->GetAsciiHost(hostName);
>      if (NS_FAILED(rv)) return rv;
> @@ -2642,6 +2645,7 @@
>  
>      if (!userPass.IsEmpty())
>        NS_UnescapeURL(userPass); // hopefully we're not unescaping ':' or nasty
> control chars
> +*/

No problem; missed one when I was cleaning up after testing.  I will generate a
patch for that this weekend.  I'm away from my tree 'til then.
> 
> 
> Also, why do you call SetScheme on the url's? I would have thought the scheme
> would get parsed out by the uri parser...

The uri comes in with the scheme set to 'mailbox'; since that could be either a
set of local folders or a pop account (or a movemail account on unix) you have
to replace try each scheme.  It seemed easiest to just use SetScheme to switch
this instead of doing string wrangling.  Since the url I'm working with is built
from the input string, I'm not changing the original uri.  I guess, that I could
move this check into "InternalFindServerByUrl" and do the testing there, but it
might be a little messy (i.e. I need to think about it and see if it makes sense)

Kevin

Kevin

Comment 52

[Kevin Teuscher]

David:  after this goes in someone will only be able to take advantage if they
know how to manage the prefs.js or are willing to create an account with a bogus
host and then edit it to the proper one in prefs.js.  Do we want to add UI for
this capability, or is this to be left as an advanced feature only?

Another thought:  Should we rework FindServer to create a url from the input and
then pass it to FindServerByURL to eliminate code duplication?

Kevin

Comment 53

[David :Bienvenu]

Kevin, re the first question, is this because the new account code prevents you
from creating an account with the same user name and host name as another
account? If so, I think that code should be changed to prevent you from creating
an account with the same user name, host name, and port - which would mean also
prompting for the port in the new account wizard...hmm. But, can't the user go
in and edit the server name through the account settings UI, by using a bogus
server name first, w/o going into prefs.js?

Re the second question, if it's just as reliable, and less code, it's worth a
try. But I'm confused - doesn't FindServer just call InternalFindServer? It's
hard to tell from the diffs.

Comment 54

[Kevin Teuscher]

(In reply to comment #53)
> Kevin, re the first question, is this because the new account code prevents you
> from creating an account with the same user name and host name as another
> account? 

Yes, the FE UI calls FindServer which only allows input of username,host,proto

>If so, I think that code should be changed to prevent you from creating
> an account with the same user name, host name, and port - which would mean also
> prompting for the port in the new account wizard...hmm. But, can't the user go
> in and edit the server name through the account settings UI, by using a bogus
> server name first, w/o going into prefs.js?

I'll have to take another look, but I think if you change the hostname in the
account settings, it might check for a duplicate based only on
(username,host,proto).  That is where you would get an error.  I need to verify
that behavior
> 
> Re the second question, if it's just as reliable, and less code, it's worth a
> try. But I'm confused - doesn't FindServer just call InternalFindServer? It's
> hard to tell from the diffs.

I was thinking of having FindServer make a url out of its input and send that to
InternalFindServerByUrl and then eliminating InternalFindServer and findServer
(these are what I copied basically to make InternalFindServerByUrl and
findServerUrl).  Right now with the submitted patch, we basically have
duplication of tasks for InternalFindServer/InternalFindServerByUrl and
findServer/findServerUrl.

Kevin

Comment 55

[Kevin Teuscher]

Attachment: Cleaned up version

Removed the commented out code per David's comment.  I am not obsoleting the
diff -w version, but it has not been updated to show removal of what I had
commented out.

David,
I rethought about converting FindServer over to calling InternalFindServerUrl. 
There are some cases (blank userpass, blank host) that are used that I need to
investigate prior to converting (Have to see how the URL code responds to those
inputs) and since beta is close, thought it would be better to get this in
rather than push.

If we get this in for beta, we can at least allow advanced users to use this
functionality in 1.7 and TB 0.6

Requesting r and sr.  Do I have time for appr1.7b?

Kevin

Comment 56

[Kevin Teuscher]

Comment on attachment 143649 [details] [diff] [review]
Full patch for backend changes

Removing request on obsolete patch

Comment 57

[Kevin Teuscher]

Comment on attachment 144039 [details] [diff] [review]
Cleaned up version

Removing review requests since I have more to do.

bienvenu:  In working on bug 238583 (the UI stuff) I am finding that we don't
import port info from other programs.  If I modify the CreateIncomingServer
function to add the port number, I need to do something with the import code.
What is your opinion, fix the import code at the same time or rename the
current "CreateIncomingServer" to CreateIncomingServerLegacy" and change the
import code to point at it and then do a new bug to update the import code?

I can do it all here, but it is going to turn into a big bug fix with lots of
changes to review.

Kevin

Comment 58

[Stefan Borggraefe]

*** Bug 249398 has been marked as a duplicate of this bug. ***

Comment 59

[Kevin Teuscher]

Attachment: Review version

Review patch for adding FindServerByURL

patch for checkin has some tabs and whitespace fixes.  I'll attach later. 
Should be good for 1.8a 

I have put on the back burner the problem of importing ports from other
programs.  I got around the problem in the JS by setting the port and secure
properties on the incomingServer object that is exposed to JS.

Have at for review.
Kevin

Comment 60

[Kevin Teuscher]

Comment on attachment 155024 [details] [diff] [review]
Review version

I think this is ready to review

Comment 61

[David :Bienvenu]

Comment on attachment 155024 [details] [diff] [review]
Review version

thx for doing this!

+     
(!nsCRT::strcmp((hostname.get()?hostname.get():""),m_lastFindServerHostName.get
())) &&
+     
(!nsCRT::strcmp((username.get()?username.get():""),m_lastFindServerUserName.get
())) &&

You should be able to just use m_lastFindServerHostName.Equals(hostName), i.e.,
use .Equals for all these.

+  *aResult = serverInfo.server;
+  NS_ADDREF(*aResult);

this can just be NS_ADDREF(*aResult = serverInfo.server); which generates less
code.

+  // treat "" as a wild card, so if the caller passed in "" for the desired
attribute
+  // treat it as a match
+  PRBool checkType = PL_strcmp(entry->type, "");
+  PRBool checkHostname = PL_strcmp(entry->hostname,"");
+  PRBool checkUsername = PL_strcmp(entry->username,"");
+  PRBool checkPort = (entry->port != 0);
+  if ((!checkType || (PL_strcmp(entry->type, thisType)==0)) && 
+      (!checkHostname || (PL_strcasecmp(entry->hostname, thisHostname)==0)) && 
+      (!checkPort || (entry->port == thisPort)) && 
+      (!checkUsername || (PL_strcmp(entry->username, thisUsername)==0))) 
+  {
+    entry->server = server;
+    return PR_FALSE;		 // stop on first find 
+  }

don't need temp vars, or PL_strcmp so this code can just be:

if (!*entry->type || strcmp(entry->type, thisType) etc.

could you just remove this commented out code?

+//  nsUnescape(username);
+//  nsUnescape(hostname);

maybe add a comment that 
InternalFindServerByUrl will do the unescaping?

Comment 62

[Kevin Teuscher]

Attachment: Updated per comments

(In reply to comment #61)
> thx for doing this!
You're welcome! :)

>+     
>(!nsCRT::strcmp((hostname.get()?hostname.get():""),m_lastFindServerHostName.get

>())) &&
>+     
>(!nsCRT::strcmp((username.get()?username.get():""),m_lastFindServerUserName.get

>())) &&
>
>You should be able to just use m_lastFindServerHostName.Equals(hostName),
i.e.,
>use .Equals for all these.

Changed to use .Equals

>+  *aResult = serverInfo.server;
>+  NS_ADDREF(*aResult);
>
>this can just be NS_ADDREF(*aResult = serverInfo.server); which generates less

>code.

These instances, plus some others in the same file changed to single line form.


>+  // treat "" as a wild card, so if the caller passed in "" for the desired
>attribute
>+  // treat it as a match
>+  PRBool checkType = PL_strcmp(entry->type, "");
>+  PRBool checkHostname = PL_strcmp(entry->hostname,"");
>+  PRBool checkUsername = PL_strcmp(entry->username,"");
>+  PRBool checkPort = (entry->port != 0);
>+  if ((!checkType || (PL_strcmp(entry->type, thisType)==0)) && 
>+	(!checkHostname || (PL_strcasecmp(entry->hostname, thisHostname)==0))
&& 
>+	(!checkPort || (entry->port == thisPort)) && 
>+	(!checkUsername || (PL_strcmp(entry->username, thisUsername)==0))) 
>+  {
>+    entry->server = server;
>+    return PR_FALSE;		 // stop on first find 
>+  }
>
>don't need temp vars, or PL_strcmp so this code can just be:
>
>if (!*entry->type || strcmp(entry->type, thisType) etc.

Changed per your suggestion

>could you just remove this commented out code?
>
>+//  nsUnescape(username);
>+//  nsUnescape(hostname);

Code removed

>maybe add a comment that 
>InternalFindServerByUrl will do the unescaping?

Comment added just above call to FindServerByUrl() in that procedure

patch is a diff -w -u8.  There are a bunch of tabs in some of these files.
Do you want a patch with those fixed to check-in?

P.S. I don't have cvs access so someone will have to do the honors after
reviews are done.

Comment 63

[Kevin Teuscher]

Comment on attachment 155024 [details] [diff] [review]
Review version

remove requests on old patch

Comment 64

[David :Bienvenu]

Comment on attachment 155084 [details] [diff] [review]
Updated per comments

other than that, looks fine. When we're finished with the reviews, I'll checkin
the patch that has the whitespace changes.

+    if (type.Equals("pop"))
+      type.Assign("pop3");
+    // we use "nntp" in the server list so translate it here.
+    if (type.Equals("news"))
+      type.Assign("nntp");

should be else if (type.Equals...)

Comment 65

[Kevin Teuscher]

Attachment: patch for checkin

patch with whitespace fixes and else if change from last comment

Comment 66

[David :Bienvenu]

Comment on attachment 155084 [details] [diff] [review]
Updated per comments

switching sr request to Neil...

Comment 67

[[e-mail]]

Comment on attachment 155084 [details] [diff] [review]
Updated per comments

>+   * search for the server with the given url
>+   * an analog to FindServer()
>+   */
>+  nsIMsgIncomingServer
>+      FindServerByUrl(in string url);
>+
>+  /*
>+   * Same as FindServerByURL() except it compares the input values against
>+   * 'realhostname' and 'realuserName' pref settings.
>+   */
>+  nsIMsgIncomingServer
>+      FindRealServerByUrl(in string url);
Do these need to be accessible from JS? a) the "string" type doesn't completely
express a URL, e.g. a username of François or a hostname of öko.de b) all the
callers already have a URL, and currently have to GetSpec() and get() a spec to
pass to these functions. I think you would be better of with something along
the lines of nsIMsgIncomingServer FindServerByUri(in nsIURI aURI, in boolean
aRealFlag);

>+  // Get username and hostname and port so we can get the server
>+  nsCAutoString username;
>+  rv = aUrl->GetUserPass(username);
Shouldn't you be using the Username functions rather than the UserPass ones?

>+  if (NS_SUCCEEDED(rv) && !username.IsEmpty())
>+    nsUnescape(NS_CONST_CAST(char*,username.get()));
I think NS_UnescapeURL will suffice. Please don't abuse the string API with
const casts.

>+    if (type.Equals("pop"))
EqualsLiteral, please.

>+      type.Assign("pop3");
AssignLiteral, please.

>+  // hostname might be blank, pass "" instead
>+  serverInfo.hostname = hostname.get() ? hostname.get() : "";
Actually only xpidl strings may return null from get().

>+  // Dummy string to initialize the URL
>+  nsCAutoString spec("http://user@hostname:1111");
Intriguing... does setting the scheme, host, user and port not work without
this?

>+  nsXPIDLCString thisHostname;
>+  if (entry->useRealSetting)
>+    rv = server->GetRealHostName(getter_Copies(thisHostname));
>+  else
>+  rv = server->GetHostName(getter_Copies(thisHostname));
You missed some indent here, I think.

>+      nsXPIDLCString turl;
Why not an nsCAutoString? (Although given my comments above this should get
removed entirely.)

>+      tschm.Assign(GetIncomingServerType());
>+      url->SetScheme(tschm);
I think you can use nsDependentCString(GetIncomingServerType()) here.

Comment 68

[Kevin Teuscher]

Attachment: v2

Neil,
Like this?

Comment 69

[Kevin Teuscher]

Neil,
Replies to your review.


(In reply to comment #67)
> (From update of attachment 155084 [details] [diff] [review])
> >+   * search for the server with the given url
> >+   * an analog to FindServer()
> >+   */
> >+  nsIMsgIncomingServer
> >+      FindServerByUrl(in string url);
> >+
> >+  /*
> >+   * Same as FindServerByURL() except it compares the input values against
> >+   * 'realhostname' and 'realuserName' pref settings.
> >+   */
> >+  nsIMsgIncomingServer
> >+      FindRealServerByUrl(in string url);
> Do these need to be accessible from JS? a) the "string" type doesn't completely
> express a URL, e.g. a username of François or a hostname of öko.de b) all the
> callers already have a URL, and currently have to GetSpec() and get() a spec to
> pass to these functions. I think you would be better of with something along
> the lines of nsIMsgIncomingServer FindServerByUri(in nsIURI aURI, in boolean
> aRealFlag);

Only findRealServer is used by JS, so I took your idea and went with it.

> 
> >+  // Get username and hostname and port so we can get the server
> >+  nsCAutoString username;
> >+  rv = aUrl->GetUserPass(username);
> Shouldn't you be using the Username functions rather than the UserPass ones?

Using Userpass functions because that is what is used throughout mailnews. 
bienvenu might have the background. 

As I type, I just thought of a reason why we use UserPass:
Some ISPs have the username as [e-mail].  If we use just the username
functions, then we would not get the complete username from the URL because it
would be parsed at the @
> 
> >+  if (NS_SUCCEEDED(rv) && !username.IsEmpty())
> >+    nsUnescape(NS_CONST_CAST(char*,username.get()));
> I think NS_UnescapeURL will suffice. Please don't abuse the string API with
> const casts.
> 
Done.

> >+    if (type.Equals("pop"))
> EqualsLiteral, please.
> 
Done.

> >+      type.Assign("pop3");
> AssignLiteral, please.
> 
Done.

> >+  // hostname might be blank, pass "" instead
> >+  serverInfo.hostname = hostname.get() ? hostname.get() : "";
> Actually only xpidl strings may return null from get().
> 
> >+  // Dummy string to initialize the URL
> >+  nsCAutoString spec("http://user@hostname:1111");
> Intriguing... does setting the scheme, host, user and port not work without
> this?
That is correct.  I ran into the problem while debugging.  If the userpass,
scheme or port aren't set at the beginning, the set functions won't allow them
to be changed.  In other words, you can't build a URL from scratch.  You have to
put a dummy value like this in and then change the parts you need to change. 
I'm not sure if this is a bug or if there is a purpose behind the behavior. 
Darin would know, I suppose.
> 
> >+  nsXPIDLCString thisHostname;
> >+  if (entry->useRealSetting)
> >+    rv = server->GetRealHostName(getter_Copies(thisHostname));
> >+  else
> >+  rv = server->GetHostName(getter_Copies(thisHostname));
> You missed some indent here, I think.
Got it.

> 
> >+      nsXPIDLCString turl;
> Why not an nsCAutoString? (Although given my comments above this should get
> removed entirely.)
Removed as you mentioned.
> 
> >+      tschm.Assign(GetIncomingServerType());
> >+      url->SetScheme(tschm);
> I think you can use nsDependentCString(GetIncomingServerType()) here.
Done.

> 

I'm going to ask bienvenu to review again, since I did modify this quite a bit.

Could you look at the beginning of a patch in 238583 for the UI portion and give
me some feedback?

Thanks,
Kevin

Comment 70

[[e-mail]]

Comment on attachment 155979 [details] [diff] [review]
v2

OK, so if bienvenu's happy with GetUserPass then so be it, it just looks odd to
me.

You only appear to have two callers of InternalFindServerByUri, would it be too
much to ask to move the code into FindServerByUri and call that from
FindRealServer?

Comment 71

[David :Bienvenu]

Kevin is right; mailnews has always used those odd necko GetUserPass methods,
though I think we could use GetUsername instead - probably better to deal with
that after this patch lands.

I'll wait for Kevin to respond to Neil's last comment about moving
InternalFindServerByUri into FindServerByUri...I'm all for getting rid of an
extra method if it doesn't result in more than negligible code duplication.

Comment 72

[Kevin Teuscher]

(In reply to comment #71)
> Kevin is right; mailnews has always used those odd necko GetUserPass methods,
> though I think we could use GetUsername instead - probably better to deal with
> that after this patch lands.
Because of my example ([e-mail] as the mail username), I don't think
you'll be able to get away from UserPass (unless you do some magic with
escaping).  At first glance, doesn't seem worth the effort, but I'm a rookie, so
I could very easily be wrong.
> 
> I'll wait for Kevin to respond to Neil's last comment about moving
> InternalFindServerByUri into FindServerByUri...I'm all for getting rid of an
> extra method if it doesn't result in more than negligible code duplication.
Makes sense.  I'll spin a new patch (including whitespace changes).
As a followup bug after this is all in, we could move FindServer() to use
FindServerByUri() and get rid of the support functions as well.  I didn't want
to do that in this bug because of the use of FindServer() in the import code.

Kevin

Comment 73

[timeless]

Comment on attachment 155979 [details] [diff] [review]
v2

>Index: mozilla/mailnews/base/public/nsIMsgAccountManager.idl
>+  nsIMsgIncomingServer
>+      FindServerByUri(in nsIURI aURI, in boolean aRealFlag);

please make the method lowercase. we can fix the other methods now or later.

>   nsIMsgIncomingServer
>-      findRealServer(in string userName, in string hostname, in string type);
>+      findRealServer(in string userName, in string hostname, in string type, in long port );

>Index: mozilla/mailnews/base/src/nsMsgAccountManager.cpp

>+nsMsgAccountManager::InternalFindServerByUri(nsIURI *aUri,
>+                                PRBool useRealSetting,
>+                                nsIMsgIncomingServer** aResult)

Does this cover the netscape mock protocol, or scale to cover other internal
protocols that people may add?

>+  nsCAutoString hostname;

this pattern bothers me:
>+  serverInfo.hostname = hostname.get() ? hostname.get() : "";
>+  serverInfo.username = username.get() ? username.get() : "";
>+  serverInfo.type = type.get() ? type.get() : "";


>+  if (!serverInfo.server) return NS_ERROR_UNEXPECTED;

return on same line makes breakpointing harder.

> nsMsgAccountManager::FindRealServer(const char* username,
>                                 const char* hostname,
>                                 const char* type,
>+                                PRInt32 port,
>                                 nsIMsgIncomingServer** aResult)
> {
>-  InternalFindServer(username, hostname, type, PR_TRUE, aResult);
>+  // Dummy string to initialize the URL
>+  nsCAutoString spec("http://user@hostname:1111");
>+  nsresult rv;
>+  nsCOMPtr<nsIURL> aUrl = do_CreateInstance(NS_STANDARDURL_CONTRACTID, &rv);
>+  if (NS_FAILED(rv)) return NS_OK;

you aren't clearing aResult but you're returning NS_OK. this feels really bad

>+#ifdef DEBUG

please change this to DEBUG_<you> or use nspr logging. thanks.

>+  aUrl->GetSpec(spec);
>+  printf("aUrl == %s\n", spec.get());
>+#endif
>+  InternalFindServerByUri(aUrl, PR_TRUE, aResult);
>   return NS_OK;
> }


>+nsMsgAccountManager::findServerUrl(nsISupports *aElement, void *data)
>+{
>+  nsresult rv;
>+  
>+  nsCOMPtr<nsIMsgIncomingServer> server = do_QueryInterface(aElement, &rv);

since you aren't really using rv here, i'd suggest you save do_QI from setting
it. just nullcheck server.

>+  if (NS_FAILED(rv)) return PR_TRUE;

>+  // Don't try and get a port for the 'none' service

service or protocol?

>Index: mozilla/mailnews/base/util/nsMsgMailNewsUrl.cpp
>@@ -167,53 +167,50 @@
> NS_IMETHODIMP nsMsgMailNewsUrl::GetServer(nsIMsgIncomingServer ** aIncomingServer)
>+  nsresult rv;
>+  nsCAutoString turl;

turl?

>Index: mozilla/mailnews/local/src/nsLocalUtils.cpp
>+  // No unescaping of username or hostname done here.
>+  // This is done inside of InternalFindServerByUri
This => It (?)

>     // in an imap hiearchy. look for an imap server.
		    ^^^^^^^^ (sp)

> nsLocalURI2Server(const char* uriStr,
>                   nsIMsgIncomingServer ** aResult)

>   *aResult = server;
>   NS_IF_ADDREF(*aResult);

I wonder why you didn't combine those two lines

>Index: mozilla/mailnews/local/src/nsLocalMailFolder.cpp
>@@ -3107,81 +3107,63 @@

that code looks like it's asking for a loop.

Comment 74

[Kevin Teuscher]

(In reply to comment #73)
> (From update of attachment 155979 [details] [diff] [review])
> >Index: mozilla/mailnews/base/public/nsIMsgAccountManager.idl
> >+  nsIMsgIncomingServer
> >+      FindServerByUri(in nsIURI aURI, in boolean aRealFlag);
> 
> please make the method lowercase. we can fix the other methods now or later.
Done
+  nsIMsgIncomingServer
+      findserverbyuri(in nsIURI aURI, in boolean aRealFlag);


> 
> >   nsIMsgIncomingServer
> >-      findRealServer(in string userName, in string hostname, in string type);
> >+      findRealServer(in string userName, in string hostname, in string type,
in long port );
Done also.

+      findrealserver(in string userName, in string hostname, in string type, in
long port );

> 
> >Index: mozilla/mailnews/base/src/nsMsgAccountManager.cpp
> 
> >+nsMsgAccountManager::InternalFindServerByUri(nsIURI *aUri,
> >+                                PRBool useRealSetting,
> >+                                nsIMsgIncomingServer** aResult)
> 
> Does this cover the netscape mock protocol, or scale to cover other internal
> protocols that people may add?
> 
> >+  nsCAutoString hostname;
> 
> this pattern bothers me:
> >+  serverInfo.hostname = hostname.get() ? hostname.get() : "";
> >+  serverInfo.username = username.get() ? username.get() : "";
> >+  serverInfo.type = type.get() ? type.get() : "";
> 
> 
> >+  if (!serverInfo.server) return NS_ERROR_UNEXPECTED;
> 
> return on same line makes breakpointing harder.
> 
> > nsMsgAccountManager::FindRealServer(const char* username,
> >                                 const char* hostname,
> >                                 const char* type,
> >+                                PRInt32 port,
> >                                 nsIMsgIncomingServer** aResult)
> > {
> >-  InternalFindServer(username, hostname, type, PR_TRUE, aResult);
> >+  // Dummy string to initialize the URL
> >+  nsCAutoString spec("http://user@hostname:1111");
> >+  nsresult rv;
> >+  nsCOMPtr<nsIURL> aUrl = do_CreateInstance(NS_STANDARDURL_CONTRACTID, &rv);
> >+  if (NS_FAILED(rv)) return NS_OK;
> 
> you aren't clearing aResult but you're returning NS_OK. this feels really bad
>
Comment above the procedure say we must return NS_OK.  All callers check the
result for validity.  Should I do something more?
 
> >+#ifdef DEBUG
> 
> please change this to DEBUG_<you> or use nspr logging. thanks.
> 
> >+  aUrl->GetSpec(spec);
> >+  printf("aUrl == %s\n", spec.get());
> >+#endif
> >+  InternalFindServerByUri(aUrl, PR_TRUE, aResult);
> >   return NS_OK;
> > }
> 
Done.

> 
> >+nsMsgAccountManager::findServerUrl(nsISupports *aElement, void *data)
> >+{
> >+  nsresult rv;
> >+  
> >+  nsCOMPtr<nsIMsgIncomingServer> server = do_QueryInterface(aElement, &rv);
> 
> since you aren't really using rv here, i'd suggest you save do_QI from setting
> it. just nullcheck server.
> 
> >+  if (NS_FAILED(rv)) return PR_TRUE;
> 
Done.
+  nsCOMPtr<nsIMsgIncomingServer> server = do_QueryInterface(aElement);
+  if (!server) return PR_TRUE;


> >+  // Don't try and get a port for the 'none' service
> 
> service or protocol?
> 
Changed comment to following:
+  // Don't try and get a port for the 'none' scheme 

> >Index: mozilla/mailnews/base/util/nsMsgMailNewsUrl.cpp
> >@@ -167,53 +167,50 @@
> > NS_IMETHODIMP nsMsgMailNewsUrl::GetServer(nsIMsgIncomingServer **
aIncomingServer)
> >+  nsresult rv;
> >+  nsCAutoString turl;
> 
> turl?
My shorthand for "temp url"; changed to urlstr
+  nsCAutoString urlstr;


> 
> >Index: mozilla/mailnews/local/src/nsLocalUtils.cpp
> >+  // No unescaping of username or hostname done here.
> >+  // This is done inside of InternalFindServerByUri
> This => It (?)
> 
Changed second line of comment to be clearer.
+  // The unescaping is done inside of FindServerByUri

> >     // in an imap hiearchy. look for an imap server.
> 		    ^^^^^^^^ (sp)
Fixed.

> 
> > nsLocalURI2Server(const char* uriStr,
> >                   nsIMsgIncomingServer ** aResult)
> 
> >   *aResult = server;
> >   NS_IF_ADDREF(*aResult);
> 
> I wonder why you didn't combine those two lines

I missed one?  Changed now.
> 
> >Index: mozilla/mailnews/local/src/nsLocalMailFolder.cpp
> >@@ -3107,81 +3107,63 @@
> 
> that code looks like it's asking for a loop.
> 
Being a rookie, I didn't have a bright idea of how do do this simply.  I came up
with the following, but I think that I would have problems with setting mType to
the xxxx and then having it disappear off the stack.  I'll take a suggestion,
otherwise I'll leave it as is.

My attempt (extra details cut out)
  int i;
  int maxSchemes;
  nsCAutoString scheme;
#ifdef HAVE_MOVEMAIL
  maxSchemes = 4
#else
  maxSchemes = 3
#endif /* HAVE_MOVEMAIL */

  for(i=1; i <= maxSchemes; i++) {
    switch (i)
    {
      case 1: scheme.Assign("none"); break;
      case 2: scheme.Assign("pop"); break;
      case 3: scheme.Assign("rss"); break;
      case 4: scheme.Assign("movemail"); break;
      default: break;
    }

    url->SetScheme(scheme);
    rv = accountManager->FindServerByUri(url, PR_FALSE,
                                    getter_AddRefs(server));
    if (NS_SUCCEEDED(rv) && server) 
    {
      mType = scheme.get();
      return mType;
    }
  }



Thanks for all of the help from everyone.
Kevin

Comment 75

[Kevin Teuscher]

Attachment: v3

fixes per timeless

Comment 76

[Kevin Teuscher]

Comment on attachment 156069 [details] [diff] [review]
v3

Carry forward sr from neil

Comment 77

[Kevin Teuscher]

Comment on attachment 155979 [details] [diff] [review]
v2

clear request on obsolete patch

Comment 78

[Kevin Teuscher]

To avoid posting the whitespace version multiple times, once I have reviews
finished, I'll post the whitespace version for checkin.

Kevin

Comment 79

[David :Bienvenu]

Comment on attachment 156069 [details] [diff] [review]
v3

sr=bienvenu, with one nit - we need a new uid for nsIMsgAccountManager since we
added methods.

Comment 80

[Kevin Teuscher]

(In reply to comment #79)
> (From update of attachment 156069 [details] [diff] [review])
> sr=bienvenu, with one nit - we need a new uid for nsIMsgAccountManager since we
> added methods.
> 
David,
Can I just use guidgen on my window box to get this guid?  I read that I can get
one on irc.mozilla.org, but I don't have access to irc here at work.  If I can
use guidgen, I'll have the patch shortly.

Kevin

Comment 81

[David :Bienvenu]

yes, I think so - I use uuidgen, but I assume guidgen is equivalent...

Comment 82

[Kevin Teuscher]

Attachment: Final version for checkin

Final version for checkin
David,
I changed the uuid in this patch.  I also attached two small changes in the JS
to call the new version of findrealserver until the front end work in bug
238583 is done.  I just pass 0 so that the port is not checked.  I had
overlooked this change because I am simultaneously working on the front end.

Kevin

Comment 83

[Kevin Teuscher]

*** Bug 55540 has been marked as a duplicate of this bug. ***

Comment 84

[Kevin Teuscher]

Attachment: Real final version

Being a rookie, I messed up on changing the case for findserverbyuri and
findrealserver.  This patch is the same as the previous check-in patch, just
with the case of those two procedures corrected so that it will compile.

I best learn the case mangling that the idl compiler does for next time.

Comment 85

[[e-mail]]

Looks like your real final version wasn't :-(

Comment 86

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

timeless checked in a bustage fix to nsLocalMailServer.cpp (missed movemail
changes) and I checked on in to nsMsgAccountManager.h (duplicate const)

Comment 87

[David :Bienvenu]

Attachment: fix thread safety problems in imap introduced by this patch

the main patch introduced some imap problems. The first is a ton of
thread-safety assertions. This patch fixes the thread-safety assertions by
caching the server key in the url, so we don't have to get the server object.
There are still problems - all imap headers are downloaded every time I startup
and we don't ever seem to see the .msf files for the imap folders...I'll look
into that next.

Comment 88

[David :Bienvenu]

Attachment: fix imap folder path handling

this fixes imap folder path handling so we won't think we have to hash every
folder name, and throw away .msf files.

Comment 89

[Scott MacGregor]

Comment on attachment 156474 [details] [diff] [review]
fix thread safety problems in imap introduced by this patch

thanks for cleaning this stuff up david.

Comment 90

[timeless]

mozilla/mailnews/base/prefs/resources/content/AccountWizard.js 
	1.125
mozilla/mailnews/compose/src/nsSmtpServer.cpp 	1.47
mozilla/mailnews/base/util/nsMsgDBFolder.cpp 	1.227
mozilla/mailnews/base/util/nsMsgMailNewsUrl.cpp 	1.101
mozilla/mailnews/base/src/nsMsgAccountManager.cpp 	
	1.286
mozilla/mailnews/base/src/nsMsgAccountManager.h 	
	1.46
mozilla/mailnews/local/src/nsLocalMailFolder.cpp 	1.473
mozilla/mailnews/local/src/nsLocalUtils.cpp  	1.53
mozilla/mailnews/base/public/nsIMsgAccountManager.idl 
	1.62
mozilla/mailnews/news/src/nsNntpService.cpp 	1.251
mozilla/mailnews/imap/src/nsImapService.cpp 	1.302
mozilla/mailnews/imap/src/nsImapUtils.cpp 	1.87
mozilla/mailnews/base/prefs/resources/content/AccountManager.js 
	1.109
mozilla/layout/xul/base/src/nsMenuBarListener.h 	1.20
mozilla/mailnews/base/src/nsMsgAccountManager.h 	
	1.47

Comment 91

[David :Bienvenu]

Attachment: fix assertion with rss servers

if you have an rss account, you get a lot of assertions - this fixes those
assertions by returning the same default port as the no incoming server server.

Comment 92

[(not reading, please use [e-mail] instead)]

Comment on attachment 156513 [details] [diff] [review]
fix assertion with rss servers

sr=sspitzer

Comment 93

[David :Bienvenu]

Attachment: proposed fix for regression introduced by attachment 156748

my fix to fix the thread-safety assertions broke a special case of imap online
server directory handling - basically, we weren't adding the hierarchy
delimiter because we couldn't find the namespace. This was trunk only

Comment 94

[Kevin Teuscher]

*** Bug 101911 has been marked as a duplicate of this bug. ***

Comment 95

[Christian Eyrich]

*** Bug 254194 has been marked as a duplicate of this bug. ***

Comment 96

[Doug Wright]

*** Bug 274995 has been marked as a duplicate of this bug. ***

Comment 97

[Marcel van Beurden]

Any news on this bug? I just ran into it with Thunderbird 1.5.

Thunderbird complains that I'm trying to add another account with the same server/username, which is not true. But it seems my prefs.js contains much more accounts than are shown in Thunderbird. Like the original reporter I'm using SSH with port forwarding to access mutiple servers through localhost.

Any work-around?

Comment 98

[timeless]

Comment on attachment 156474 [details] [diff] [review]
fix thread safety problems in imap introduced by this patch

bienvenu%nventure.com	2004-09-07 17:25
mozilla/mailnews/imap/src/nsImapUrl.cpp	 1.170

Comment 99

[timeless]

Comment on attachment 156478 [details] [diff] [review]
fix imap folder path handling

neil%parkwaycc.co.uk	2004-08-18 16:11
mozilla/mailnews/imap/src/nsImapUtils.cpp	 1.87

Comment 100

[timeless]

Comment on attachment 156513 [details] [diff] [review]
fix assertion with rss servers

bienvenu%nventure.com	2004-08-19 08:21
mozilla/mailnews/local/src/nsRssService.cpp	 1.2

Comment 101

[timeless]

Comment on attachment 158144 [details] [diff] [review]
proposed fix for regression introduced by attachment 156748 [details]

bienvenu%nventure.com	2004-09-07 17:25
mozilla/mailnews/imap/src/nsImapUrl.cpp	 1.170

Comment 102

[timeless]

Comment on attachment 156474 [details] [diff] [review]
fix thread safety problems in imap introduced by this patch

oops. wrong annotation. this is really:
bienvenu%nventure.com	2004-08-19 07:41
mozilla/mailnews/imap/src/nsImapUrl.cpp	 1.169

and i'm way too sleepy to be doing this.

Comment 103

[David :Bienvenu]

Marcel, can you give any more details? The work around may be to clear out the cruft from prefs.js, the account prefs that don't correspond to accounts in the account list - mail.accountmanager.accounts contains a list of accounts, and then there are corresponding prefs like mail.account.account9.identities, and mail.account.account9.server. So, you could remove the prefs for accounts that aren't in the list. And you could remove the servers that aren't hooked up to accounts.

But I'm not sure that's really going to help - I think we ignore those unused prefs, for the most part, because we build up the in memory structures from the accountmanager.accounts pref, i.e., we add accounts for each of those, and then load the prefs for each of the servers pointed to by the accounts.

Comment 104

[Marcel van Beurden]

I just tried to add a new POP-account with the same user name and server as already exists in my profile. The wizard give me an error this is not possible. Why is this an error? 

It shouldn't be an error since sometimes you will have multiple accounts that you access with ssh through port forwarding. Let's say I have 2 accounts with ISP X and ISP Y and both have the same username. 

I would setup ssh forwarding like this:
localhost:8110 --> mail.ispx.com:110
localhost:9110 --> mail.ispy.com:110

In this case I would not be able to setup these both accounts in Thunderbird because it would complain I have two accounts with same server and user name.

So either the check would be removed or should include the port number while comparing. Right now it's not possible to provide a port number in the wizard.

Comment 105

[Marcel van Beurden]

Attachment: Error I get in Thunderbird 1.5.0.7 (20060909)

Comment 107

[wearyofallthiscrap]

Lots of patches and code review... from three years ago.  Still broken as of 1.5.0.13.

I can create a second IMAP account with the same host and username, but the port setting is ignored.  Actually, it's worse than ignored:  if I change the port number in the second account, the port number in the first account is also changed!

Comment 108

[Steve Kelem]

This is still broken in Thunderbird 2.0.0.0.  I used the Account Manager to make a second account at my server.  The new account doesn't show up inside the Account Manager and it doesn't show up in the Folders pane.  However, if I click on the 1st account at the server, it asks for the password for the (messed up) second account!

I'm pretty sure I have to edit the prefs.js file, but I'm not sure exactly how much stuff to take out.

Comment 109

[Magnus Melin [:mkmelin]]

This bug is done, isn't it? UI for it is bug 238583.

Comment 112

[Frank Eskesen]

You can kind of get around this problem by manipulating the mail.accountmanager.accounts list, putting the account that you want to use first in the list. Then your old working account silently disappears, replaced by the new one. Re-order the list to get the other account back. This works when the server/port number pair is identical as can easily occur in newsgroups. This workaround has not been explicitly tested for the condition actually described in the bug.

You could also possibly get sometimes around this (again for newsgroups) by using the multiple identities feature, if you could only always remember to use the correct identity consistently. I was hoping to be able to create different newsgroup "accounts" to access different newsgroups using the same server and the same port. Alas, this ten year old bug/feature gets in the way.

Comment 115

[Randell Jesup [:jesup] (needinfo me)]

My apologies, but this is insane that it hasn't been fixed in approaching 12 years (it was filed around the time I started working on Mozilla).  It look like we've come close to fixing it at least twice.

Even if those are horridly bit-rotted, the front-end account creation code SHOULD NOT let you create a known you-will-be-screwed situation.  Or at least it should warn you off strenuously!  Look at how close I came (bug 709442) to losing 2.5 months of email *forever* because our own admins thought this *should* work, to the point where they didn't even feel the need to test it with Thunderbird (just mail.app on Mac, I assume).

I'll morph bug 709442 into a "make the UI block dangerous account creates" bug, but guys, this is just clownshoes - we can't honestly call Thunderbird a solid client if it lets you do stuff like this.  One experience with a bug like this and entire organizations can swear off it forever.  

Leaving priority alone since bug 709442 is already critical, and that will block the dataloss side of things.

Comment 116

[onlywebmail]

Yes, i realy hate this *feature* of TB.
Sad thing ...

I´m not a programmer, but such bugs *should* be fixed, IMHO.


BTW, it is also not OK to just import all mails from each new account added!
I have accounts with gigabyte of collected mails - they should NOT be imported automatically! Realy ...

Comment 117

[:aceman]

(In reply to Magnus Melin from comment #109)
> This bug is done, isn't it? UI for it is bug 238583.

So why is it still open?

Comment 118

[Randell Jesup [:jesup] (needinfo me)]

Well, if the original comment in that bug is correct, that's just the UI front-end to whatever fix is implemented via this bug.  However, I haven't reviewed the patches there; the bug may have morphed into covering backend and frontend; if so this bug can probably be dupped against it.

Comment 119

[:aceman]

This bug here seems to be only about backend. I ask if that is finished because this bug here is still NEW.

Patches in bug 238583 seem to be frontend only. I could work on that bug but need to know if the backend is prepared for this.

Comment 121

[Magnus Melin [:mkmelin]]

Given the amount of patches that did go into the backend (in this bug), it likely works, possibly buggy.

Comment 123

[Graeme]

The error message seems to have changed, and the underlying problem does too (perhaps this requires a new bug report).

I can set up two POP accounts on the same server, but TB then warns me that ""the directory specified in the local directory setting is already used by the [other account name]". Checking the account settings shows that the local directory in server settings is the same. If I change it, it changes for both accounts.

Comment 124

[bugzilla]

Not only can you not really have multiple email accounts with differing servers, but if you do it just right trying to add a second one, you can actually get Thunderbird to obliterate your existing, working email configuration, as I just have. Guess I should've checked the twenty-year-old bugs before attempting it...

Comment 125

[e.weimann]

22 Years later and still a problem. Never had a a need for it in the past but discovered the problem today.
The local ImapMail folder gets a mess when trying to use multiple accounts on the same host...
Why not prefix the hostname with the username when creating the local ImapMail folder? Or give users the option to select the "Local Directory" while setting up the emailaccount instead of creating a mess when someone chooses to go for "Advanced config"...

Damn the auto-configure option!

Comment 126

[ola.e.karlsson]

I'm also running into this, the way my email provider has their servers setup, all different emails use the same imap ans smtp server adresses but different usernames and passwords. I do not seem to be able to set that up to work in Thunderbird 91.7.0.