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
Created attachment 58458 [details] [diff] [review] proposed fix SetSpec sets the username and port, no need to set them separately.
cc bienvenu for review. darin, could you also review.
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.
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 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+
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.
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
You need to log in before you can comment on or make changes to this bug.