diff --git a/src/client/conversation-viewer/conversation-email.vala b/src/client/conversation-viewer/conversation-email.vala index 3a94b2f1..ea57e20e 100644 --- a/src/client/conversation-viewer/conversation-email.vala +++ b/src/client/conversation-viewer/conversation-email.vala @@ -289,6 +289,9 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { /** Determines if the email is a draft message. */ public bool is_draft { get; private set; } + /** The email's primary originator, if any. */ + public Geary.RFC822.MailboxAddress? primary_originator { get; private set; } + /** The view displaying the email's primary message headers and body. */ public ConversationMessage primary_message { get; private set; } @@ -444,6 +447,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { base_ref(); this.email = email; this.is_draft = is_draft; + this.primary_originator = Util.Email.get_primary_originator(email); this.email_store = email_store; this.contact_store = email_store.account.get_contact_store(); this.avatar_store = avatar_store; @@ -507,9 +511,11 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { // Construct the view for the primary message, hook into it bool load_images = email.load_remote_images().is_certain(); - Geary.Contact contact = this.contact_store.get_by_rfc822( - email.get_primary_originator() - ); + + Geary.Contact? contact = null; + if (this.primary_originator != null) { + contact = this.contact_store.get_by_rfc822(this.primary_originator); + } if (contact != null) { load_images |= contact.always_load_remote_images(); } @@ -577,7 +583,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { } catch (IOError.CANCELLED err) { // okay } catch (Error err) { - Geary.RFC822.MailboxAddress? from = this.email.get_primary_originator(); + Geary.RFC822.MailboxAddress? from = this.primary_originator; debug("Avatar load failed for \"%s\": %s", from != null ? from.to_string() : "", err.message); } @@ -1038,7 +1044,7 @@ public class ConversationEmail : Gtk.Box, Geary.BaseInterface { private void on_remember_remote_images(ConversationMessage view) { - Geary.RFC822.MailboxAddress? sender = this.email.get_primary_originator(); + Geary.RFC822.MailboxAddress? sender = this.primary_originator; if (sender != null) { Geary.Contact? contact = this.contact_store.get_by_rfc822(sender); if (contact != null) { diff --git a/src/client/conversation-viewer/conversation-message.vala b/src/client/conversation-viewer/conversation-message.vala index 17614426..e8cd4f27 100644 --- a/src/client/conversation-viewer/conversation-message.vala +++ b/src/client/conversation-viewer/conversation-message.vala @@ -289,7 +289,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { bool load_remote_images, Configuration config) { this( - email.get_primary_originator(), + Util.Email.get_primary_originator(email), email.from, email.reply_to, email.sender, @@ -315,7 +315,7 @@ public class ConversationMessage : Gtk.Grid, Geary.BaseInterface { bool load_remote_images, Configuration config) { this( - message.get_primary_originator(), + Util.Email.get_primary_originator(message), message.from, message.reply_to, message.sender, diff --git a/src/client/notification/libnotify.vala b/src/client/notification/libnotify.vala index d82a5a9c..11bf793f 100644 --- a/src/client/notification/libnotify.vala +++ b/src/client/notification/libnotify.vala @@ -95,7 +95,8 @@ public class Libnotify : Geary.BaseObject { return; // possible to receive email with no originator - Geary.RFC822.MailboxAddress? primary = email.get_primary_originator(); + Geary.RFC822.MailboxAddress? primary = + Util.Email.get_primary_originator(email); if (primary == null) { notify_new_mail(folder, 1); diff --git a/src/client/util/util-email.vala b/src/client/util/util-email.vala index 05a82828..55bda5a1 100644 --- a/src/client/util/util-email.vala +++ b/src/client/util/util-email.vala @@ -38,6 +38,65 @@ namespace Util.Email { return !Geary.String.is_empty(cleaned) ? cleaned : _("(no subject)"); } + /** + * Returns a mailbox for the primary originator of an email. + * + * RFC 822 allows multiple and absent From header values, and + * software such as Mailman and GitLab will mangle the names in + * From mailboxes. This provides a canonical means to obtain a + * mailbox (that is, name and email address) for the first + * originator, and with the mailbox's name having been fixed up + * where possible. + * + * The first From mailbox is used and de-mangled if found, if not + * the Sender mailbox is used if present, else the first Reply-To + * mailbox is used. + */ + public Geary.RFC822.MailboxAddress? + get_primary_originator(Geary.EmailHeaderSet email) { + Geary.RFC822.MailboxAddress? primary = null; + if (email.from != null && email.from.size > 0) { + // We have a From address, so attempt to de-mangle it + Geary.RFC822.MailboxAddresses? from = email.from; + + string from_name = ""; + if (from != null && from.size > 0) { + primary = from[0]; + from_name = primary.name ?? ""; + } + + Geary.RFC822.MailboxAddresses? reply_to = email.reply_to; + Geary.RFC822.MailboxAddress? primary_reply_to = null; + string reply_to_name = ""; + if (reply_to != null && reply_to.size > 0) { + primary_reply_to = reply_to[0]; + reply_to_name = primary_reply_to.name ?? ""; + } + + // Spaces are important + const string VIA = " via "; + + if (reply_to_name != "" && from_name.has_prefix(reply_to_name)) { + // Mailman sometimes sends the true originator as the + // Reply-To for the email + primary = primary_reply_to; + } else if (VIA in from_name) { + // Mailman, GitLib, Discourse and others send the + // originator's name prefixing something starting with + // "via". + primary = new Geary.RFC822.MailboxAddress( + from_name.split(VIA, 2)[0], primary.address + ); + } + } else if (email.sender != null) { + primary = email.sender; + } else if (email.reply_to != null && email.reply_to.size > 0) { + primary = email.reply_to[0]; + } + + return primary; + } + /** * Returns a shortened recipient list suitable for display. * diff --git a/src/engine/api/geary-email.vala b/src/engine/api/geary-email.vala index e22d5762..2b4d7ec4 100644 --- a/src/engine/api/geary-email.vala +++ b/src/engine/api/geary-email.vala @@ -530,25 +530,6 @@ public class Geary.Email : BaseObject, EmailHeaderSet { return (preview != null) ? preview.buffer.to_string() : ""; } - /** - * Returns the primary originator of an email, which is defined as the first mailbox address - * in From:, Sender:, or Reply-To:, in that order, depending on availability. - * - * Returns null if no originators are present. - */ - public RFC822.MailboxAddress? get_primary_originator() { - if (from != null && from.size > 0) - return from[0]; - - if (sender != null) - return sender; - - if (reply_to != null && reply_to.size > 0) - return reply_to[0]; - - return null; - } - public string to_string() { return "[%s] ".printf(id.to_string()); } diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala index 9bbfb5a0..5f70b854 100644 --- a/src/engine/rfc822/rfc822-message.vala +++ b/src/engine/rfc822/rfc822-message.vala @@ -441,25 +441,6 @@ public class Geary.RFC822.Message : BaseObject, EmailHeaderSet { : ""; } - /** - * Returns the primary originator of an email, which is defined as the first mailbox address - * in From:, Sender:, or Reply-To:, in that order, depending on availability. - * - * Returns null if no originators are present. - */ - public RFC822.MailboxAddress? get_primary_originator() { - if (from != null && from.size > 0) - return from[0]; - - if (sender != null) - return sender; - - if (reply_to != null && reply_to.size > 0) - return reply_to[0]; - - return null; - } - public Gee.List? get_recipients() { Gee.List addrs = new Gee.ArrayList(); diff --git a/test/client/util/util-email-test.vala b/test/client/util/util-email-test.vala new file mode 100644 index 00000000..3630276c --- /dev/null +++ b/test/client/util/util-email-test.vala @@ -0,0 +1,115 @@ +/* + * Copyright 2019 Michael Gratton + * + * This software is licensed under the GNU Lesser General Public License + * (version 2.1 or later). See the COPYING file in this distribution. + */ + +public class Util.Email.Test : TestCase { + + public Test() { + base("UtilEmailTest"); + add_test("null_originator", null_originator); + add_test("from_originator", from_originator); + add_test("sender_originator", sender_originator); + add_test("reply_to_originator", reply_to_originator); + add_test("reply_to_via_originator", reply_to_via_originator); + add_test("plain_via_originator", plain_via_originator); + } + + public void null_originator() throws GLib.Error { + Geary.RFC822.MailboxAddress? originator = get_primary_originator( + new_email(null, null, null) + ); + + assert_null(originator); + } + + public void from_originator() throws GLib.Error { + Geary.RFC822.MailboxAddress? originator = get_primary_originator( + new_email( + new Geary.RFC822.MailboxAddress("from", "from@example.com"), + new Geary.RFC822.MailboxAddress("sender", "sender@example.com"), + new Geary.RFC822.MailboxAddress("reply-to", "reply-to@example.com") + ) + ); + + assert_non_null(originator); + assert_string("from", originator.name); + assert_string("from@example.com", originator.address); + } + + public void sender_originator() throws GLib.Error { + Geary.RFC822.MailboxAddress? originator = get_primary_originator( + new_email( + null, + new Geary.RFC822.MailboxAddress("sender", "sender@example.com"), + new Geary.RFC822.MailboxAddress("reply-to", "reply-to@example.com") + ) + ); + + assert_non_null(originator); + assert_string("sender", originator.name); + assert_string("sender@example.com", originator.address); + } + + public void reply_to_originator() throws GLib.Error { + Geary.RFC822.MailboxAddress? originator = get_primary_originator( + new_email( + null, + null, + new Geary.RFC822.MailboxAddress("reply-to", "reply-to@example.com") + ) + ); + + assert_non_null(originator); + assert_string("reply-to", originator.name); + assert_string("reply-to@example.com", originator.address); + } + + public void reply_to_via_originator() throws GLib.Error { + Geary.RFC822.MailboxAddress? originator = get_primary_originator( + new_email( + new Geary.RFC822.MailboxAddress("test via bot", "bot@example.com"), + null, + new Geary.RFC822.MailboxAddress("test", "test@example.com") + ) + ); + + assert_non_null(originator); + assert_string("test", originator.name); + assert_string("test@example.com", originator.address); + } + + public void plain_via_originator() throws GLib.Error { + Geary.RFC822.MailboxAddress? originator = get_primary_originator( + new_email( + new Geary.RFC822.MailboxAddress("test via bot", "bot@example.com"), + null, + null + ) + ); + + assert_non_null(originator); + assert_string("test", originator.name); + assert_string("bot@example.com", originator.address); + } + + private Geary.Email new_email(Geary.RFC822.MailboxAddress? from, + Geary.RFC822.MailboxAddress? sender, + Geary.RFC822.MailboxAddress? reply_to) + throws GLib.Error { + Geary.Email email = new Geary.Email(new Geary.MockEmailIdentifer(1)); + email.set_originators( + from != null + ? new Geary.RFC822.MailboxAddresses(Geary.Collection.single(from)) + : null, + sender, + reply_to != null + ? new Geary.RFC822.MailboxAddresses(Geary.Collection.single(reply_to)) + : null + ); + return email; + } + +} diff --git a/test/meson.build b/test/meson.build index 6054796a..ab3ea78c 100644 --- a/test/meson.build +++ b/test/meson.build @@ -68,6 +68,7 @@ geary_test_client_sources = [ # and the engine test sute needs to depend # geary-engine_internal.vapi, which leads to duplicate symbols when # linking + 'engine/api/geary-email-identifier-mock.vala', 'engine/api/geary-credentials-mediator-mock.vala', 'client/accounts/accounts-manager-test.vala', @@ -76,6 +77,7 @@ geary_test_client_sources = [ 'client/components/client-web-view-test-case.vala', 'client/composer/composer-web-view-test.vala', 'client/util/util-avatar-test.vala', + 'client/util/util-email-test.vala', 'js/client-page-state-test.vala', 'js/composer-page-state-test.vala', diff --git a/test/test-client.vala b/test/test-client.vala index 3386d183..1852ba6e 100644 --- a/test/test-client.vala +++ b/test/test-client.vala @@ -44,6 +44,7 @@ int main(string[] args) { client.add_suite(new ComposerWebViewTest().get_suite()); client.add_suite(new ConfigurationTest().get_suite()); client.add_suite(new Util.Avatar.Test().get_suite()); + client.add_suite(new Util.Email.Test().get_suite()); TestSuite js = new TestSuite("js");