Closed Bug 580764 Opened 14 years ago Closed 14 years ago

Setting pop/imap port to the default value for the security setting not selected appears to get reset.

Categories

(Thunderbird :: Account Manager, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(blocking-thunderbird3.1 .2+, thunderbird3.1 .2-fixed, thunderbird3.0 unaffected)

RESOLVED FIXED
Thunderbird 3.3a1
Tracking Status
blocking-thunderbird3.1 --- .2+
thunderbird3.1 --- .2-fixed
thunderbird3.0 --- unaffected

People

(Reporter: mozilla, Assigned: neil)

References

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.2.7) Gecko/20100713 Firefox/3.6.7 ( .NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.2.7) Gecko/20100713 Thunderbird/3.1.1 In Thunderbird (Windoze) 3.1 and 3.1.1 this happens: I'm trying to access my NHSmail account (...@nhs.net - a Microsoft Exchange account - UK) They're very picky with security, but the following works just fine in Qualcomm Eudora 7.1.0.9 This version of Eudora is now very old and a bit creaky! POP: pop.nhs.net - Port 995 - SSL Required, Alternate Port SMTP: send.nhs.net - Port 587 - If Available, STARTTLS - Authentication Allowed With Thunderbird if you select STARTTLS for POP, the default port is 110 but you can change it to 995 (obviously) however, it does not 'remember' 995 and sets itself back to 110 Using port 110 gets a connection refused, as does SSL/TLS on port 995 A previous version of Thunderbird allowed and remembered this setting, connected successfully, though did not retrieve any mail! Perhaps this 'bug' will be fixed in a later release. Bruce Reproducible: Always Steps to Reproduce: As in 'Details', email account manually configured
confirmed by dupe
Status: UNCONFIRMED → NEW
Component: General → Account Manager
Ever confirmed: true
QA Contact: general → account-manager
blocking-thunderbird3.1: --- → ?
Neil and I have determined that this is a regression from bug 525238. The change to re-use some code (http://hg.mozilla.org/comm-central/rev/895928822eb3#l20.26) has the side effect of also trying to reset the port number for when it is the opposite default port than expected. So hopefully a better explanation: - Set the security setting to non-SSL - Set the port number to the default port for SSL - Select OK - Re-enter the account wizard and relevant pane => The default port shows as the default port for the non-SSL setting (the preferences are set correctly). This is an issue for both POP and IMAP. Neil has a fix for this.
Assignee: nobody → neil
Blocks: 525238
blocking-thunderbird3.1: ? → .3+
Keywords: regression
Summary: POP port not remembered when set to anything other than 110 → Setting pop/imap port to the default value for the security setting not selected appears to get reset.
Attached patch Proposed patchSplinter Review
Attachment #461201 - Flags: superreview?(bugzilla)
Attachment #461201 - Flags: review?(bugzilla)
> With Thunderbird if you select STARTTLS for POP, the default port is 110 > but you can change it to 995 (obviously) Just note that this server is misconfigured. 995 is the IMAP-over-SSL port, not normal IMAP with STARTTLS. I think there should be a better fix, but this one should work, and code is fine. r=BenB on the patch.
Keywords: relnote
Attached patch MozMill Test (obsolete) — Splinter Review
As Neil found the issue in the code for me, I wrote the MozMill test :-)
Attachment #461539 - Flags: review?(bwinton)
Comment on attachment 461201 [details] [diff] [review] Proposed patch Thanks Neil. As we also want this on 1.9.2 branch, I'll check it in everywhere for you.
Attachment #461201 - Flags: superreview?(bugzilla)
Attachment #461201 - Flags: superreview+
Attachment #461201 - Flags: review?(bugzilla)
Attachment #461201 - Flags: review+
Attachment #461201 - Flags: approval-thunderbird3.1.2+
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.2a1
Flags: in-testsuite?
blocking-thunderbird3.1: .3+ → .2+
(In reply to comment #3) > => The default port shows as the default port for the non-SSL setting (the > preferences are set correctly). That's not what the reporter reported: > however, it does not 'remember' 995 and sets itself back to 110 > Using port 110 gets a connection refused, as does SSL/TLS on port 995 I see another 3.1.1 user reporting this symptom, as well: http://getsatisfaction.com/mozilla_messaging/topics/3_1_1_xp_incoming_server_port_wont_save Haven't tried this myself....yet.
Works OK for me on 3.1.1 on Mac.
Comment on attachment 461539 [details] [diff] [review] MozMill Test >+++ b/mail/test/mozmill/account/test-account-port-setting.js >@@ -0,0 +1,169 @@ >+// This test expects the first account to be a POP account with port number 110 >+// and no security >+const PORT_NUMBERS_TO_TEST = >+ [ >+ 110, // The original port number. We don't input this the though. Is there an extra "the" up there? >+ 456, // Random port number. >+ 995, // The SSL port number. >+ 110 // Back to the original. >+ ]; >+ >+var gTestNumber; >+ >+function clickTreeCell(controller, tree, rowIndex, columnIndex, eventDetails) So, it seems a little odd to have some utility functions before the setupModule, and others after. Can we group them all together? >+function setupModule(module) { >+ let wh = collector.getModule("window-helpers"); >+ wh.installInto(module); >+ let fdh = collector.getModule("folder-display-helpers"); >+ fdh.installInto(module); >+} >+ >+// Open the Account Manager from the Mail Account Setup Wizard >+function open_advanced_settings(callback) { >+ plan_for_modal_dialog("mailnews:accountmanager", callback); >+ mc.click(new elib.Elem(mc.menus.tasksMenu.menu_accountmgr)); >+ return wait_for_modal_dialog("mailnews:accountmanager"); >+} >+ >+// Emulate manual input >+function input_value(controller, str) { >+ for (let i = 0; i < str.length; i++) >+ controller.keypress(null, str.charAt(i), {}); >+} Wow, do these functions ever look similar to the ones in mail/test/mozmill/account/test-mail-account-setup-wizard.js. Let's move them all into a more appropriate file, and fix up the places we call them. And we can even add delete_existing to that file, too, if you're up for it. >+function subtest_check_set_port_number(amc) { >+ amc.waitForEval("subject.currentAccount != null", 6000, 600, amc.window); >+ >+ clickTreeCell(amc, amc.window.document.getElementById("accounttree"), >+ 1, 0, {}); >+ >+ amc.waitForEval("subject.pendingAccount == null", 6000, 600, amc.window); >+ >+ let iframe = amc.window.document.getElementById("contentFrame"); >+ let portElem = iframe.contentDocument.getElementById("server.port"); >+ portElem.focus(); >+ >+ if (portElem.value != String(PORT_NUMBERS_TO_TEST[gTestNumber - 1])) So, you always seem to be making strings out of the numbers in PORT_NUMBERS_TO_TEST. What do you think about saving a few characters, and just storing the port numbers in the array as strings? (I think that would even shorten all the lines that are longer than 80 characters, too. :) >+ throw new Error("Port Value is not " + >+ String(PORT_NUMBERS_TO_TEST[gTestNumber - 1]) + >+ " as expected, it is: " + portElem.value); >+ >+ delete_existing(amc, new elib.Elem(portElem), 3); >+ input_value(amc, String(PORT_NUMBERS_TO_TEST[gTestNumber])); >+ >+ mc.sleep(0); >+ >+ amc.window.document.getElementById("accountManager").acceptDialog(); >+} >+ >+function subtest_check_port_number(amc) { This function looks a little too similar to the previous function for my liking. What do you think about removing the "acceptDialog" line, and then calling this function from subtest_check_set_port_number? (My other thought was that you could pass in an optional "set" function, when you wanted to also set the port number.) >+function test_account_port_setting() { >+ for (gTestNumber = 1; gTestNumber < PORT_NUMBERS_TO_TEST.length; ++gTestNumber) { I know you say it up above, but maybe we want to mention that we're starting at 1 because we only want to check the first port number. Then again, I figured it out, so maybe it doesn't really matter. So, I trust that you can fix the things I've mentioned, so I'm going to say r=me with those changes. If you feel that they end up being bigger than you're comfortable with, feel free to ask me for another review. Thanks, Blake.
Attachment #461539 - Flags: review?(bwinton) → review+
Updated mozmill test based on Blake's comments.
Attachment #461539 - Attachment is obsolete: true
Attachment #473469 - Flags: review+
Setting checkin-needed so I don't forget to land the testcase when the tree reopens.
Keywords: relnotecheckin-needed
Comment on attachment 473469 [details] [diff] [review] [checked in trunk+branch] MozMill Test v2 Checked in: http://hg.mozilla.org/comm-central/rev/bcb4ed3a3eb9 (I've got it on my queue for branch as well).
Attachment #473469 - Attachment description: MozMill Test v2 → [checked in] MozMill Test v2
Comment on attachment 473469 [details] [diff] [review] [checked in trunk+branch] MozMill Test v2 Also checked test into 1.9.2 branch: http://hg.mozilla.org/releases/comm-1.9.2/rev/4e5ea677fec3 for 3.1.5
Attachment #473469 - Attachment description: [checked in] MozMill Test v2 → [checked in trunk+branch] MozMill Test v2
Flags: in-testsuite? → in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: