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)
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)
1.48 KB,
patch
|
standard8
:
review+
standard8
:
superreview+
standard8
:
approval-thunderbird3.1.2+
|
Details | Diff | Splinter Review |
16.45 KB,
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
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
Comment 2•14 years ago
|
||
confirmed by dupe
Status: UNCONFIRMED → NEW
Component: General → Account Manager
Ever confirmed: true
QA Contact: general → account-manager
Updated•14 years ago
|
blocking-thunderbird3.1: --- → ?
Comment 3•14 years ago
|
||
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.
Assignee | ||
Comment 4•14 years ago
|
||
Attachment #461201 -
Flags: superreview?(bugzilla)
Attachment #461201 -
Flags: review?(bugzilla)
Comment 5•14 years ago
|
||
> 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.
Updated•14 years ago
|
status-thunderbird3.0:
--- → unaffected
Comment 6•14 years ago
|
||
As Neil found the issue in the code for me, I wrote the MozMill test :-)
Attachment #461539 -
Flags: review?(bwinton)
Comment 7•14 years ago
|
||
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+
Comment 8•14 years ago
|
||
Checked in on trunk: http://hg.mozilla.org/comm-central/rev/b8f34cb7b0af
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.2a1
Updated•14 years ago
|
Flags: in-testsuite?
Comment 9•14 years ago
|
||
Checked into 1.9.2 for 3.1.2 and on default for 3.1.3:
http://hg.mozilla.org/releases/comm-1.9.2/rev/05459b075888
http://hg.mozilla.org/releases/comm-1.9.2/rev/ed6424a1194e
blocking-thunderbird3.1: .3+ → .2+
status-thunderbird3.1:
--- → .2-fixed
Comment 10•14 years ago
|
||
(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.
Comment 11•14 years ago
|
||
Works OK for me on 3.1.1 on Mac.
Comment 12•14 years ago
|
||
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+
Comment 13•14 years ago
|
||
Updated mozmill test based on Blake's comments.
Attachment #461539 -
Attachment is obsolete: true
Attachment #473469 -
Flags: review+
Comment 14•14 years ago
|
||
Setting checkin-needed so I don't forget to land the testcase when the tree reopens.
Keywords: relnote → checkin-needed
Updated•14 years ago
|
Keywords: checkin-needed
Comment 15•14 years ago
|
||
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 16•14 years ago
|
||
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
Updated•14 years ago
|
Flags: in-testsuite? → in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•