167834 - Cancelling from advanced smtp settings dialog doesnt work correctly.

Status: VERIFIED FIXED

(SeaMonkey :: MailNews: Account Configuration, defect)

Description

[varada]

Currently when we open the advanced smtp server settings dialog and do Add or
Delete an smtp server it works regardless of whether we cancel out of that
dialog or click OK. This creates a problem for when we set the default to be
another smtp server and then cancel.
First we create a new smtp server and then assign it as the default
then we either delete the old default or let it be there.
The OnOk() of the dialog returns a true value and this causes the Outgoing SMTP
panel to be reloaded with the new default smtpserver.
When we do a cancel it doesnt reload the panel and hence the current page values
are saved as it is when we either go to another panel or go back to the advanced
settings window. This means that the new server that we had designated as the
default gets overwritten with the older default smtp server settings.

Comment 1

[varada]

nominating for buffy.

Comment 2

[varada]

Attachment: Patch V 1.0

We store the added and deleted smtp servers in two arrays. whenever a delete is
done we merely add the key to the deletedArray and remove it from the tree when
it refreshes. when we add a smtp server we store the key in the addedArray as
well as create the server. 
OnOK - we use the deletedArray values to get rid of deleted smtpServers as well
as pass a flag to replace the default servers(iff they have been deleted) with
new defaults.
OnCancel - we use the addedArray values to get rid of the added smtpServers as
well as set the default SmtpServer value back to the old value.

Comment 3

[varada]

ignore changes to am-copies.xul.

Comment 4

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

1)
 
+var oldDefaultSmtpServer;
+var addedSmtpServers = new Array;
+var deletedSmtpServers = new Array;

follow convention, and do:

var gOldDefaultSmtpServer;
var gAddedSmtpServers = new Array;
var gDeletedSmtpServers = new Array;

2)

+//  Instead of deleting the server we store the key in the array and delete it
later only if the 
+//  window is not cancelled.
+//  smtpService.deleteSmtpServer(server);

remove the commented out code, but keep the comments.

3)

+    var len = deletedSmtpServers.length;
+    deletedSmtpServers[len] = server.key;

var len not necessary, do:

+    deletedSmtpServers[deletedSmtpServers.length;] = server.key;

4)

most importantly, what happens if I add and delete, or delete and add a server
in the same session?

Comment 5

[varada]

>>4)
>>most importantly, what happens if I add and delete, or delete and add a server
>>in the same session?

1.When we add and delete and click ok - the server is deleted.
  when we add and delete and click cancel - the server is deleted.

2.When we delete and add and click ok - in this case any account that was
associated with this server will be disassociated and then set to the default
server.
 when we delete and add and click cancel - since it was only hidden and not
truly deleted the account that was associated with this server remains associated.

Comment 6

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

Comment on attachment 99398 [details] [diff] [review]
Patch V 1.0

1)

+    var len = deletedSmtpServers.length;
+    deletedSmtpServers[len] = server.key;

just do:

+ deletedSmtpServers[deletedSmtpServers.length] = server.key;

2)

does the right thing happen when you delete smtp servers in the same session
that you change the default smtp server?

when replaceWithDefaultSmtpServer()
gets called is smtpService.defaultServer
already set to the right thing?

I'm thinking about this scenario:
a)  change default from server1 to server2
b)  delete server1
c)  exit

Comment 7

[varada]

Attachment: Patch V1.1

Got rid of the var len and used the array.length. 
>>2)

>>does the right thing happen when you delete smtp servers in the same session
>>that you change the default smtp server?

>>when replaceWithDefaultSmtpServer()
>>gets called is smtpService.defaultServer
>>already set to the right thing?

>>I'm thinking about this scenario:
>>a)  change default from server1 to server2
>>b)  delete server1
>>c)  exit

When this scenario is present we do the right thing, change default to server2
and then delete server1 and any account using server1 will now default to
server2.

Action		  View in Edit Dialog	     onOk	   onCancel
Add		  server is added	     nothing	   deleted
Delete		  server is hidden	     deleted	   nothing
SetDefault	  default is set	     nothing	   old default is set.

Comment 8

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

Comment on attachment 99560 [details] [diff] [review]
Patch V1.1

sr=sspitzer

sounds like you've tested well.

please list all the scenarios you've tested so that QA can use them as a
starting point for the test plans.

Comment 9

[varada]

Marking Fixed.

Comment 10

[Ninoschka Baca]

Buffy Trunk build 2002-10-29: WinMe, Linux 8, Mac 10.1.3
Verified Fixed.(Ran scenarios using comments from #5 and #7)