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.
This commit is contained in:
Michael James Gratton 2017-01-25 11:16:20 +11:00
parent 721ecd892e
commit dec06d93be
8 changed files with 164 additions and 123 deletions

View file

@ -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;

View file

@ -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 ?? "";
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 />";
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;
StringBuilder html = new StringBuilder();
if (!is_draft) {
if (!Geary.String.is_empty(body)) {
html.append(body);
html.append(SPACER);
html.append(SPACER);
}
base.load_html(HTML_BODY.printf(html));
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));
}
/**
@ -345,6 +369,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.
*/

View file

@ -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
@ -591,10 +572,55 @@ 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,

View file

@ -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))

View file

@ -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) {

View file

@ -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();
}

View file

@ -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();
}

View file

@ -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>"