[geary] Fix client use of MailboxAddress(es).to_rfc822_string().



commit 7073206c8d03890e69f4c29f26ae7da4df2e979f
Author: Michael James Gratton <mike vee net>
Date:   Thu Feb 1 23:56:23 2018 +1030

    Fix client use of MailboxAddress(es).to_rfc822_string().
    
    Turns out the client was in inexplicably using
    MailboxAddresses.to_rfc822_string() in a bunch of places to display
    emails to users, mostly in the composer. This fixes that.
    
    Followup to b7eea857.
    
    * src/engine/rfc822/rfc822-mailbox-addresses.vala (MailboxAddresses):
      Make list param for default ctor nullable for your testing convenience,
      add a test. Add to_full_display() method that the client should nearly
      always be using to display a list to users, use that in all the places.
      Remove empty param for list_to_string since all but one callers just
      pass in the empty string, which also fixes some stupid regressions
      introduced in commit 178ce351 that causes it actually break
      to_rfc822_string, rather than fix it as claimed, update call sites.

 src/client/composer/composer-widget.vala          |    7 +++-
 src/client/composer/contact-entry-completion.vala |    6 ++--
 src/client/composer/contact-list-store.vala       |    6 ++--
 src/client/composer/email-entry.vala              |    2 +-
 src/engine/imap-db/imap-db-message-row.vala       |   22 +---------------
 src/engine/rfc822/rfc822-mailbox-addresses.vala   |   28 +++++++++++++++-----
 test/engine/rfc822-mailbox-addresses-test.vala    |    9 +++---
 7 files changed, 39 insertions(+), 41 deletions(-)
---
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index 09438ae..e3e38c8 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -2011,7 +2011,7 @@ public class ComposerWidget : Gtk.EventBox {
     private bool add_account_emails_to_from_list(Geary.Account other_account, bool set_active = false) {
         Geary.RFC822.MailboxAddresses primary_address = new Geary.RFC822.MailboxAddresses.single(
             other_account.information.primary_mailbox);
-        this.from_multiple.append_text(primary_address.to_rfc822_string());
+        this.from_multiple.append_text(primary_address.to_full_display());
         this.from_list.add(new FromAddressMap(other_account, primary_address));
         if (!set_active && this.from.equal_to(primary_address)) {
             this.from_multiple.set_active(this.from_list.size - 1);
@@ -2026,7 +2026,10 @@ public class ComposerWidget : Gtk.EventBox {
                 // Displayed in the From dropdown to indicate an "alternate email address"
                 // for an account.  The first printf argument will be the alternate email
                 // address, and the second will be the account's primary email address.
-                string display = _("%1$s via %2$s").printf(addresses.to_rfc822_string(), 
other_account.information.display_name);
+                string display = _("%1$s via %2$s").printf(
+                    addresses.to_full_display(),
+                    other_account.information.display_name
+                );
                 this.from_multiple.append_text(display);
                 this.from_list.add(new FromAddressMap(other_account, addresses));
                 
diff --git a/src/client/composer/contact-entry-completion.vala 
b/src/client/composer/contact-entry-completion.vala
index dafa0d0..f999b7b 100644
--- a/src/client/composer/contact-entry-completion.vala
+++ b/src/client/composer/contact-entry-completion.vala
@@ -63,10 +63,10 @@ public class ContactEntryCompletion : Gtk.EntryCompletion {
         Gtk.CellRendererText text_renderer = (Gtk.CellRendererText) cell;
         text_renderer.markup = highlighted_result;
     }
-    
+
     private bool on_match_selected(Gtk.EntryCompletion sender, Gtk.TreeModel model, Gtk.TreeIter iter) {
-        string full_address = list_store.get_rfc822_string(iter);
-        
+        string full_address = list_store.to_full_address(iter);
+
         Gtk.Entry? entry = sender.get_entry() as Gtk.Entry;
         if (entry == null)
             return false;
diff --git a/src/client/composer/contact-list-store.vala b/src/client/composer/contact-list-store.vala
index 54d945c..821dffd 100644
--- a/src/client/composer/contact-list-store.vala
+++ b/src/client/composer/contact-list-store.vala
@@ -104,9 +104,9 @@ public class ContactListStore : Gtk.ListStore {
         
         return (Geary.Contact) contact_value.get_object();
     }
-    
-    public string get_rfc822_string(Gtk.TreeIter iter) {
-        return get_contact(iter).get_rfc822_address().to_rfc822_string();
+
+    public string to_full_address(Gtk.TreeIter iter) {
+        return get_contact(iter).get_rfc822_address().to_full_display();
     }
 
     private inline void add_contact(Geary.Contact contact) {
diff --git a/src/client/composer/email-entry.vala b/src/client/composer/email-entry.vala
index 7dab162..94b8a5b 100644
--- a/src/client/composer/email-entry.vala
+++ b/src/client/composer/email-entry.vala
@@ -32,7 +32,7 @@ public class EmailEntry : Gtk.Entry {
             
             updating = true;
             modified = true;
-            text = (addresses == null) ? "" : addresses.to_rfc822_string();
+            text = (addresses == null) ? "" : addresses.to_full_display();
             updating = false;
         });
 
diff --git a/src/engine/imap-db/imap-db-message-row.vala b/src/engine/imap-db/imap-db-message-row.vala
index f9c294c..9dcfcae 100644
--- a/src/engine/imap-db/imap-db-message-row.vala
+++ b/src/engine/imap-db/imap-db-message-row.vala
@@ -262,27 +262,7 @@ private class Geary.ImapDB.MessageRow {
     }
 
     private static string? flatten_addresses(RFC822.MailboxAddresses? addrs) {
-        if (addrs == null)
-            return null;
-        
-        switch (addrs.size) {
-            case 0:
-                return null;
-            
-            case 1:
-                return addrs[0].to_rfc822_string();
-            
-            default:
-                StringBuilder builder = new StringBuilder();
-                foreach (RFC822.MailboxAddress addr in addrs) {
-                    if (!String.is_empty(builder.str))
-                        builder.append(", ");
-                    
-                    builder.append(addr.to_rfc822_string());
-                }
-                
-                return builder.str;
-        }
+        return (addrs == null || addrs.size == 0) ? null : addrs.to_rfc822_string();
     }
 
     private RFC822.MailboxAddress? unflatten_address(string? str) {
diff --git a/src/engine/rfc822/rfc822-mailbox-addresses.vala b/src/engine/rfc822/rfc822-mailbox-addresses.vala
index 9d06baf..f94019b 100644
--- a/src/engine/rfc822/rfc822-mailbox-addresses.vala
+++ b/src/engine/rfc822/rfc822-mailbox-addresses.vala
@@ -27,11 +27,10 @@ public class Geary.RFC822.MailboxAddresses :
      * returned.
      */
     private static string list_to_string(Gee.List<MailboxAddress> addrs,
-                                          string empty,
                                           ListToStringDelegate to_s) {
         switch (addrs.size) {
             case 0:
-                return empty;
+                return "";
 
             case 1:
                 return to_s(addrs[0]);
@@ -61,8 +60,10 @@ public class Geary.RFC822.MailboxAddresses :
     private Gee.List<MailboxAddress> addrs = new Gee.ArrayList<MailboxAddress>();
 
 
-    public MailboxAddresses(Gee.Collection<MailboxAddress> addrs) {
-        this.addrs.add_all(addrs);
+    public MailboxAddresses(Gee.Collection<MailboxAddress>? addrs = null) {
+        if (addrs != null) {
+            this.addrs.add_all(addrs);
+        }
     }
 
     public MailboxAddresses.single(MailboxAddress addr) {
@@ -147,6 +148,17 @@ public class Geary.RFC822.MailboxAddresses :
     }
 
     /**
+     * Returns the addresses suitable for display to a human.
+     *
+     * @return a string containing each message in the list,
+     * serialised by a call to {@link Message.to_full_display},
+     * separated by commas.
+     */
+    public string to_full_display() {
+        return list_to_string(addrs, (a) => a.to_full_display());
+    }
+
+    /**
      * Returns the addresses suitable for insertion into an RFC822 message.
      *
      * RFC822 quoting is performed if required.
@@ -154,7 +166,7 @@ public class Geary.RFC822.MailboxAddresses :
      * @see MailboxAddress.to_rfc822_string
      */
     public string to_rfc822_string() {
-        return list_to_string(addrs, ", ", (a) => a.to_rfc822_string());
+        return list_to_string(addrs, (a) => a.to_rfc822_string());
     }
 
     public uint hash() {
@@ -191,11 +203,13 @@ public class Geary.RFC822.MailboxAddresses :
      * See Geary.MessageData.SearchableMessageData.
      */
     public string to_searchable_string() {
-        return list_to_string(addrs, " ", (a) => a.to_searchable_string());
+        return list_to_string(addrs, (a) => a.to_searchable_string());
     }
 
     public override string to_string() {
-        return list_to_string(addrs, "(no addresses)", (a) => a.to_string());
+        return this.size > 0
+            ? list_to_string(addrs, (a) => a.to_string())
+            : "(no addresses)";
     }
 
 }
diff --git a/test/engine/rfc822-mailbox-addresses-test.vala b/test/engine/rfc822-mailbox-addresses-test.vala
index 3c252d8..3d91d6e 100644
--- a/test/engine/rfc822-mailbox-addresses-test.vala
+++ b/test/engine/rfc822-mailbox-addresses-test.vala
@@ -30,10 +30,11 @@ class Geary.RFC822.MailboxAddressesTest : Gee.TestCase {
     }
 
     public void to_rfc822_string() {
-        assert(new_addreses({ "test1 example com" }).to_rfc822_string() ==
-               "test1 example com");
-        assert(new_addreses({ "test1 example com", "test2 example com" }).to_rfc822_string() ==
-               "test1 example com, test2 example com");
+        assert(new MailboxAddresses().to_rfc822_string() == "");
+        assert(new_addreses({ "test1 example com" })
+               .to_rfc822_string() == "test1 example com");
+        assert(new_addreses({ "test1 example com", "test2 example com" })
+               .to_rfc822_string() == "test1 example com, test2 example com");
     }
 
     private MailboxAddresses new_addreses(string[] address_strings) {


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]