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

RESOLVED FIXED in Thunderbird 3.3a1

Status

Thunderbird
Account Manager
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: Bruce Gilson, Assigned: neil@parkwaycc.co.uk)

Tracking

({regression})

unspecified
Thunderbird 3.3a1
x86
Windows XP
regression
Bug Flags:
in-testsuite +

Firefox Tracking Flags

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

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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
Duplicate of this bug: 582395
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.
(Assignee)

Comment 4

7 years ago
Created attachment 461201 [details] [diff] [review]
Proposed patch
Attachment #461201 - Flags: superreview?(bugzilla)
Attachment #461201 - Flags: review?(bugzilla)

Comment 5

7 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.
status-thunderbird3.0: --- → unaffected
Keywords: relnote
Created attachment 461539 [details] [diff] [review]
MozMill Test

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+
Checked in on trunk: http://hg.mozilla.org/comm-central/rev/b8f34cb7b0af
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.2a1
Flags: in-testsuite?
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

7 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

7 years ago
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+
Created attachment 473469 [details] [diff] [review]
[checked in trunk+branch] MozMill Test v2

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: relnote → checkin-needed
Keywords: checkin-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.