SMTP Editor, change default server twice resul ts w/same name

VERIFIED FIXED in M18

Status

SeaMonkey
MailNews: Account Configuration
P2
normal
VERIFIED FIXED
19 years ago
14 years ago

People

(Reporter: Ninoschka Baca, Assigned: Alec Flett)

Tracking

Trunk
All
Windows NT

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3-][PDTP2][rtm++]Fix in hand)

Attachments

(4 attachments)

(Reporter)

Description

19 years ago
Build 2000-04-13-09M16: NT4, Linux 6.0, Mac 8.5.1

Overview: Changing the default smtp server twice results in both servers having 
the same name.

Steps to reproduce:
A. You should have 2 SMTP Servers:
1. Open Account Settings and go to the Outgoing SMTP Server panel
2. Select the Advanced button and it should display a default server (i.e. 
"server1 (default)")
3. Add another server to the list (i.e. server2)
4. Select OK to close the SMTP Editor
5. Select OK to close Account Settings

B. After you have 2 SMTP servers:
6. Go back to the Outgoing SMTP Server panel
7. Select the Advanced button
8. In the SMTP Editor, select server2 and select the "Set Default" button and it 
should display "server2 (default)".
9. Select the OK button to close the SMTP Editor 
10. Focus should now be in the Ougoing SMTP Server panel and the Server Name 
should say "server2". **Do not close Account Settings**
11. Select the Advanced button again
12. Select server1 and select the "Set Default" button and it should display 
"server1 (default)"
13. Select the OK button to close the SMTP Editor

Actual Results: Notice that 
- the Server Name in the Outgoing SMTP Server panel states "server2" when it 
should state "server1"
- Select the Advanced button again and it still shows that server1 is the 
default followed by server2.
- Select OK to close the SMTP Editor and again the Server Name in the Outgoing 
SMTP Server panel still points to "server2"
- Select OK to close Account Settings
- Open Account Settings, go to the Outgoing SMTP Server panel, select the 
Advanced button and now both servers repeat the same name:
  - server2 (default)
  - server2

Expected Results: After switching the default server twice it should update 
displaying the latest changes for the Server Name.
(Assignee)

Updated

19 years ago
Status: NEW → ASSIGNED
Priority: P3 → P2
Target Milestone: --- → M16

Comment 1

19 years ago
Cleanup work, marking M18.
Target Milestone: M16 → M18

Comment 2

18 years ago
nbaca, after you last step if you were to close the account settings dialog and 
then reopen it, would it still say Server2, Server2?  Or would it have the 
correct information?

If it has the correct information and it only happens on these specific steps, 
then we will probably move this to the future milestone.
(Reporter)

Comment 3

18 years ago
It displays incorrect information (server2, server2) after:
- Closing the SMTP Editor, closing Account Settings, reopening Account Settings 
and viewing the Outgoing SMTP settings including the Advanced button.
- After quitting and restarting it still shows server2, server2 in the SMTP 
Editor.

Comment 4

18 years ago
adding b3mail keyword.
Keywords: b3mail

Comment 5

18 years ago
Adding nsbeta3 to b3mail bugs.
Keywords: nsbeta3

Comment 6

18 years ago
Removing b3mail keyword (these bugs have been promoted now.)
Keywords: b3mail

Updated

18 years ago
Summary: SMTP Editor, change default server twice results w/same name → SMTP Editor, change default server twice resul ts w/same name
Whiteboard: [nsbeta3-]
Target Milestone: M18 → Future
(Assignee)

Comment 7

18 years ago
this bug manifests itself in a really common situation, which I just hit: Create 
a new SMTP server and make it the default. You end up blowing away your default 
smtp server.. what I consider dataloss and it really cripples this feature.

It was a oneline fix, so I'm renominating for nsbeta3.
Basically I had forgotten to refresh the smtp page after the user hits OK in the 
smtp server list

(attaching the patch now)
Whiteboard: [nsbeta3-] → Fix in hand
(Assignee)

Comment 8

18 years ago
Created attachment 12713 [details] [diff] [review]
one-line fix - call full onload handler to clear widgets

Updated

18 years ago
Whiteboard: Fix in hand → Fix in hand[nsbeta3+]

Comment 9

18 years ago
marking [nsbeta3+]
(Assignee)

Comment 10

18 years ago
*** Bug 47008 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 11

