GNOME Bugzilla – Bug 418954
Add a separate entry combo for port numbers
Last modified: 2011-04-07 00:33:49 UTC
I read yet another review where the write told that it's difficult to know how to specify a port for the SMTP server and/or the POP/IMAP server. Basically, no user knows that you have to add ":port" after the name of the server.
hmm, perhaps a "Port (if known)" field next to the "server" field? even if no user knows this, my main question would be: how many users need this? at least it's part of the user docs.
Some suggestions: 1) Using a tooltip for server entry boxes, giving example of mail.example.com:110 etc. 2) Filled server entry boxes, which would clear the example string on click/focus. Entry box can have example string like "mail.example.com:110". 3) Ignore it. Really, if someone is willing to use non standard port first thing s/he would try is to add ':' at the end of domain.
Baris: (3) is what we thought too, but it's somehow the last thing people think of in reality (I have no idea why it isn't the first) the first 2 ideas are decent workarounds, tho what probably needs to happen is an added port GtkEntry box.
Does an "advanced settings" options button or tab make sense?, i don't know the hig point of view on this, but could be useful if there's more things to add later, and this way the main window won't be over charged with options. BTW, there's a bug related to this at https://bugs.launchpad.net/ubuntu/+source/evolution/+bug/160304
Created attachment 99320 [details] [review] Patch file for to add help message before server entry box. Adding text box for help message in Imap/smtp/pop account setup window before server entry box. It specify, how to specify a port for the SMTP server and/or the POP/IMAP server. Message have example string "mail.example.com:110".
Rework. 1. Your glade changes are unnecessary. It removes translation from a few things 2. There should be no N_(" "). It would be better to be NULL. 3. The text needs to be corrected. Space_after_comma and better language.
and "portdesc" as a displayed name in the User Interface is a total no-go. ashish, do you think a user or translator understands what "portdesc" means?
*** Bug 512927 has been marked as a duplicate of this bug. ***
ashish, can you rework your patch so we can get this in for 2.23.1? the current situation is not obvious enough and leads to plenty of bug reports. what about having an additional entry field "Port (optional):" in the first-time wizards and edit->prefs->accounts->edit, like for the "post office agent SOAP port" we currently have for groupwise accounts? srag, can we set this to high priority for 2.23, please?
Andre: Sure.
Bumping version to a stable release.
In Evolution 2.22.1 in Debian Testing even if I specify the port number on the SMTP server it connects to the port 25, while you are looking into this please look for any possible problems/bugs in that code... Thanks very much indeed!
@Ivan: Totally different issue, please file a seperate bug report and explain which port you want to use and which other settings (SSL/TLS) you use. Thanks.
*** Bug 569898 has been marked as a duplicate of this bug. ***
Best approach for this I think is to use a GtkEntryCombo alongside the server entry field, with descriptive entries for standard port numbers in the popup menu. I did this recently while rewriting the LDAP config dialog. Lame attempt to illustrate the popup menu: Server: [____________________] Port: [ 143 |v] +--------------------------+ | 143 Standard IMAP Port | | 993 IMAP over TLS/SSL | +--------------------------+ When the port number in the entry box matches one of the items in the popup menu, the GtkEntryCombo widget should set a tooltip with the same descriptive text as used in the popup menu. For example, in the illustration above the tooltip text would be "Standard IMAP Port". Would be good to capture this behaviour in a stand-alone widget class (EPortEntry) so we can reuse it across Evolution.
Further thoughts: To facilitate property bindings, the EPortEntry class should have a "port" property (type: unsigned int, range: 0 - G_MAXUINT16) and perhaps also a boolean "valid" property. The "valid" property would go FALSE if the user attempts to input garbage in the entry box, or if the port number is outside the valid range. The "valid" property could then be used to help determine whether a dialog's OK button should sensitive or not.
Created attachment 182696 [details] [review] Add PortEntry struct to evolution-data-server camel providers This patch adds an array of structs PortEntry to every Camel provider. The struct containes port number, text description and is_ssl boolean value. The structs are then used in account configuration to load list of available ports and display them in combobox next to the hostname entry (see the next patch).
Created attachment 182698 [details] [review] Imeplemts EPortEntry widget and adds it to account configuration This patch implements EPortEntry widget and adds it to account configuration dialog. The widget has properties port and is_valid. At this moment, it provides only e_port_entry_set_camel_entries() method to add the ports to model (see previous patch), but the API can be easily extended to support another way of adding ports. Secondary, this patch adds the EPortEntry to the account editor dialog next to the hostname. When user changes the encryption (SSL/no-SSL) respective port is actived in the EPortEntry. When an invalid value is set in the EPortEntry, the dialog cannot be confirmed.
Created attachment 182702 [details] [review] Fixed patch with EPortEntry, the previous fails to build
Review of attachment 182696 [details] [review]: Looks great! In the SMTP list we should also have { 587, _("Message submission port"), FALSE } to cover RFC 2476. I'll take a look at the widget next.
Review of attachment 182702 [details] [review]: Widget looks good. I'd still like to see a tooltip when the port number matches one of the combo box items (see comment #15). Also we need to check that the port number is within the range defined by the "port" property, and set "is-valid" to FALSE if it's not (atoi() alone isn't sufficient). Also, and this is purely an aesthetic request, but I'd like the descriptive text in the drop-down list to be insensitive (grayed out) so the port numbers stand out more. You can do this by simply setting the description text renderer's "sensitive" property to FALSE: g_object_set (renderer, "sensitive", FALSE, NULL) GNOME 3.0 is in string and UI lockdown, so this feature will have to wait until we create a "gnome-3-0" branch and the master branch becomes 3.1 development. I think with the refinements I mentioned this is pretty much ready to go. Nice job!
Created attachment 183058 [details] [review] Add PortEntry struct to evolution-data-server camel providers Added SMTP port 587, as mentioned in comment #20
Created attachment 183059 [details] [review] Introduces EPortEntry widget and uses it in Account Editor Some improvements and fixes in EPortEntry, as mentioned in comments above: - fixed port-range checking (added check for upper limit, for lower limit and invalid (non-numerical) input check atoi() seems to be enough) - port descriptions are insensitive - when a default port is selected the description is set as widget's tooltip - check if user didn't happen to manually typed in a default port number and in such case set matching tooltip too
Review of attachment 183058 [details] [review]: Patch has all the changes listed twice, but otherwise looks ready to commit.
Review of attachment 183059 [details] [review]: Nice, the new patch addresses everything and works great! I noticed a few memory leaks in EPortEntry from embedded g_strdup_printf() calls. The string returned from g_strdup_printf() must always be explicitly freed. I fixed it locally, so no need for a new patch. I have both patches staged to commit for 3.1 after we branch on Monday.
I spotted some initialization issues when viewing an existing account. The port entry's value was getting stomped on by the late call to emae_service_provider_changed(). I reworked the initialization logic and I *think* it's okay now, but that code is such a mess that who knows for sure. I'll keep an eye out for any regressions. Anyway, committed the E-D-S and Evo patches for 3.1.1: http://git.gnome.org/browse/evolution/commit/?id=07f943554d6d5f945fc9de59b2a14571537c51ef http://git.gnome.org/browse/evolution-data-server/commit/?id=446b65e90dd40ef03ebe621694ee504bd8a23f10
*** Bug 623341 has been marked as a duplicate of this bug. ***
I can't remember why I didn't see this in the docs, it's possible that I hadn't “built my index”, or some such nonsense, (using Fedora twelve, net install CD, maybe the doc index wasn't built, or something,) I don't remember, but at any rate, having a bit of the screen indicate something like “enter your outgoing SMTP server in the following format”: “your.server.name:proper_port_number” ...would be incredibly easy to do. In response to “how many users need this? at least it's part of the user docs.” They ALL need it. Either something is fully and properly documented, or it isn't. Again, I may have missed something somewhere, but I've been in tech support (other areas of expertise, obviously), for twenty years, but _I_ didn't know that I was supposed to do that. In fact, I'm nearly positive that earlier versions of Evolution either did it automatically, or else they were 'docced' properly, and in the right place... Thanx fer' listening... jdg
This wasn't supposed to be a comment, it was supposed to be a reply to a particular comment above. I've got too many windows open, and too many buffers to flush. I should just log out and go home. Acck!!! :^p jdg