User-Agent: Mozilla/5.0 (Windows; U; Win98; de-DE; rv:1.4b) Gecko/20030409 Build Identifier: Mozilla/5.0 (Windows; U; Win98; de-DE; rv:1.4b) Gecko/20030409 I've multiple smtp entries in my account manager. Originally they where in the order I created them (the same order the corresponding accounts are created and listed in the folders panel). But now they changed their orders and still changing them from time to time. It's no really big thing but if they where in the same order as the corresponding accounts or at least in a stable order would be very helpful. And I see this as a indicator that something must be wrong. Reproducible: Sometimes Steps to Reproduce:
I can confirm this. One part is a regression from fix of bug 96207 and one part was always there. The "resort" happens in two cases. 1. When a mail gets send, an list entry with the sending server gets created. If you then open the account manager, the rest of the servers will be added to the list - behind the entry created while the sending process. 2. If a smtp server gets deleted, a hole in the smtp server list emerges (e.g. smtp1, smtp3). If you then create a new one, it becomes smtp2 but will be appended to the list, so we have smtp1, smtp3, smtp2. Neither case 1 nor case 2 is a functional problem. But in case 1 this is an obvious error and should not occur. Three ways are possible for this: 1. Resort the list before display it. 2. Clear the list before loading the servers from the prefs.js into it. 3. Loading the servers into the list at startup of Mailnews. 1. is quite expensive. 2. can be dangerous when done while sending a mail. 3. Is easy and no big thing. So I'd vote for 3. Just add a call to loadSmtpServers() to nsSmtpService::nsSmtpService(). In case 2 it's not clear if it's how it should be or not. The server list will loose it's ascending order in meaning of the server names. But it saves the order the servers are created and to this it doesn't happen very often. So I don't think we'll change this.
Assignee: racham → ch.ey
Status: UNCONFIRMED → NEW
Ever confirmed: true
Created attachment 120859 [details] [diff] [review] proposed patch Patch for loading the smtp server list right in the constructor of nsSmtpService. That should make all other calls to loadSmtpServers() unnecessary. But I think for security reasons and since they aren't very costly, we can keep them.
Hey, you have a patch since nearly a month but it doesn't fix the problem. The order of the servers still changes from time to time.
Johannes, the patch has not been checked in and thus can't take effect. Patches need a review and super-review before checking in. I tried to get some attention in the past but failed. Now I'm directly trying to get a review.
I want to double check that this won't have any side effects. eyeballing it, I don't think it will (for migration, account creation, etc). it looks like alecf did it this way originally, so that we'd do the work lazily. but it looks like most (if not all) callers who do getservice on this service end up calling a method that calls loadSmtpServers() right away anyways. if we do take this for 1.4 final, we should clean it up in 1.5a 1) remove mSmtpServersLoaded 2) make loadSmtpServers() return void 3) remove the callers to loadSmtpServers() 4) clean up nsSmtpService::GetSmtpServers()
Does this apply to Thunderbird too?
This bug no longer applies, as the UI for the SMTP server selection has changed radically.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → WORKSFORME
Comment on attachment 120859 [details] [diff] [review] proposed patch Since this bug has been resolved as WFM, I'm removing the review request.
You need to log in before you can comment on or make changes to this bug.