18 years ago
fix is in
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
(Reporter)

Updated

18 years ago
QA Contact: lchiang → nbaca
(Reporter)

Comment 12

18 years ago
Build 2000-09-12-08M18: NT4
Reopening

- nsmail-1 was originally set to default
- I added nsmail-2 and made it the default
- Exit, restart the application
- Now only nsmail-2 is listed, and nsmail-1 is gone.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 13

18 years ago
*sigh* this was exactly the case (even the same hostnames) that I used to test 
this originally :( wonder what broke now
Status: REOPENED → ASSIGNED

Comment 14

18 years ago
PDT agrees P2
Whiteboard: Fix in hand[nsbeta3+] → Fix in hand[nsbeta3+][PDTP2]
(Assignee)

Comment 15

18 years ago
hooray for seth, he fixed this yesterday as a side effect of fixing another bug
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 16

18 years ago
Build 2000-09-15-06M18: NT4
Reopening. SMTP Editing results in many strange occurances:

1. The original problem reported on 4/13/2000 still exists.
2. The problem reported on 9/12/2000 also still exists (only nsmail-2 is 
present). Try and add nsmail-1 back in and now the entries are duplicated:

nsmail-2 (Defalt)
nsmail-1
nsmail-2 (Default
nsmail-1
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
alec, I still see this bug too.  I don't think my fix affects this bug.

I think you were thinking of #41856, which got fixed by fix for #51546

I'll do a little investigating to see if I can figure out this wildabeast.
(Assignee)

Comment 18

18 years ago
ok, I figured out what's going on. the smtp servers are not getting loaded when
you call GetDefaultServer() - which means if you call GetDefaultServer() before
you call GetServers(), then you ONLY get the default server... the servers are
finally loaded when you call CreateSmtpServer().


attaching a fix. loadSmtpServers is a no-op if the servers are already loaded,
and we already do this same thing in CreateSmtpServers().
We really should probably put loadSmtpServers() inside of every smtp
server-related call, but I think that is not worth the risk at this stage.
Status: REOPENED → ASSIGNED
(Assignee)

Comment 19

18 years ago
Created attachment 14827 [details] [diff] [review]
proposed fix.. can I get a reviewer?
I'll test and review this patch now.

give me about 5 minutes.
that fix works for me.  r=sspitzer.

mscott also gives it the super-review, if you need one to check in.

Updated

18 years ago
Whiteboard: Fix in hand[nsbeta3+][PDTP2] → [nsbeta3+][PDTP2]Fix in hand
Target Milestone: Future → M18
(Assignee)

Comment 22

18 years ago
fix is in.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 23

18 years ago
Build 2000-09-19-05M18: NT4
Reopening

1. The original problem reported on 4/13/2000 still exists.
2. The 9/12/2000 problem is fixed.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 24

18 years ago
dunno what's going on here, removing fix in hand comment
Status: REOPENED → ASSIGNED
Whiteboard: [nsbeta3+][PDTP2]Fix in hand → [nsbeta3+][PDTP2]
(Assignee)

Comment 25

18 years ago
I noticed that if you
1) open the server list
2) edit the default server's hostname
3) hit ok, back to the main smtp page

the second time you do this, the default smtp's hostname is not updated... could
be related.

Comment 26

18 years ago
Created attachment 15368 [details] [diff] [review]
Should fix it !
(Assignee)

Comment 27

18 years ago
no that fix will return the selected server, which means that the default server
will get the same values as the server you select in the picker, which would be
bad.
(Assignee)

Comment 28

18 years ago
good LORD. I'm amazed this ever worked. I had two big 'ol typos.
Attaching a patch for someone's review.

In here basically:
in restorePage() I had currentPageId and currentServerId reversed! This allowed
it work the first time, amazingly enough, because.. well, I'm not sure. but in
any case, this is what would cause it to fail the second time.

When I fixed this, the page would NEVER restore.

And so in the second part, I clear the array appropriately - I don't know what
the heck I was thinking when I wrote clearAccountData the first time.

Now attaching the patch and testing
(Assignee)

Comment 29

18 years ago
Created attachment 15561 [details] [diff] [review]
the real fix
(Assignee)

Comment 30

18 years ago
nominating for rtm since this probably won't make beta3
Keywords: rtm
Whiteboard: [nsbeta3+][PDTP2] → [nsbeta3+][PDTP2]Fix in hand
the fix looks good.

