518315 - Prefill default ports in "add new" form

Status: RESOLVED FIXED

(Webtools :: ISPDB Server, defect)

Description

[Robert Kaiser]

It would be convenient to prefill the incoming/outgoing ports fields with the default for the protocol, dependent on the values selected (POP/IMAP, socket types). I needed to look up the default POP3 port myself, and I'm probably among the more knowledgeable people ;-)

Comment 1

[Louie]

Does anyone know the default ports that we want to fill in?
i.e
pop3 = 110
imap = 143

I was wondering if
imap + ssl = ???
imap + tls = ???

Also, are there default ports to fill depending on outgoing socket type?

Comment 2

[Ludovic Hirlimann [:Usul]]

(In reply to comment #1)


Looking at http://www.iana.org/assignments/port-numbers

> I was wondering if
> imap + ssl = ???

993.

> imap + tls = ???

ditto

and pop3s = 995
 
> Also, are there default ports to fill depending on outgoing socket type?

I don't get the question.

Comment 3

[Robert Kaiser]

Anything + TLS has the same as without security, TLS only hooks in after the initial connection is made like it would be insecure, only the +SSL have different ports.
So here are the default ports for the protocols we support:

Incoming:
POP3, POP3+TLS: 110
POP3+SSL: 995
IMAP, IMAP+TLS: 143
IMAP+SSL: 993

Outgoing:
SMTP, SMTP+TLS: 25
SMTP+SSL: 465


I hope that info is everything you need. :)

Comment 4

[Louie]

Attachment: Adds port autofill in add form template

How do you spread a javascript expression over multiple lines?
I tried \ but firebug kept giving me an error.

Comment 5

[Blake Winton (:bwinton) (:☕️)]

Louie, you seem to be working on this, so I'm going to assign it to you.

Also, I think you can just use newlines if your expression is in ()s.
i.e.:
    if (autoFillIncoming.type == 'imap' &&
        (autoFillIncoming.socketType == undefined ||
         autoFillIncoming.socketType == 'TLS' ||
         autoFillIncoming.socketType == "None" ))

