From 6530c66e28c930c51b5094a6040d690b3ae858e9 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 25 Oct 2020 12:55:28 +1100 Subject: [PATCH 1/6] Geary.Imap.FolderSession: Fix null param critical with vala 0.50 Cancellable can be null, mark it as such - more fallout from GNOME/vala#299 Fixes #1044 --- src/engine/imap/api/imap-folder-session.vala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/engine/imap/api/imap-folder-session.vala b/src/engine/imap/api/imap-folder-session.vala index 8a2290cb..3b777ee6 100644 --- a/src/engine/imap/api/imap-folder-session.vala +++ b/src/engine/imap/api/imap-folder-session.vala @@ -85,8 +85,8 @@ private class Geary.Imap.FolderSession : Geary.Imap.SessionObject { public async FolderSession(ClientSession session, Imap.Folder folder, - Cancellable cancellable) - throws Error { + GLib.Cancellable? cancellable) + throws GLib.Error { base(session); this.folder = folder; this.quirks = session.quirks; From 6f1f94e5546c8df1b7627ebcd70edae8eab96fc6 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 25 Oct 2020 17:05:03 +1100 Subject: [PATCH 2/6] Composer.Widget: Suppress unsupported draft folder messages There's nothing people can do about the draft folder being unusable for saving drafts, so just log a debug message and continue. Fixes #858 --- src/client/composer/composer-widget.vala | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index 9148a88e..fe29011d 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -1533,6 +1533,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { : new Geary.EmailFlags() ); + bool opened = false; try { var new_manager = yield new Geary.App.DraftManager( this.sender_context.account, @@ -1548,7 +1549,13 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { new_manager.fatal .connect(on_draft_manager_fatal); this.draft_manager = new_manager; + opened = true; debug("Draft manager opened"); + } catch (Geary.EngineError.UNSUPPORTED err) { + debug( + "Drafts folder unsupported, no drafts will be saved: %s", + err.message + ); } catch (GLib.Error err) { this.header.show_save_and_close = false; throw err; @@ -1556,8 +1563,10 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { this.draft_manager_opening = null; } - update_draft_state(); - this.header.show_save_and_close = true; + this.header.show_save_and_close = opened; + if (opened) { + update_draft_state(); + } } /** From be7938316eb31a95ed2f461fea635aeebb458f1c Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 25 Oct 2020 18:49:10 +1100 Subject: [PATCH 3/6] Geary.App.DraftManager: Wait for remote to be open in ctor When working offline, the draft manager is more or less useless and will just consume memory as drafts pile up. Also, when opening a composer immediately after application launch, `FolderProperties.create_never_returns_id` may not have been updated yet, and hence will prevent drafts from being saved, even if supported. So after opening the drafts folder, block waiting for the remote to open so we get an accurate idea of if the folder is usable. Fixes drafts issue mentioned in #955 --- src/engine/app/app-draft-manager.vala | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/engine/app/app-draft-manager.vala b/src/engine/app/app-draft-manager.vala index 8e9863af..9d5a31b1 100644 --- a/src/engine/app/app-draft-manager.vala +++ b/src/engine/app/app-draft-manager.vala @@ -219,8 +219,17 @@ public class Geary.App.DraftManager : BaseObject { yield drafts_folder.open_async(Folder.OpenFlags.NO_DELAY, cancellable); - // if drafts folder doesn't return the identifier of newly created emails, then this object - // can't do it's work ... wait until open to check for this, to be absolutely sure + // if drafts folder doesn't return the identifier of newly + // created emails, then this object can't do it's work + // ... wait until open to check for this, to be absolutely + // sure + // + // Since open_async returns before a remote connection is + // made, need to wait for it here to ensure + var engine = this.drafts_folder as ImapEngine.MinimalFolder; + if (engine != null) { + yield engine.claim_remote_session(cancellable); + } if (drafts_folder.properties.create_never_returns_id) { try { yield drafts_folder.close_async(); From 6d5b0bc5b6ee45b6f31ba53d98c6a6c77e5ce2cd Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 25 Oct 2020 18:57:47 +1100 Subject: [PATCH 4/6] Composer.Widget: Rework draft manager management Since the draft manager will now block until a remote for the draft folder has been obtained, it now must be ensured that all calls to opening the manager are backgrounded so that the UI isn't blocked when offline. --- src/client/composer/composer-widget.vala | 76 ++++++++++++++++-------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index fe29011d..61e3a84c 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -971,7 +971,21 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { this.header.set_sensitive(enabled); if (enabled) { - this.open_draft_manager.begin(this.saved_id, null); + var current_account = this.sender_context.account; + this.open_draft_manager.begin( + this.saved_id, + (obj, res) => { + try { + this.open_draft_manager.end(res); + } catch (GLib.Error error) { + this.application.report_problem( + new Geary.AccountProblemReport( + current_account.information, error + ) + ); + } + } + ); } else { if (this.container != null) { this.container.close(); @@ -984,7 +998,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { public async void set_save_to_override(Geary.Folder? save_to) throws GLib.Error { this.save_to = save_to; - yield reopen_draft_manager(); + this.reopen_draft_manager.begin(); } /** @@ -1423,11 +1437,21 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { is_body_complete ); - try { - yield open_draft_manager(this.saved_id); - } catch (Error e) { - debug("Could not open draft manager: %s", e.message); - } + var current_account = this.sender_context.account; + this.open_draft_manager.begin( + this.saved_id, + (obj, res) => { + try { + this.open_draft_manager.end(res); + } catch (GLib.Error error) { + this.application.report_problem( + new Geary.AccountProblemReport( + current_account.information, error + ) + ); + } + } + ); } private async bool should_send() { @@ -1501,6 +1525,11 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { /** * Creates and opens the composer's draft manager. + * + * Note that since the draft manager may block until a remote + * connection is open, this method may likewise do so. Hence this + * method typically needs to be called from the main loop as a + * background async task using the `begin` async call form. */ private async void open_draft_manager(Geary.EmailIdentifier? editing_draft_id) throws GLib.Error { @@ -1572,13 +1601,21 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { /** * Closes current draft manager, if any, then opens a new one. */ - private async void reopen_draft_manager() - throws GLib.Error { + private async void reopen_draft_manager() { // Discard the draft, if any, since it may be on a different // account - yield close_draft_manager(DISCARD); - yield open_draft_manager(null); - yield save_draft(); + var current_account = this.sender_context.account; + try { + yield close_draft_manager(DISCARD); + yield open_draft_manager(null); + yield save_draft(); + } catch (GLib.Error error) { + this.application.report_problem( + new Geary.AccountProblemReport( + current_account.information, error + ) + ); + } } private async void close_draft_manager(DraftPolicy draft_policy) @@ -2254,20 +2291,7 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { this.update_signature.begin(null); load_entry_completions(); - var current_account = this.sender_context.account; - this.reopen_draft_manager.begin( - (obj, res) => { - try { - this.reopen_draft_manager.end(res); - } catch (GLib.Error error) { - this.application.report_problem( - new Geary.AccountProblemReport( - current_account.information, error - ) - ); - } - } - ); + this.reopen_draft_manager.begin(); } } } From 36daf80120a76b981a0d83e0765ad1720c5dba84 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 25 Oct 2020 18:59:45 +1100 Subject: [PATCH 5/6] Composer.Widget: Clean up set_save_to_override method signature The method now no longer requires async or throws an error, so remove both. --- .../application/application-plugin-manager.vala | 14 +------------- src/client/composer/composer-widget.vala | 5 ++--- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/src/client/application/application-plugin-manager.vala b/src/client/application/application-plugin-manager.vala index de1e01b0..1d736222 100644 --- a/src/client/application/application-plugin-manager.vala +++ b/src/client/application/application-plugin-manager.vala @@ -402,19 +402,7 @@ public class Application.PluginManager : GLib.Object { public void save_to_folder(Plugin.Folder? location) { var engine = this.application.globals.folders.to_engine_folder(location); if (engine != null && engine.account == this.backing.sender_context.account) { - this.backing.set_save_to_override.begin( - engine, - (obj, res) => { - try { - this.backing.set_save_to_override.end(res); - } catch (GLib.Error err) { - debug( - "Error setting folder for saving: %s", - err.message - ); - } - } - ); + this.backing.set_save_to_override(engine); } } diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index 61e3a84c..46986b8b 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -994,9 +994,8 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { } } - /** Overrides the draft folder as a destination for saving. */ - public async void set_save_to_override(Geary.Folder? save_to) - throws GLib.Error { + /** Overrides the folder used for saving drafts. */ + public void set_save_to_override(Geary.Folder? save_to) { this.save_to = save_to; this.reopen_draft_manager.begin(); } From 5533ae323d186df61d775b5bcfdb9f02dafd5f99 Mon Sep 17 00:00:00 2001 From: Michael Gratton Date: Sun, 25 Oct 2020 19:00:36 +1100 Subject: [PATCH 6/6] Composer.Widget: Hide save button by default Determining if this can be enabled currently requires opening the remote, so hide it by default until we know for sure. --- src/client/composer/composer-widget.vala | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala index 46986b8b..6b158f83 100644 --- a/src/client/composer/composer-widget.vala +++ b/src/client/composer/composer-widget.vala @@ -439,6 +439,8 @@ public class Composer.Widget : Gtk.EventBox, Geary.BaseInterface { this.header = new Headerbar(config); this.header.expand_composer.connect(on_expand_compact_headers); + // Hide until we know we can save drafts + this.header.show_save_and_close = false; // Setup drag 'n drop const Gtk.TargetEntry[] target_entries = { { URI_LIST_MIME_TYPE, 0, 0 } };