diff --git a/src/client/accounts/accounts-editor-row.vala b/src/client/accounts/accounts-editor-row.vala index 11a49f02..44336775 100644 --- a/src/client/accounts/accounts-editor-row.vala +++ b/src/client/accounts/accounts-editor-row.vala @@ -172,6 +172,7 @@ private abstract class Accounts.ServiceRow : AccountRow V value) { base(account, label, value); this.service = service; + this.service.notify.connect(on_notify); bool is_editable = this.is_value_editable; set_activatable(is_editable); @@ -186,6 +187,14 @@ private abstract class Accounts.ServiceRow : AccountRow } } + ~ServiceRow() { + this.service.notify.disconnect(on_notify); + } + + private void on_notify() { + update(); + } + } @@ -302,13 +311,40 @@ internal class Accounts.SmtpAuthComboBox : Gtk.ComboBoxText { } +/** + * Displaying and manages validation of popover-based forms. + */ internal class Accounts.EditorPopover : Gtk.Popover { - internal Gtk.Grid layout { get; private set; default = new Gtk.Grid(); } + internal Gtk.Grid layout { + get; private set; default = new Gtk.Grid(); + } + + internal Gee.Collection validators { + owned get { return this.validator_backing.read_only_view; } + } protected Gtk.Widget popup_focus = null; + private Gee.Collection validator_backing = + new Gee.LinkedList(); + + + /** + * Emitted when a validated widget is activated all are valid. + * + * This signal will be emitted when all of the following are true: + * + * 1. At least one validator has been added to the popover + * 2. The user activates an entry that is being monitored by a + * validator + * 3. The validation for the has completed (i.e. is not in + * progress) + * 4. All validators are in the valid state + */ + public override signal void valid_activated(); + public EditorPopover() { get_style_context().add_class("geary-editor"); @@ -352,6 +388,11 @@ internal class Accounts.EditorPopover : Gtk.Popover { } } + public void add_validator(Components.Validator validator) { + validator.activated.connect(on_validator_activated); + this.validator_backing.add(validator); + } + public void add_labelled_row(string label, Gtk.Widget value) { Gtk.Label label_widget = new Gtk.Label(label); label_widget.get_style_context().add_class(Gtk.STYLE_CLASS_DIM_LABEL); @@ -366,6 +407,13 @@ internal class Accounts.EditorPopover : Gtk.Popover { destroy(); } + private void on_validator_activated() { + if (Geary.traverse(this.validator_backing).all( + (v) => v.state == Components.Validator.Validity.VALID)) { + valid_activated(); + } + } + } diff --git a/src/client/accounts/accounts-editor-servers-pane.vala b/src/client/accounts/accounts-editor-servers-pane.vala index b10e640e..1274177d 100644 --- a/src/client/accounts/accounts-editor-servers-pane.vala +++ b/src/client/accounts/accounts-editor-servers-pane.vala @@ -17,6 +17,8 @@ internal class Accounts.EditorServersPane : Gtk.Grid, EditorPane, AccountPane { protected weak Accounts.Editor editor { get; set; } + private Geary.Engine engine; + // These are copies of the originals that can be updated before // validating on apply, without breaking anything. private Geary.ServiceInformation imap_mutable; @@ -26,6 +28,9 @@ internal class Accounts.EditorServersPane : Gtk.Grid, EditorPane, AccountPane { [GtkChild] private Gtk.HeaderBar header; + [GtkChild] + private Gtk.Overlay osd_overlay; + [GtkChild] private Gtk.Grid pane_content; @@ -41,6 +46,13 @@ internal class Accounts.EditorServersPane : Gtk.Grid, EditorPane, AccountPane { [GtkChild] private Gtk.ListBox sending_list; + [GtkChild] + private Gtk.Button apply_button; + + [GtkChild] + private Gtk.Spinner apply_spinner; + + private SaveDraftsRow save_drafts; private ServiceSmtpAuthRow smtp_auth; private ServiceLoginRow smtp_login; @@ -48,12 +60,12 @@ internal class Accounts.EditorServersPane : Gtk.Grid, EditorPane, AccountPane { public EditorServersPane(Editor editor, Geary.AccountInformation account) { this.editor = editor; this.account = account; - - this.pane_content.set_focus_vadjustment(this.pane_adjustment); - + this.engine = ((GearyApplication) editor.application).engine; this.imap_mutable = account.imap.temp_copy(); this.smtp_mutable = account.smtp.temp_copy(); + this.pane_content.set_focus_vadjustment(this.pane_adjustment); + this.details_list.set_header_func(Editor.seperator_headers); // Only add an account provider if it is esoteric enough. if (this.account.imap.mediator is GoaMediator) { @@ -69,20 +81,23 @@ internal class Accounts.EditorServersPane : Gtk.Grid, EditorPane, AccountPane { service_provider.set_dim_label(true); service_provider.activatable = false; this.details_list.add(service_provider); - this.details_list.add(new SaveDraftsRow(this.account)); + this.save_drafts = new SaveDraftsRow(this.account); + this.details_list.add(this.save_drafts); this.receiving_list.set_header_func(Editor.seperator_headers); - this.receiving_list.add(new ServiceHostRow(account, account.imap)); - this.receiving_list.add(new ServiceSecurityRow(account, account.imap)); - this.receiving_list.add(new ServiceLoginRow(account, account.imap)); + this.receiving_list.add(new ServiceHostRow(account, this.imap_mutable)); + this.receiving_list.add(new ServiceSecurityRow(account, this.imap_mutable)); + this.receiving_list.add(new ServiceLoginRow(account, this.imap_mutable)); this.sending_list.set_header_func(Editor.seperator_headers); - this.sending_list.add(new ServiceHostRow(account, account.smtp)); - this.sending_list.add(new ServiceSecurityRow(account, account.smtp)); - this.smtp_auth = new ServiceSmtpAuthRow(account, account.smtp); + this.sending_list.add(new ServiceHostRow(account, this.smtp_mutable)); + this.sending_list.add(new ServiceSecurityRow(account, this.smtp_mutable)); + this.smtp_auth = new ServiceSmtpAuthRow( + account, this.smtp_mutable, this.imap_mutable + ); this.smtp_auth.value.changed.connect(on_smtp_auth_changed); this.sending_list.add(this.smtp_auth); - this.smtp_login = new ServiceLoginRow(account, account.smtp); + this.smtp_login = new ServiceLoginRow(account, this.smtp_mutable); this.sending_list.add(this.smtp_login); this.account.information_changed.connect(on_account_changed); @@ -99,11 +114,114 @@ internal class Accounts.EditorServersPane : Gtk.Grid, EditorPane, AccountPane { return this.header; } + private async void save(GLib.Cancellable? cancellable) { + this.apply_button.set_sensitive(false); + this.apply_spinner.show(); + this.apply_spinner.start(); + + // Only need to validate if a generic account + bool is_valid = true; + bool has_changed = false; + if (this.account.service_provider == Geary.ServiceProvider.OTHER) { + is_valid = yield validate(cancellable); + + if (is_valid) { + has_changed = this.engine.update_account_service( + this.account, imap_mutable + ); + has_changed = this.engine.update_account_service( + this.account, smtp_mutable + ); + } + } + + if (is_valid) { + if (this.save_drafts.value_changed) { + this.account.save_drafts = this.save_drafts.value.state; + has_changed = true; + } + + if (has_changed) { + this.account.information_changed(); + } + + this.editor.pop(); + } + + this.apply_button.set_sensitive(true); + this.apply_spinner.stop(); + this.apply_spinner.hide(); + } + + private async bool validate(GLib.Cancellable? cancellable) { + string message = ""; + bool imap_valid = false; + try { + yield this.engine.validate_imap( + this.account, this.imap_mutable, cancellable + ); + imap_valid = true; + } catch (Geary.ImapError.UNAUTHENTICATED err) { + debug("Error authenticating IMAP service: %s", err.message); + // Translators: In-app notification label + message = _("Check your receiving login and password"); + } catch (GLib.Error err) { + debug("Error validating IMAP service: %s", err.message); + // Translators: In-app notification label + message = _("Check your receiving server details"); + } + + bool smtp_valid = false; + if (imap_valid) { + debug("Validating SMTP..."); + try { + yield this.engine.validate_smtp( + this.account, + this.smtp_mutable, + this.imap_mutable.credentials, + cancellable + ); + smtp_valid = true; + } catch (Geary.SmtpError.AUTHENTICATION_FAILED err) { + debug("Error authenticating SMTP service: %s", err.message); + // There was an SMTP auth error, but IMAP already + // succeeded, so the user probably needs to + // specify custom creds here + this.smtp_auth.value.source = Geary.SmtpCredentials.CUSTOM; + // Translators: In-app notification label + message = _("Check your sending login and password"); + } catch (GLib.Error err) { + debug("Error validating SMTP service: %s", err.message); + // Translators: In-app notification label + message = _("Check your sending server details"); + } + } + + bool is_valid = imap_valid && smtp_valid; + debug("Validation complete, is valid: %s", is_valid.to_string()); + + if (!is_valid) { + add_notification( + new InAppNotification( + // Translators: In-app notification label, the + // string substitution is a more detailed reason. + _("Account not updated: %s").printf(message) + ) + ); + } + + return is_valid; + } + + private void add_notification(InAppNotification notification) { + this.osd_overlay.add_overlay(notification); + notification.show(); + } + private void update_smtp_auth() { this.smtp_login.set_visible( this.smtp_auth.value.source == Geary.SmtpCredentials.CUSTOM ); - this.smtp_login.update(); } [GtkCallback] @@ -113,6 +231,7 @@ internal class Accounts.EditorServersPane : Gtk.Grid, EditorPane, AccountPane { [GtkCallback] private void on_apply_button_clicked() { + this.save.begin(null); } [GtkCallback] @@ -178,7 +297,6 @@ private class Accounts.AccountProviderRow : ); this.accounts = accounts; - update(); } @@ -228,21 +346,34 @@ private class Accounts.SaveDraftsRow : AccountRow { + public bool value_changed { + get { return this.initial_value != this.value.state; } + } + + private bool initial_value; + + public SaveDraftsRow(Geary.AccountInformation account) { + Gtk.Switch value = new Gtk.Switch(); base( account, // Translators: This label describes an account // preference. _("Save drafts on server"), - new Gtk.Switch() + value ); set_activatable(false); - update(); + value.notify["active"].connect(on_activate); } public override void update() { - this.value.state = this.account.save_drafts; + this.initial_value = this.account.save_drafts; + this.value.state = this.initial_value; + } + + private void on_activate() { + this.account.save_drafts = this.value.state; } } @@ -279,60 +410,58 @@ private class Accounts.ServiceHostRow : } public override void activated(EditorServersPane pane) { - EditorPopover popover = new EditorPopover(); - - string? value = this.service.host; + string? text = get_host_text() ?? ""; Gtk.Entry entry = new Gtk.Entry(); - entry.set_text(value ?? ""); - entry.set_placeholder_text(value ?? ""); + entry.set_text(text); + entry.set_placeholder_text(text); entry.set_width_chars(20); entry.show(); + EditorPopover popover = new EditorPopover(); popover.set_relative_to(this.value); popover.layout.add(entry); + popover.add_validator(new Components.NetworkAddressValidator(entry)); + popover.valid_activated.connect(on_popover_activate); popover.popup(); } public override void update() { - string value = this.service.host; + string value = get_host_text(); if (Geary.String.is_empty(value)) { value = _("None"); } - - // Only show the port if it not the appropriate default port - bool custom_port = false; - int port = this.service.port; - Geary.TlsNegotiationMethod security = this.service.transport_security; - switch (this.service.protocol) { - case Geary.Protocol.IMAP: - if (!(port == Geary.Imap.IMAP_PORT && - (security == Geary.TlsNegotiationMethod.NONE || - security == Geary.TlsNegotiationMethod.START_TLS)) && - !(port == Geary.Imap.IMAP_TLS_PORT && - security == Geary.TlsNegotiationMethod.TRANSPORT)) { - custom_port = true; - } - break; - case Geary.Protocol.SMTP: - if (!(port == Geary.Smtp.SMTP_PORT && - (security == Geary.TlsNegotiationMethod.NONE || - security == Geary.TlsNegotiationMethod.START_TLS)) && - !(port == Geary.Smtp.SUBMISSION_PORT && - (security == Geary.TlsNegotiationMethod.NONE || - security == Geary.TlsNegotiationMethod.START_TLS)) && - !(port == Geary.Smtp.SUBMISSION_TLS_PORT && - security == Geary.TlsNegotiationMethod.TRANSPORT)) { - custom_port = true; - } - break; - } - if (custom_port) { - value = "%s:%d".printf(value, this.service.port); - } - this.value.set_text(value); } + private string? get_host_text() { + string? value = this.service.host ?? ""; + if (!Geary.String.is_empty(value)) { + // Only show the port if it not the appropriate default port + uint16 port = this.service.port; + if (port != this.service.get_default_port()) { + value = "%s:%d".printf(value, this.service.port); + } + } + return value; + } + + private void on_popover_activate(EditorPopover popover) { + Components.NetworkAddressValidator validator = + (Components.NetworkAddressValidator) Geary.traverse( + popover.validators + ).first(); + + GLib.NetworkAddress? address = validator.validated_address; + if (address != null) { + this.service.host = address.hostname; + this.service.port = address.port != 0 + ? (uint16) address.port + : this.service.get_default_port(); + } + + popover.popdown(); + } + } @@ -343,8 +472,9 @@ private class Accounts.ServiceSecurityRow : Geary.ServiceInformation service) { TlsComboBox value = new TlsComboBox(); base(account, service, value.label, value); - update(); + set_activatable(false); value.changed.connect(on_value_changed); + update(); } public override void update() { @@ -352,7 +482,17 @@ private class Accounts.ServiceSecurityRow : } private void on_value_changed() { - this.service.transport_security = this.value.method; + if (this.service.transport_security != this.value.method) { + // Update the port if we're currently using the default, + // otherwise keep the custom port as-is. + bool update_port = ( + this.service.port == this.service.get_default_port() + ); + this.service.transport_security = this.value.method; + if (update_port) { + this.service.port = this.service.get_default_port(); + } + } } } @@ -377,8 +517,6 @@ private class Accounts.ServiceLoginRow : } public override void activated(EditorServersPane pane) { - EditorPopover popover = new EditorPopover(); - string? value = null; if (this.service.credentials != null) { value = this.service.credentials.user; @@ -389,12 +527,19 @@ private class Accounts.ServiceLoginRow : entry.set_width_chars(20); entry.show(); + EditorPopover popover = new EditorPopover(); popover.set_relative_to(this.value); popover.layout.add(entry); + popover.add_validator(new Components.Validator(entry)); + popover.valid_activated.connect(on_popover_activate); popover.popup(); } public override void update() { + this.value.set_text(get_login_text()); + } + + private string? get_login_text() { string? label = null; if (this.service.credentials != null) { string method = "%s"; @@ -433,7 +578,15 @@ private class Accounts.ServiceLoginRow : // by an account's IMAP or SMTP service. label = _("None"); } - this.value.set_text(label); + return label; + } + + private void on_popover_activate(EditorPopover popover) { + Components.Validator validator = + Geary.traverse(popover.validators).first(); + this.service.credentials = + this.service.credentials.copy_with_user(validator.target.text); + popover.popdown(); } } @@ -441,13 +594,19 @@ private class Accounts.ServiceLoginRow : private class Accounts.ServiceSmtpAuthRow : ServiceRow { + + Geary.ServiceInformation imap_service; + + public ServiceSmtpAuthRow(Geary.AccountInformation account, - Geary.ServiceInformation service) { + Geary.ServiceInformation smtp_service, + Geary.ServiceInformation imap_service) { SmtpAuthComboBox value = new SmtpAuthComboBox(); - base(account, service, value.label, value); + base(account, smtp_service, value.label, value); + this.imap_service = imap_service; this.activatable = false; - update(); value.changed.connect(on_value_changed); + update(); } public override void update() { @@ -455,7 +614,23 @@ private class Accounts.ServiceSmtpAuthRow : } private void on_value_changed() { - this.service.smtp_credentials_source = this.value.source; + if (this.service.smtp_credentials_source != this.value.source) { + // The default SMTP port also depends on the auth method + // used, so also update the port here if we're currently + // using the default, otherwise keep the custom port + // as-is. + bool update_port = ( + this.service.port == this.service.get_default_port() + ); + this.service.smtp_credentials_source = this.value.source; + this.service.credentials = + (this.service.smtp_credentials_source != CUSTOM) + ? null + : new Geary.Credentials(Geary.Credentials.Method.PASSWORD, ""); + if (update_port) { + this.service.port = this.service.get_default_port(); + } + } } } diff --git a/src/engine/api/geary-credentials.vala b/src/engine/api/geary-credentials.vala index a9039aca..50a14453 100644 --- a/src/engine/api/geary-credentials.vala +++ b/src/engine/api/geary-credentials.vala @@ -84,6 +84,10 @@ public class Geary.Credentials : BaseObject, Gee.Hashable { return this.token != null; } + public Credentials copy_with_user(string user) { + return new Credentials(this.supported_method, user, this.token); + } + public Credentials copy_with_token(string? token) { return new Credentials(this.supported_method, this.user, token); } diff --git a/src/engine/api/geary-engine.vala b/src/engine/api/geary-engine.vala index 4768d7c0..f8bafbba 100644 --- a/src/engine/api/geary-engine.vala +++ b/src/engine/api/geary-engine.vala @@ -433,6 +433,59 @@ public class Geary.Engine : BaseObject { } } + /** + * Changes the service configuration for an account. + * + * This updates an account's service configuration with the given + * configuration, by copying it over the account's existing + * configuration for that service. The corresponding {@link + * Account.incoming} or {@link Account.outgoing} client service + * will also be updated so that the new configuration will start + * taking effect immediately. + * + * Returns true if the account's service was updated, or false if + * the configuration was the same. + */ + public bool update_account_service(AccountInformation account, + ServiceInformation updated) { + // Ensure account is closed. + Account? impl = this.account_instances.get(account.id); + if (impl == null) { + throw new EngineError.BAD_PARAMETERS( + "Account has not been added to the engine: %s", account.id + ); + } + + ServiceInformation? existing = null; + ClientService? service = null; + switch (updated.protocol) { + case Protocol.IMAP: + existing = account.imap; + service = impl.incoming; + break; + case Protocol.SMTP: + existing = account.smtp; + service = impl.outgoing; + break; + } + + bool was_updated = false; + if (service != null) { + if (!existing.equal_to(updated)) { + existing.copy_from(updated); + was_updated = true; + + Endpoint endpoint = get_shared_endpoint( + account.service_provider, existing + ); + impl.set_endpoint(service, endpoint); + account.information_changed(); + } + } + + return was_updated; + } + private Geary.Endpoint get_shared_endpoint(ServiceProvider provider, ServiceInformation service) { string key = "%s/%s:%u".printf( diff --git a/src/engine/api/geary-service-information.vala b/src/engine/api/geary-service-information.vala index 0eb6f093..c7cdd221 100644 --- a/src/engine/api/geary-service-information.vala +++ b/src/engine/api/geary-service-information.vala @@ -233,6 +233,7 @@ public abstract class Geary.ServiceInformation : GLib.Object { } + /** Returns a temporary copy of this service for editing. */ public abstract ServiceInformation temp_copy(); /** @@ -262,6 +263,25 @@ public abstract class Geary.ServiceInformation : GLib.Object { return port; } + /** + * Returns true if another object is equal to this one. + */ + public bool equal_to(Geary.ServiceInformation other) { + return ( + this == other || + (this.host == other.host && + this.port == other.port && + this.use_starttls == other.use_starttls && + this.use_ssl == other.use_ssl && + (this.credentials == null && other.credentials != null || + this.credentials != null && this.credentials.equal_to(other.credentials)) && + this.mediator == other.mediator && + this.remember_password == other.remember_password && + this.smtp_noauth == other.smtp_noauth && + this.smtp_use_imap_credentials == other.smtp_use_imap_credentials) + ); + } + public void copy_from(Geary.ServiceInformation from) { this.host = from.host; this.port = from.port; diff --git a/ui/accounts_editor_servers_pane.ui b/ui/accounts_editor_servers_pane.ui index f54e07f2..6b2101e4 100644 --- a/ui/accounts_editor_servers_pane.ui +++ b/ui/accounts_editor_servers_pane.ui @@ -2,6 +2,158 @@ + True False @@ -31,6 +183,17 @@ True False + 12 + + + True + False + + + 0 + 0 + + Apply @@ -43,7 +206,7 @@ - 0 + 1 0 @@ -59,147 +222,4 @@ 1 10 -