Commit 12c6bbca authored by Michael Gratton's avatar Michael Gratton 🤞

Fix signature not being updated when composer first opened without one

The ComposerPageState JS object assumed that if no signature was present
when first loaded, that none ever would be. This broke changing the
signature when the composer was opened for an account without one, and
the from account was changed to an account with a sig.

Instead of including the signature as part of the loaded body, always
include just a skeleton signature DIV and ensure the signature is loaded
dynamically after the body has been loaded. Update code and tests to
match this assumption, and add a unit test for updating the sig.

Fixes #309
parent 791c321a
Pipeline #67000 passed with stages
in 27 minutes and 14 seconds
......@@ -133,7 +133,6 @@ public class ComposerWebView : ClientWebView {
* Loads a message HTML body into the view.
*/
public new void load_html(string body,
string signature,
string quote,
bool top_posting,
bool is_draft) {
......@@ -142,9 +141,7 @@ public class ComposerWebView : ClientWebView {
const string BODY_PRE = """
<div id="geary-body" dir="auto">""";
const string BODY_POST = """</div>
""";
const string SIGNATURE = """
<div id="geary-signature" dir="auto">%s</div>
<div id="geary-signature" class="geary-no-display" dir="auto"></div>
""";
const string QUOTE = """
<div id="geary-quote" dir="auto"><br />%s</div>
......@@ -171,10 +168,6 @@ public class ComposerWebView : ClientWebView {
html.append(CURSOR);
html.append(BODY_POST);
if (!Geary.String.is_empty(signature)) {
html.append_printf(SIGNATURE, signature);
}
if (top_posting && !Geary.String.is_empty(quote)) {
html.append_printf(QUOTE, quote);
}
......
......@@ -461,6 +461,7 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
this.editor = new ComposerWebView(config);
this.editor.set_hexpand(true);
this.editor.set_vexpand(true);
this.editor.content_loaded.connect(on_editor_content_loaded);
this.editor.show();
this.body_container.add(this.editor);
......@@ -628,10 +629,8 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
update_attachments_view();
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
......@@ -2164,41 +2163,43 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
if (selected.account != this.account) {
this.account = selected.account;
this.load_signature.begin(null, (obj, res) => {
this.editor.update_signature(this.load_signature.end(res));
});
this.update_signature.begin(null);
load_entry_completions();
this.reopen_draft_manager_async.begin();
}
}
}
private async string load_signature(Cancellable? cancellable = null) {
string account_sig = "";
private async void update_signature(Cancellable? cancellable = null) {
string sig = "";
if (this.account.information.use_signature) {
account_sig = account.information.signature;
if (Geary.String.is_empty_or_whitespace(account_sig)) {
sig = account.information.signature;
if (Geary.String.is_empty_or_whitespace(sig)) {
// No signature is specified in the settings, so use
// ~/.signature
File signature_file = File.new_for_path(Environment.get_home_dir()).get_child(".signature");
try {
uint8[] data;
yield signature_file.load_contents_async(cancellable, out data, null);
account_sig = (string) data;
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);
}
}
}
account_sig = (!Geary.String.is_empty_or_whitespace(account_sig))
? Geary.HTML.smart_escape(account_sig)
: "";
}
return account_sig;
// Still want to update the signature even if it is empty,
// since when changing the selected from account, if the
// previously selected account had a sig but the newly
// selected account does not, the old sig gets cleared out.
if (Geary.String.is_empty_or_whitespace(sig)) {
// Clear out multiple spaces etc so smart_escape
// doesn't create &nbsp;'s
sig = "";
}
this.editor.update_signature(Geary.HTML.smart_escape(sig));
}
private async ComposerLinkPopover new_link_popover(ComposerLinkPopover.Type type,
......@@ -2230,6 +2231,10 @@ public class ComposerWidget : Gtk.EventBox, Geary.BaseInterface {
get_action(GearyApplication.ACTION_REDO).set_enabled(can_redo);
}
private void on_editor_content_loaded() {
this.update_signature.begin(null);
}
private void on_draft_id_changed() {
draft_id_changed(this.draft_manager.current_draft_id);
}
......
......@@ -7,7 +7,6 @@
public class ComposerWebViewTest : ClientWebViewTestCase<ComposerWebView> {
private const string BODY_TEMPLATE = """<div id="geary-body" dir="auto">%s<div><br></div><div><br></div></div>""";
public ComposerWebViewTest() {
base("ComposerWebViewTest");
......@@ -23,6 +22,7 @@ public class ComposerWebViewTest : ClientWebViewTestCase<ComposerWebView> {
add_test("get_text_with_named_link", get_text_with_named_link);
add_test("get_text_with_url_link", get_text_with_named_link);
add_test("get_text_with_surrounding_nbsps", get_text_with_surrounding_nbsps);
add_test("update_signature", update_signature);
}
public void load_resources() throws Error {
......@@ -45,17 +45,12 @@ public class ComposerWebViewTest : ClientWebViewTestCase<ComposerWebView> {
assert(new ComposerWebView.EditContext("0,,,12").font_size == 12);
}
public void get_html() throws Error {
string html = "<p>para</p>";
load_body_fixture(html);
public void get_html() throws GLib.Error {
string BODY = "<p>para</p>";
load_body_fixture(BODY);
this.test_view.get_html.begin((obj, ret) => { async_complete(ret); });
try {
assert(this.test_view.get_html.end(async_result()) ==
BODY_TEMPLATE.printf(html));
} catch (Error err) {
print("Error: %s\n", err.message);
assert_not_reached();
}
string html = this.test_view.get_html.end(async_result());
assert_string(ComposerPageStateTest.COMPLETE_BODY_TEMPLATE.printf(BODY), html);
}
public void get_text() throws Error {
......@@ -210,12 +205,40 @@ long, long, long, long, long, long, long, long, long, long,
}
}
public void update_signature() throws GLib.Error {
const string BODY = "<p>para</p>";
load_body_fixture(BODY);
string html = "";
const string SIG1 = "signature text 1";
this.test_view.update_signature(SIG1);
this.test_view.get_html.begin((obj, ret) => { async_complete(ret); });
html = this.test_view.get_html.end(async_result());
assert_true(BODY in html, "Body not present");
assert_true(SIG1 in html, "Signature 1 not present");
const string SIG2 = "signature text 2";
this.test_view.update_signature(SIG2);
this.test_view.get_html.begin((obj, ret) => { async_complete(ret); });
html = this.test_view.get_html.end(async_result());
assert_true(BODY in html, "Body not present");
assert_false(SIG1 in html, "Signature 1 still present");
assert_true(SIG2 in html, "Signature 2 not present");
this.test_view.update_signature("");
this.test_view.get_html.begin((obj, ret) => { async_complete(ret); });
html = this.test_view.get_html.end(async_result());
assert_true(BODY in html, "Body not present");
assert_false(SIG1 in html, "Signature 1 still present");
assert_false(SIG2 in html, "Signature 2 still present");
}
protected override ComposerWebView set_up_test_view() {
return new ComposerWebView(this.config);
}
protected override void load_body_fixture(string html = "") {
this.test_view.load_html(html, "", "", false, false);
this.test_view.load_html(html, "", false, false);
while (this.test_view.is_loading) {
Gtk.main_iteration();
}
......
......@@ -7,8 +7,9 @@
class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
private const string COMPLETE_BODY_TEMPLATE = """<div id="geary-body" dir="auto">%s<div><br></div><div><br></div></div>""";
private const string CLEAN_BODY_TEMPLATE = "%s<div><br></div><div><br></div>";
public const string COMPLETE_BODY_TEMPLATE =
"""<div id="geary-body" dir="auto">%s<div><br></div><div><br></div></div><div id="geary-signature" dir="auto"></div>""";
public const string CLEAN_BODY_TEMPLATE = "%s<div><br></div><div><br></div>";
public ComposerPageStateTest() {
base("ComposerPageStateTest");
......@@ -89,7 +90,6 @@ class ComposerPageStateTest : ClientWebViewTestCase<ComposerWebView> {
some text
""",
"-- <br>sig",
"<p>outerquote text</p>",
true
);
......@@ -103,9 +103,6 @@ some text
assert(!WebKitUtil.to_bool(run_javascript(
@"geary.containsAttachmentKeyword(\"innerquote\", \"subject text\");"
)));
assert(!WebKitUtil.to_bool(run_javascript(
@"geary.containsAttachmentKeyword(\"sig\", \"subject text\");"
)));
assert(!WebKitUtil.to_bool(run_javascript(
@"geary.containsAttachmentKeyword(\"outerquote\", \"subject text\");"
)));
......@@ -292,14 +289,13 @@ unknown://example6.com
}
protected override void load_body_fixture(string body = "") {
load_body_fixture_full(body, "", "", true);
load_body_fixture_full(body, "", true);
}
protected void load_body_fixture_full(string body,
string sig,
string quote,
bool top_posting) {
this.test_view.load_html(body, sig, quote, top_posting, false);
this.test_view.load_html(body, quote, top_posting, false);
while (this.test_view.is_loading) {
Gtk.main_iteration();
}
......
......@@ -24,6 +24,10 @@ body.plain a {
cursor: text;
}
body > *.geary-no-display {
display: none !important;
}
body > div#geary-body {
margin: 0 !important;
border: 0 !important;
......
......@@ -133,6 +133,7 @@ ComposerPageState.prototype = {
if (!enabled) {
this.stopBodyObserver();
}
this.bodyPart.contentEditable = true;
if (this.signaturePart != null) {
this.signaturePart.contentEditable = true;
......@@ -140,6 +141,7 @@ ComposerPageState.prototype = {
if (this.quotePart != null) {
this.quotePart.contentEditable = true;
}
if (enabled) {
// Enable modification observation only after the document
// has been set editable as WebKit will alter some attrs
......@@ -208,8 +210,13 @@ ComposerPageState.prototype = {
},
updateSignature: function(signature) {
if (this.signaturePart != null) {
console.log(signature);
this.signaturePart.innerHTML = signature;
if (signature.trim()) {
this.signaturePart.innerHTML = signature;
this.signaturePart.classList.remove("geary-no-display");
} else {
this.signaturePart.innerHTML = "";
this.signaturePart.classList.add("geary-no-display");
}
}
},
deleteQuotedMessage: function() {
......@@ -315,13 +322,17 @@ ComposerPageState.prototype = {
if (this.signaturePart != null) {
parent.appendChild(
ComposerPageState.cleanPart(this.signaturePart.cloneNode(true), false)
ComposerPageState.cleanPart(
this.signaturePart.cloneNode(true), false
)
);
}
if (this.quotePart != null) {
parent.appendChild(
ComposerPageState.cleanPart(this.quotePart.cloneNode(true), false)
ComposerPageState.cleanPart(
this.quotePart.cloneNode(true), false
)
);
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment