code cleanup: BuildPop3Url()

VERIFIED FIXED

Status

MailNews Core
Backend
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: Navin Gupta)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
over to naving.

see the comments in the code.
Assignee: sspitzer → naving
(Assignee)

Comment 2

17 years ago
Created attachment 58458 [details] [diff] [review]
proposed fix

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

Comment 3

17 years ago
cc bienvenu for review. darin, could you also review. 

Comment 4

17 years ago
Comment on attachment 58458 [details] [diff] [review]
proposed fix

r/sr=darin
Attachment #58458 - Flags: review+
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?
> 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

17 years ago
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

17 years ago
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.
Attachment #58458 - Flags: superreview+
(Assignee)

Comment 9

17 years ago
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. 
(Assignee)

Comment 10

17 years ago
fix checked in. 
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → VERIFIED
OS: Windows 2000 → All
QA Contact: esther → stephend
Hardware: PC → All
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.