r=sspitzer

Comment 32

18 years ago
I can see why this is low risk, but how big is the impact of not fixing this?
Whiteboard: [nsbeta3+][PDTP2]Fix in hand → [nsbeta3+][PDTP2][rtm need info]Fix in hand

Comment 33

18 years ago
Doesn't seem like a PR3 stopper. Marking nsbeta3- and letting rtm nomination 
take over.
Whiteboard: [nsbeta3+][PDTP2][rtm need info]Fix in hand → [nsbeta3-][PDTP2][rtm need info]Fix in hand
(Assignee)

Comment 34

18 years ago
the impact is dataloss for the user - they can potentially overwrite their 
preferences and lose their default SMTP server, which will prevent them from 
sending mail.
(Assignee)

Comment 35

18 years ago
oops - removing rtm need info since I supplied the info.
This fix is INCREDIBLY low risk. The latest patch ONLY affects the exact case of
this bug, and nothing else.
Whiteboard: [nsbeta3-][PDTP2][rtm need info]Fix in hand → [nsbeta3-][PDTP2]Fix in hand

Comment 36

18 years ago
Wouldn't normally rtm+ since the case is rather obscure.  I'll talk to the PDT
in person about whether this can be taken on the basis of extreme low risk.
Provisional rtm+ to keep it on the radar for the moment.
Whiteboard: [nsbeta3-][PDTP2]Fix in hand → [nsbeta3-][PDTP2][rtm+]Fix in hand

Comment 37

18 years ago
can we get an "sr=" as well? It'll help speed up the process.

Updated

18 years ago
Whiteboard: [nsbeta3-][PDTP2][rtm+]Fix in hand → [nsbeta3-][PDTP2][rtm+ need info]Fix in hand
(Assignee)

Comment 38

18 years ago
adding mscott for super review - please review the "the real fix" patch.

Comment 39

18 years ago
sr=mscott
(Assignee)

Comment 40

18 years ago
removing need info because I have a reviewer and super reviewer.
Whiteboard: [nsbeta3-][PDTP2][rtm+ need info]Fix in hand → [nsbeta3-][PDTP2][rtm+]Fix in hand

Comment 41

18 years ago
PDT marking [rtm++]
Whiteboard: [nsbeta3-][PDTP2][rtm+]Fix in hand → [nsbeta3-][PDTP2][rtm++]Fix in hand
(Assignee)

Comment 42

18 years ago
fix checked into trunk, just waiting for branch tree to open.
(Assignee)

Comment 43

18 years ago
ok, fix finally in the branch.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 44

18 years ago
Verifying this bug.
(Reporter)

Comment 45

18 years ago
Branch build 2000-10-06-09MN6: NT4
Branch build 2000-10-06-10MN6: Linux 6.0
Branch build 2000-10-06-13MN6: Mac 9.04
Fixed.

Marking 'vtrunk' so it's checked in the trunk.
Keywords: vtrunk

Comment 46

18 years ago
Verified on branch.

OK using:
2000-10-11-09-MN6 commercial build on mac and NT 
2000-10-14-09-MN6 on Linux
Status: RESOLVED → VERIFIED
(Reporter)

Comment 47

18 years ago
Do you mean you verified it on the trunk?

Comment 48

18 years ago
gayatrib@netscape.com can you clarify where you verfied this?  is it now
verified on Branch and Trunk?
(Reporter)

Comment 49

18 years ago
It was a misunderstanding and Varada verified this on the branch so it still
needs to be verified on the trunk. 

Comment 50

18 years ago
I had verified this on the branch.

Comment 51

18 years ago
reopening to reresolve as Fixed.  Bugs stay resolved until verified on both the
branch and the trunk.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---

Comment 52

18 years ago
Resolving as Fixed.  vtrunk keyword is in place requesting trunk verification
before this bug changes state to Verified.  When this is verified on the trunk
vtrunk keyword should be removed and the bug Status should be set to Verified.
Sorry for the spam, just trying to make sure we cover all of our bases here.
Status: REOPENED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
(Reporter)

Comment 53

18 years ago
Build 2000-11-28-09: Win95, Linux 6.0, Mac 9.04
Verified Fixed on the trunk (removing 'vtrunk' from keywords).
Status: RESOLVED → VERIFIED
Keywords: vtrunk
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.