[geary/bug/728002-webkit2: 109/140] Clean up how composer loads content into its web view.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/bug/728002-webkit2: 109/140] Clean up how composer loads content into its web view.
- Date: Tue, 31 Jan 2017 23:07:42 +0000 (UTC)
commit dec06d93bea25f669bd9cd7c533bc1143a246f86
Author: Michael James Gratton <mike vee net>
Date: Wed Jan 25 11:16:20 2017 +1100
Clean up how composer loads content into its web view.
The main gist of this is to ensure that the composer's widgets are
constructed seperately to loading its content, and that we only ever call
ComposerWebView::load_html precisely once per composer instance.
* src/client/composer/composer-widget.vala: Remove referred message,
quote text and draft flag param from constructor signature, move any
calls that loaded data from them to new load method. Don't load
anything into the editor here. Make loading the signature file async,
and call new ComposerWebView::updateSignature method on the editor to
update it.
(ComposerWidget::load): New async message for loading content into the
composer. Move related code from the constructor and GearyController
here, make methods that were previously public for that private
again. Tidy up calls a bit now that we have a single place from which
to do it all, and can understand the process a bit better.
(ComposerWidget::on_editor_key_press_event): Don't reload the editor to
remove the quoted text, call new ComposerWebView::delete_quoted_message
method on it instead.
* src/client/composer/composer-web-view.vala
(ComposerWebView): Add ::delete_quoted_message ::update_signature
methods, thunk to JS.
(ComposerWebView::load_html): Add quote and is_draft parameters,
construct HTML for the composer using apporporate spacing here, instead
of relying on all the disparate parts from doing the right thing.
* src/client/application/geary-controller.vala
(GearyController::create_compose_widget_async): Load composer content
after adding it to the widget hierarchy, set focus only after
everything is set up.
* src/engine/rfc822/rfc822-utils.vala (quote_email_for_reply,
quote_email_for_forward): Don't add extra padding around quoted parts -
let callers manage their own whitespace.
* test/client/components/client-web-view-test-case.vala
(TestCase:load_body_fixture): Make HTML param non-nullable, update
subclasses.
* ui/composer-web-view.js (ComposerPageState): Add ::updateSignature and
::deleteQuotedMessage method stubs.
src/client/application/geary-controller.vala | 54 +++------
src/client/composer/composer-web-view.vala | 68 ++++++++--
src/client/composer/composer-widget.vala | 131 +++++++++++--------
src/engine/rfc822/rfc822-utils.vala | 20 +--
.../components/client-web-view-test-case.vala | 2 +-
test/client/composer/composer-web-view-test.vala | 4 +-
test/js/composer-page-state-test.vala | 4 +-
ui/composer-web-view.js | 8 ++
8 files changed, 166 insertions(+), 125 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index 8452fba..b80e051 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -2306,30 +2306,9 @@ public class GearyController : Geary.BaseObject {
if (mailto != null) {
widget = new ComposerWidget.from_mailto(current_account, mailto, application.config);
} else {
- Geary.Email? full = null;
- if (referred != null) {
- try {
- full = yield email_stores.get(current_folder.account).fetch_email_async(
- referred.id, Geary.ComposedEmail.REQUIRED_REPLY_FIELDS,
- Geary.Folder.ListFlags.NONE, cancellable_folder);
- } catch (Error e) {
- message("Could not load full message: %s", e.message);
- }
- }
-
- widget = new ComposerWidget(current_account, compose_type, application.config, full, quote,
is_draft);
+ widget = new ComposerWidget(current_account, compose_type, application.config);
}
-
- Geary.EmailIdentifier? draft_id = null;
- if (is_draft) {
- draft_id = referred.id;
- // Restore widget state before displaying the composer and
- // opening the manager, so the changing widgets do not
- // flash at the user, or make it look like the draft has
- // changed hence triggering a redundant save
- yield widget.restore_draft_state_async();
- }
-
+ widget.destroy.connect(on_composer_widget_destroy);
widget.link_activated.connect((uri) => { open_uri(uri); });
widget.show_all();
@@ -2337,8 +2316,7 @@ public class GearyController : Geary.BaseObject {
// an exit without losing their data.
composer_widgets.add(widget);
debug(@"Creating composer of type $(widget.compose_type); $(composer_widgets.size) composers total");
- widget.destroy.connect(on_composer_widget_destroy);
-
+
if (inline) {
if (widget.state == ComposerWidget.ComposerState.NEW ||
widget.state == ComposerWidget.ComposerState.PANED) {
@@ -2358,22 +2336,22 @@ public class GearyController : Geary.BaseObject {
widget.state = ComposerWidget.ComposerState.DETACHED;
}
- // Now that the composer has been added to a window, we can
- // set up its focus.
- widget.set_focus();
-
- try {
- yield widget.open_draft_manager_async(draft_id);
- } catch (Error e) {
- debug("Could not open draft manager: %s", e.message);
+ // Load the widget's content
+ Geary.Email? full = null;
+ if (referred != null) {
+ try {
+ full = yield email_stores.get(current_folder.account).fetch_email_async(
+ referred.id, Geary.ComposedEmail.REQUIRED_REPLY_FIELDS,
+ Geary.Folder.ListFlags.NONE, cancellable_folder);
+ } catch (Error e) {
+ message("Could not load full message: %s", e.message);
+ }
}
+ yield widget.load(full, quote, is_draft);
- // For accounts with large numbers of contacts, loading the
- // entry completions can some time, so do it after the UI has
- // been shown
- yield widget.load_entry_completions();
+ widget.set_focus();
}
-
+
private bool should_create_new_composer(ComposerWidget.ComposeType? compose_type,
Geary.Email? referred, string? quote, bool is_draft, out bool inline) {
inline = true;
diff --git a/src/client/composer/composer-web-view.vala b/src/client/composer/composer-web-view.vala
index 24f2a00..e7de86a 100644
--- a/src/client/composer/composer-web-view.vala
+++ b/src/client/composer/composer-web-view.vala
@@ -61,7 +61,6 @@ public class ComposerWebView : ClientWebView {
</head><body>
<div id="message-body" dir="auto">%s</div>
</body></html>""";
- private const string CURSOR = "<span id=\"cursormarker\"></span>";
/**
@@ -178,19 +177,44 @@ public class ComposerWebView : ClientWebView {
/**
* Loads a message HTML body into the view.
*/
- public new void load_html(string? body, string? signature, bool top_posting) {
- string html = "";
- signature = signature ?? "";
-
- this.is_empty = Geary.String.is_empty(body);
- if (this.is_empty)
- html = CURSOR + "<br /><br />" + signature;
- else if (top_posting)
- html = CURSOR + "<br /><br />" + signature + body;
- else
- html = body + CURSOR + "<br /><br />" + signature;
-
- base.load_html(HTML_BODY.printf(html));
+ public new void load_html(string body,
+ string signature,
+ string quote,
+ bool top_posting,
+ bool is_draft) {
+ const string CURSOR = "<span id=\"cursormarker\"></span>";
+ const string SPACER = "<br />";
+
+ StringBuilder html = new StringBuilder();
+ if (!is_draft) {
+ if (!Geary.String.is_empty(body)) {
+ html.append(body);
+ html.append(SPACER);
+ html.append(SPACER);
+ }
+
+ if (!top_posting && !Geary.String.is_empty(quote)) {
+ html.append(quote);
+ html.append(SPACER);
+ }
+
+ html.append(CURSOR);
+
+ if (!Geary.String.is_empty(signature)) {
+ html.append(SPACER);
+ html.append(signature);
+ }
+
+ if (top_posting && !Geary.String.is_empty(quote)) {
+ html.append(SPACER);
+ html.append(SPACER);
+ html.append(quote);
+ }
+ } else {
+ html.append(quote);
+ }
+
+ base.load_html(HTML_BODY.printf(html.data));
}
/**
@@ -346,6 +370,22 @@ public class ComposerWebView : ClientWebView {
}
/**
+ * Updates the signature block if it has not been deleted.
+ */
+ public new void update_signature(string signature) {
+ this.run_javascript.begin(
+ "geary.updateSignature(\"%s\");".printf(signature), null
+ );
+ }
+
+ /**
+ * Removes the quoted message (if any) from the composer.
+ */
+ public void delete_quoted_message() {
+ this.run_javascript.begin("geary.deleteQuotedMessage();", null);
+ }
+
+ /**
* Returns the editor content as an HTML string.
*/
public async string? get_html() throws Error {
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index 44c6b13..5b81ede 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -252,8 +252,7 @@ public class ComposerWidget : Gtk.EventBox {
private ContactListStore? contact_list_store = null;
- private string? body_html = null;
- private string? signature_html = null;
+ private string body_html = "";
[GtkChild]
private Gtk.Box composer_container;
@@ -373,8 +372,7 @@ public class ComposerWidget : Gtk.EventBox {
public signal void link_activated(string url);
- public ComposerWidget(Geary.Account account, ComposeType compose_type, Configuration config,
- Geary.Email? referred = null, string? quote = null, bool is_referred_draft = false) {
+ public ComposerWidget(Geary.Account account, ComposeType compose_type, Configuration config) {
this.config = config;
this.header = new ComposerHeaderbar(config);
this.account = account;
@@ -478,30 +476,15 @@ public class ComposerWidget : Gtk.EventBox {
this.from = new Geary.RFC822.MailboxAddresses.single(account.information.primary_mailbox);
- if (referred != null) {
- fill_in_from_referred(referred, quote);
-
- if (is_referred_draft ||
- compose_type == ComposeType.NEW_MESSAGE ||
- compose_type == ComposeType.FORWARD) {
- this.pending_include = AttachPending.ALL;
- }
- }
-
- update_from_field();
- update_signature();
- update_pending_attachments(this.pending_include, true);
-
this.draft_timer = new Geary.TimeoutManager.seconds(
10, () => { this.save_draft.begin(); }
);
// Add actions once every element has been initialized and added
initialize_actions();
+ validate_send_button();
// Connect everything (can only happen after actions were added)
- validate_send_button();
- set_header_recipients();
this.to_entry.changed.connect(validate_send_button);
this.cc_entry.changed.connect(validate_send_button);
this.bcc_entry.changed.connect(validate_send_button);
@@ -518,8 +501,6 @@ public class ComposerWidget : Gtk.EventBox {
this.editor.mouse_target_changed.connect(on_mouse_target_changed);
this.editor.selection_changed.connect((has_selection) => { update_cursor_actions(); });
- this.editor.load_html(this.body_html, this.signature_html, this.top_posting);
-
this.editor_scrolled.add(editor);
// Place the message area before the compose toolbar in the focus chain, so that
@@ -592,9 +573,54 @@ public class ComposerWidget : Gtk.EventBox {
}
/**
+ * Loads the message into the composer editor.
+ */
+ public async void load(Geary.Email? referred = null,
+ string? quote = null,
+ bool is_referred_draft = false,
+ Cancellable? cancellable = null) {
+ this.last_quote = quote;
+ string referred_quote = "";
+ if (referred != null) {
+ referred_quote = fill_in_from_referred(referred, quote);
+ if (is_referred_draft ||
+ compose_type == ComposeType.NEW_MESSAGE ||
+ compose_type == ComposeType.FORWARD) {
+ this.pending_include = AttachPending.ALL;
+ }
+ }
+
+ yield restore_reply_to_state();
+
+ set_header_recipients();
+ update_from_field();
+ update_pending_attachments(this.pending_include, true);
+
+ string signature = yield load_signature(cancellable);
+ this.editor.load_html(
+ this.body_html,
+ signature,
+ referred_quote,
+ this.top_posting,
+ is_referred_draft
+ );
+
+ try {
+ yield open_draft_manager_async(is_referred_draft ? referred.id : null);
+ } catch (Error e) {
+ debug("Could not open draft manager: %s", e.message);
+ }
+
+ // For accounts with large numbers of contacts, loading the
+ // entry completions can some time, so do it after the UI has
+ // been shown
+ yield load_entry_completions();
+ }
+
+ /**
* Loads and sets contact auto-complete data for the current account.
*/
- public async void load_entry_completions() {
+ private async void load_entry_completions() {
// XXX Since ContactListStore hooks into ContactStore to
// listen for contacts being added and removed,
// GearyController or some composer-related controller should
@@ -616,13 +642,9 @@ public class ComposerWidget : Gtk.EventBox {
}
/**
- * Restores the composer's widget state from its draft.
- *
- * This should only be called once, after the composer has been
- * opened, and the draft manager should not be opened until after
- * this has completed.
+ * Restores the composer's widget state from any replied to messages.
*/
- public async void restore_draft_state_async() {
+ private async void restore_reply_to_state() {
bool first_email = true;
foreach (Geary.RFC822.MessageID mid in this.in_reply_to) {
@@ -682,15 +704,13 @@ public class ComposerWidget : Gtk.EventBox {
this.state = ComposerState.INLINE;
} else {
this.state = ComposerState.INLINE_COMPACT;
- // Set recipients in header
- set_header_recipients();
}
}
/**
* Creates and opens the composer's draft manager.
*/
- public async void open_draft_manager_async(
+ private async void open_draft_manager_async(
Geary.EmailIdentifier? editing_draft_id = null,
Cancellable? cancellable = null)
throws Error {
@@ -722,15 +742,16 @@ public class ComposerWidget : Gtk.EventBox {
}
// Copies the addresses (e.g. From/To/CC) and content from referred into this one
- private void fill_in_from_referred(Geary.Email referred, string? quote) {
+ private string fill_in_from_referred(Geary.Email referred, string? quote) {
+ string referred_quote = "";
if (this.compose_type != ComposeType.NEW_MESSAGE) {
add_recipients_and_ids(this.compose_type, referred);
this.reply_subject = Geary.RFC822.Utils.create_subject_for_reply(referred);
this.forward_subject = Geary.RFC822.Utils.create_subject_for_forward(referred);
}
this.pending_attachments = referred.attachments;
- this.last_quote = quote;
switch (this.compose_type) {
+ // Restoring a draft
case ComposeType.NEW_MESSAGE:
if (referred.from != null)
this.from = referred.from;
@@ -749,9 +770,9 @@ public class ComposerWidget : Gtk.EventBox {
try {
Geary.RFC822.Message message = referred.get_message();
if (message.has_html_body()) {
- this.body_html = message.get_html_body(null);
+ referred_quote = message.get_html_body(null);
} else {
- this.body_html = message.get_plain_body(true, null);
+ referred_quote = message.get_plain_body(true, null);
}
} catch (Error error) {
debug("Error getting draft message body: %s", error.message);
@@ -762,9 +783,9 @@ public class ComposerWidget : Gtk.EventBox {
case ComposeType.REPLY_ALL:
this.subject = reply_subject;
this.references = Geary.RFC822.Utils.reply_references(referred);
- this.body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_reply(referred, quote,
+ referred_quote = Geary.RFC822.Utils.quote_email_for_reply(referred, quote,
Geary.RFC822.TextFormat.HTML);
- if (quote != null)
+ if (!Geary.String.is_empty(quote))
this.top_posting = false;
else
this.can_delete_quote = true;
@@ -772,10 +793,11 @@ public class ComposerWidget : Gtk.EventBox {
case ComposeType.FORWARD:
this.subject = forward_subject;
- this.body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_forward(referred, quote,
+ referred_quote = Geary.RFC822.Utils.quote_email_for_forward(referred, quote,
Geary.RFC822.TextFormat.HTML);
break;
}
+ return referred_quote;
}
public void set_focus() {
@@ -2005,8 +2027,7 @@ public class ComposerWidget : Gtk.EventBox {
if (this.can_delete_quote) {
this.can_delete_quote = false;
if (event.keyval == Gdk.Key.BackSpace) {
- this.body_html = null;
- this.editor.load_html(this.body_html, this.signature_html, this.top_posting);
+ this.editor.delete_quoted_message();
return Gdk.EVENT_STOP;
}
}
@@ -2128,29 +2149,29 @@ public class ComposerWidget : Gtk.EventBox {
return false;
this.account = new_account;
- update_signature();
+ this.load_signature.begin(null, (obj, res) => {
+ this.editor.update_signature(this.load_signature.end(res));
+ });
load_entry_completions.begin();
return true;
}
- private void update_signature() {
- string? account_sig = null;
+ private async string load_signature(Cancellable? cancellable = null) {
+ string account_sig = "";
if (this.account.information.use_email_signature) {
- account_sig = account.information.email_signature;
+ account_sig = account.information.email_signature ?? "";
if (Geary.String.is_empty_or_whitespace(account_sig)) {
// No signature is specified in the settings, so use
// ~/.signature
-
- // XXX This loading should be async, but that needs to
- // be factored into how the signature HTML is passed
- // to the editor.
File signature_file = File.new_for_path(Environment.get_home_dir()).get_child(".signature");
- if (signature_file.query_exists()) {
- try {
- FileUtils.get_contents(signature_file.get_path(), out account_sig);
- } catch (Error error) {
+ try {
+ uint8[] data;
+ yield signature_file.load_contents_async(cancellable, out data, null);
+ account_sig = (string) data;
+ } catch (Error error) {
+ if (!(error is IOError.NOT_FOUND)) {
debug("Error reading signature file %s: %s", signature_file.get_path(),
error.message);
}
}
@@ -2158,10 +2179,10 @@ public class ComposerWidget : Gtk.EventBox {
account_sig = (!Geary.String.is_empty_or_whitespace(account_sig))
? Geary.HTML.smart_escape(account_sig, true)
- : null;
+ : "";
}
- this.signature_html = account_sig;
+ return account_sig;
}
private ComposerLinkPopover new_link_popover(ComposerLinkPopover.Type type,
diff --git a/src/engine/rfc822/rfc822-utils.vala b/src/engine/rfc822/rfc822-utils.vala
index 0d556bd..2b57333 100644
--- a/src/engine/rfc822/rfc822-utils.vala
+++ b/src/engine/rfc822/rfc822-utils.vala
@@ -228,9 +228,9 @@ public string email_addresses_for_reply(Geary.RFC822.MailboxAddresses? addresses
public string quote_email_for_reply(Geary.Email email, string? quote, TextFormat format) {
if (email.body == null && quote == null)
return "";
-
- string quoted = (quote == null) ? "<br /><br />" : "";
-
+
+ string quoted = "";
+
/// Format for the datetime that a message being replied to was received
/// See http://developer.gnome.org/glib/2.32/glib-GDateTime.html#g-date-time-format
string DATE_FORMAT = _("%a, %b %-e, %Y at %-l:%M %p");
@@ -255,14 +255,10 @@ public string quote_email_for_reply(Geary.Email email, string? quote, TextFormat
string QUOTED_LABEL = _("On %s:");
quoted += QUOTED_LABEL.printf(email.date.value.format(DATE_FORMAT));
}
-
+
quoted += "<br />";
-
quoted += "\n" + quote_body(email, quote, true, format);
-
- if (quote != null)
- quoted += "<br /><br />\n";
-
+
return quoted;
}
@@ -278,10 +274,8 @@ public string quote_email_for_reply(Geary.Email email, string? quote, TextFormat
public string quote_email_for_forward(Geary.Email email, string? quote, TextFormat format) {
if (email.body == null && quote == null)
return "";
-
- string quoted = "\n\n";
-
- quoted += _("---------- Forwarded message ----------");
+
+ string quoted = _("---------- Forwarded message ----------");
quoted += "\n\n";
string from_line = email_addresses_for_reply(email.from, format);
if (!String.is_empty_or_whitespace(from_line))
diff --git a/test/client/components/client-web-view-test-case.vala
b/test/client/components/client-web-view-test-case.vala
index 9854a6a..7324a99 100644
--- a/test/client/components/client-web-view-test-case.vala
+++ b/test/client/components/client-web-view-test-case.vala
@@ -34,7 +34,7 @@ public abstract class ClientWebViewTestCase<V> : Gee.TestCase {
protected abstract V set_up_test_view();
- protected virtual void load_body_fixture(string? html = null) {
+ protected virtual void load_body_fixture(string html = "") {
ClientWebView client_view = (ClientWebView) this.test_view;
client_view.load_html(html);
while (client_view.is_loading) {
diff --git a/test/client/composer/composer-web-view-test.vala
b/test/client/composer/composer-web-view-test.vala
index 5383e48..7b887bc 100644
--- a/test/client/composer/composer-web-view-test.vala
+++ b/test/client/composer/composer-web-view-test.vala
@@ -168,8 +168,8 @@ long, long, long, long, long, long, long, long, long, long,
return new ComposerWebView(this.config);
}
- protected override void load_body_fixture(string? html = null) {
- this.test_view.load_html(html, null, false);
+ protected override void load_body_fixture(string html = "") {
+ this.test_view.load_html(html, "", "", false, false);
while (this.test_view.is_loading) {
Gtk.main_iteration();
}
diff --git a/test/js/composer-page-state-test.vala b/test/js/composer-page-state-test.vala
index 42439c3..fe5f648 100644
--- a/test/js/composer-page-state-test.vala
+++ b/test/js/composer-page-state-test.vala
@@ -191,8 +191,8 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
return new ComposerWebView(this.config);
}
- protected override void load_body_fixture(string? html = null) {
- this.test_view.load_html(html, null, false);
+ protected override void load_body_fixture(string html = "") {
+ this.test_view.load_html(html, "", "", false, false);
while (this.test_view.is_loading) {
Gtk.main_iteration();
}
diff --git a/ui/composer-web-view.js b/ui/composer-web-view.js
index a81cd6f..7ef4b93 100644
--- a/ui/composer-web-view.js
+++ b/ui/composer-web-view.js
@@ -147,6 +147,14 @@ ComposerPageState.prototype = {
}
}
},
+ updateSignature: function(signature) {
+ // XXX need mark the sig somehow so we can find it, select
+ // it and replace it using execCommand
+ },
+ deleteQuotedMessage: function() {
+ // XXX need mark the quote somehow so we can find it, select
+ // it and delete it using execCommand
+ },
tabOut: function() {
document.execCommand(
"inserthtml", false, "<span style='white-space: pre-wrap'>\t</span>"
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