(Also, remember to check your indentation against the Coding Style Guide at https://wiki.mozilla.org/SeaMonkey:MailNews:CodingStyle)

Comment 6

[Louie]

Attachment: Adds Autofill

Updated to proper indentation + function naming style. Removed 80+ character lines.

Comment 7

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 410121 [details] [diff] [review]
Adds Autofill

So, first of all, there's a lot of trailing whitespace that we should get rid of, and the indentation should be 2-spaces pretty much everywhere.

>Index: templates/config/enter_config.html
>@@ -53,8 +53,79 @@
>+    $("#id_incoming_type_0").click(function(event){

There should be a space before the {.

>+function AutoFillIncoming(incomingType)
>+{
>+    var formToFill = document.getElementById("id_incoming_port")
>+    var portToUse = $("#id_incoming_port").val(); 
>+
>+    if(incomingType == 'pop3' || incomingType == 'imap')
>+       AutoFillIncoming.type = incomingType;
>+    else
>+       AutoFillIncoming.socketType = $("#id_incoming_socket_type").val();
>+
>+    if (AutoFillIncoming.type == 'imap' && 
>+        (AutoFillIncoming.socketType == undefined || 
>+         AutoFillIncoming.socketType == 'TLS' || 
>+         AutoFillIncoming.socketType == "None" ))
>+      portToUse = 143;
>+    else if (AutoFillIncoming.type == 'imap' && 
>+             AutoFillIncoming.socketType == 'SSL')
>+      portToUse = 993; 
>+    else if (AutoFillIncoming.type == 'pop3' && 
>+             (AutoFillIncoming.socketType == undefined || 
>+              AutoFillIncoming.socketType == 'TLS' || 
>+              AutoFillIncoming.socketType == "None" ))
>+      portToUse = 110;
>+    else if(AutoFillIncoming.type == 'pop3' && 
>+            AutoFillIncoming.socketType == 'SSL')
>+      portToUse = 995; 

Those seem like a lot of repeated conditions.  What about breaking it up into something like:

if (AutoFillIncoming.type == 'imap') {
  if (AutoFillIncoming.socketType == 'SSL')
    $("#id_incoming_port") = 993;
  else
    $("#id_incoming_port") = 143;
}
else if (AutoFillIncoming.type == 'pop3') {
  if (AutoFillIncoming.socketType == 'SSL')
    $("#id_incoming_port") = 995;
  else
    $("#id_incoming_port") = 110;
}
?

>+    formToFill.value = portToUse;
>+}
>+
>+function AutoFillOutgoing(outgoingSocketType)
>+{

Oh, and the {s go on the same line to match the other functions in this file.

>+    var formToFill = document.getElementById("id_outgoing_port");
>+    var portToUse; 
>+
>+    if(outgoingSocketType == "TLS" || outgoingSocketType == "None")
>+      portToUse = 25;
>+    else if(outgoingSocketType == "SSL")
>+      portToUse = 465; 
>+    else
>+      portToUse = $("#id_outgoing_port").val();
>+ 
>+    formToFill.value=portToUse;
>+}

So, I think this might be simpler if you only set the outgoing port when you needed to…  i.e.

function AutoFillOutgoing(outgoingSocketType) {
  var port = $("#id_outgoing_port");
  if(outgoingSocketType == "TLS" || outgoingSocketType == "None")
    port.value = 25;
  else if(outgoingSocketType == "SSL")
    port.value = 465;
}
(And actually, that function could be made even simpler.)

What do you think of those changes?

Later,
Blake.

Comment 8

[Louie]

Attachment: Patch v2

I've fixed up the patch as per your comments. One of the things I modified was using: 
port = document.getElementByID("id_incoming_port")
port.value = *new_value*
instead of:
 $("#id_incoming_port") = 993;
because the latter doesn't seem to work. I'm not quite why this is the case though.

Hopefully everything else is good.
:1,$[ ]$// was helpful with those whitespaces =].
Is there a way to show whitespace in vim?

Comment 9

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 415349 [details] [diff] [review]
Patch v2

(In reply to comment #8)
> One of the things I modified was
> using: 
> port = document.getElementByID("id_incoming_port")
> port.value = *new_value*
> instead of:
>  $("#id_incoming_port") = 993;
> because the latter doesn't seem to work. I'm not quite why this is the case
> though.

Because one of them is changing port.value, and the other one is changing port?  :)

> Hopefully everything else is good.
> :1,$[ ]$// was helpful with those whitespaces =].
> Is there a way to show whitespace in vim?

In your .vimrc, you can add:
--------------
augroup diff
  set includeexpr=substitute(v:fname,'^[ab]/','','')
  autocmd BufNewFile,BufRead *.diff\|*.patch match Error /^+.\{-}\zs\s\+$/
augroup END

augroup mozilla
  autocmd BufNewFile,BufRead *.c(pp)?\|*.js\|*.dtd\|*.x[mu]l match Error /\s\+$/
augroup END
--------------

Then, if you have syntax highlighting on, all the trailing spaces will show up in red.  :)

If you wanted to instead see all the whitespace, check out ":help list" and ":help listchars".

>Index: templates/config/enter_config.html
>@@ -53,8 +53,58 @@
>+function AutoFillIncoming(incomingType) {
>+  var port = document.getElementById("id_incoming_port")

I think there should be a ; at the end of this line.

>+  if(incomingType == 'pop3' || incomingType == 'imap')

These should be "s instead of 's.

>+    AutoFillIncoming.type = incomingType;

So, why is that "AutoFillIncoming.type"?
I think this might be nicer if we added the lines:
var type = "";
var socketType = "";
above the if, and then just used type and socketType in the funtion.

>+  else
>+    AutoFillIncoming.socketType = $("#id_incoming_socket_type").val();

Looking at it a little closer, I don't think that's the right thing to do, anyways.

What about just replacing the whole function:
function AutoFillIncoming(incomingType) {
  var port = document.getElementById("id_incoming_port")
  if(incomingType == 'pop3' || incomingType == 'imap')
    AutoFillIncoming.type = incomingType;
  else
    AutoFillIncoming.socketType = $("#id_incoming_socket_type").val();

  if (AutoFillIncoming.type == 'imap') {
    if (AutoFillIncoming.socketType == 'SSL')
      port.value = 993;
    else
      port.value = 143;
  }
  else if (AutoFillIncoming.type == 'pop3') {
    if (AutoFillIncoming.socketType == 'SSL')
      port.value = 995;
    else
      port.value = 110;
  }
}
with the function:
function AutoFillIncoming(incomingType) {
  var socketType = $("#id_incoming_socket_type").val();
  var port = $("#id_incoming_port");

  if (incomingType == "imap") {
    if (socketType == "SSL")
      port.value = 993;
    else
      port.value = 143;
  }
  else if (incomingType == "pop3") {
    if (socketType == "SSL")
      port.value = 995;
    else
      port.value = 110;
  }
}

(And log a new bug to figure out how to test these types of changes, because I'm not sure that I didn't mess that up.  ;)

With those changes, r=me.

Thanks,
Blake.

Comment 10

[Louie]

Attachment: patch v3

Fixed up javascript function to be stateless.

Comment 11

[Ben Bucksch (:BenB)]

+  if(outgoingSocketType == "TLS" || outgoingSocketType == "None")
+    port.value = 25;
+  else if(outgoingSocketType == "SSL")
+    port.value = 465;

Make that:
port.value = (outgoingSocketType == "SSL") ? 465 : 587;

Comment 12

[Ben Bucksch (:BenB)]

For your information, TLS != STARTTLS. See bug 533361 comment 1 and 8.

For reference:
IMAP: 143 (if STARTTLS or No transfer encryption)
IMAP: 993 (if SSL/TLS)
POP3: 143 (if STARTTLS or No transfer encryption)
POP3: 995 (if SSL/TLS)"
SMTP: 587 (if STARTTLS or No transfer encryption, and if available)
SMTP: 25  (if STARTTLS or No transfer encryption, and if 587 is not available)
SMTP: 465 (if SSL/TLS)"

Please add some description (like bug 533361 did) to the outgoing port field, that we should use 587 when available, and 25 otherwise. That description should be hidden when SMTP SSL is selected.

Comment 13

[David Ascher (:davida)]

Comment on attachment 416516 [details] [diff] [review]
patch v3

r=davida w/ ben's comment about what port to use when not using SSL.  (AFAICT, 25 is historical, and not current best practice).

Comment 14

[Ben Bucksch (:BenB)]

To clarify comment 12: I suggest as description text (underneath the field):
If outgoing is plain socket or STARTTLS, description should say something like
"Port 587 is the new standard port for SMTP mail submission (outgoing mail), port 25 is the old standard port. Please use port 587, if possible (even if the provider's documentation says port 25). Use port 25 only when 587 is not available."
(bwinton maybe has better and shorter wording proposals.)
If outgoing is SSL/TLS, description would change to
"Standard port for SMTP SSL is 465." instead of the above text.

Comment 11 was simpler code and prefills 587 instead of 25.

Comment 15

[Ben Bucksch (:BenB)]

FWIW, the spec is RFC 4409:
"Port 587 is reserved for email message submission"
and RFC 5068:
Sec 3.1: "Submission Port Use: MUAs SHOULD use the SUBMISSION port for message submission."
Sec 3.2: "As a default, MUAs SHOULD attempt to find the best possible submission port from a list of alternatives. The SUBMISSION port 587 SHOULD be placed first in the list."
(MUA = Mail user agent. Thunderbird is a MUA.)

Comment 16

[Louie]

Attachment: Final Patch

Updated outgoing SMTP port to 465 (no SSL)

Comment 17

[Blake Winton (:bwinton) (:☕️)]

I was a little worried there, until I noticed that you actually changed it to 587, as per Ben's comments.  :)

(Also, you missed Ben's "port.value = (outgoingSocketType == "SSL") ? 465 : 587;" change, so I threw that in for you. ;)

Checked in as http://viewvc.svn.mozilla.org/vc?view=revision&revision=58259

Later,
Blake.