110693 - code cleanup: BuildPop3Url()

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

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

I think there are two bits of code that might not be necessary anymore.

see the comments in the code.

  // the following is only a temporary work around hack because necko
  // is loosing our port when the url is just scheme://host:port.
  // when they fix this bug I can remove the following code where we
  // manually set the port.
  //
  // XXX is this still necessary?
    
  // escape the username before we call SetUsername().  we do this because 
GetUsername()
  // will unescape the username
  //
  // XXX is this still necessary.  before removing, make sure to test on 
accounts where
  // the username has changed.

Comment 1

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

over to naving.

see the comments in the code.

Comment 2

[Navin Gupta]

Attachment: proposed fix

SetSpec sets the username and port, no need to set them separately.

Comment 3

[Navin Gupta]

cc bienvenu for review. darin, could you also review. 

Comment 4

[Darin Fisher]

Comment on attachment 58458 [details] [diff] [review]
proposed fix

r/sr=darin

Comment 5

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

seems fine, but have you tested on a pop account where the username has an @ 
sign?  have you tested on accounts where the username has changed, since the 
account was created?

Comment 6

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

> have you tested on accounts where the username has changed, since the 
account was created?

ignore that.  when we change the username, we only change it internally, and 
then when sending the username, we get the "real" username.  (double check 
this.)

but you should still test the first scenario.  if you do a query for fixed bugs 
containing "pop ssl" you'll see a bug that tells how to get a free email 
account that has "@" in the username.

Comment 7

[David :Bienvenu]

why would we remove the ability to use pop on non-default ports? Am I missing
something? Is that done some other way now, or was it not used? There's a bug
out there about non-default ports not working...

Comment 8

[David :Bienvenu]

Comment on attachment 58458 [details] [diff] [review]
proposed fix

ah, sorry, my bad.sr=bienvenu, as long as you do the test Seth asked for.

Comment 9

[Navin Gupta]

It works for both scenarios. '@' is escaped as '%40'. 

I am not removing ability for pop to work on non-default port. you can still
do that. I am just removing setting pop host and username explicitly. 

Comment 10

[Navin Gupta]

fix checked in. 

Comment 11

[[e-mail] (gone - use [e-mail] instead)]

1.84 naving%netscape.com Nov 19 21:31 110693 r=darin sr=bienvenu. Code clean in
nsPop3Service::BuildPop3Url.

Code cleanup is in, file bugs separately on POP3 as they exist.