GNOME Bugzilla – Bug 714956
Sender with Last Name, First causes parse problem in composer
Last modified: 2017-10-24 10:46:29 UTC
---- Reported by jim@yorba.org 2012-09-28 15:04:00 -0700 ---- Original Redmine bug id: 5904 Original URL: http://redmine.yorba.org/issues/5904 Searchable id: yorba-bug-5904 Original author: Jim Nelson Original description: In the composer, the Send button will only be activated if a fully-formed email address is typed into one of the sender fields. However, if I type an email that is formed liked this: Nelson, Jim <jim@yorba.org> The Send button will remain insensitive. Note that I picked this name from the autocompletion drop-down. Related issues: related to geary - 5047: Problems validating email addresses in composer (Open) ---- Additional Comments From geary-maint@gnome.bugs 2013-10-03 17:53:00 -0700 ---- ### History #### #1 Updated by Tiago Quelhas 11 months ago * **File** geary-fix-autocompletion-in-last-first-format.patch added Hi, Fix is simple: autocompleted addresses should be stringified by to_rfc822_string(), which quotes the display name when it contains special characters (in this case a comma), as per RFC 2822 Section 3.4. Patch attached. I don't think this is much of a problem if the string is typed in by the user; most people will either use autocomplete or just type in an address without a display name. However, if handling that case is desirable, it should be simple to determine heuristically whether a comma belongs in a name or separates addresses. #### #2 Updated by Jim Nelson 11 months ago * **Target version** set to _0.3.0_ We'll definitely take this patch but I'm going to leave open this ticket because we do want to get the manually-entered case. My concern here is not the user literally typing out such a name but rather pasting it in from another app or Web page. It does seem like a more sophisticated algorithm will be needed. Still, your patch solves in my mind a worse problem, where Geary has autocompleted an address but won't let the user send to it. Thanks! #### #3 Updated by Jim Nelson 11 months ago Partial fix committed in 6b4b56f05aa16ec8fe759e00f67f0f80daba4657 #### #4 Updated by Tiago Quelhas 10 months ago Thanks, Jim! Regarding copy-pasted addresses with commas, I think we can get away with the following heuristic: consider a comma to be an address separator only if it is preceded by something that looks like an address, e.g., something@somewhere.com or <something@somewhere.com>. Otherwise, assume the comma belongs to a display name. We'd have to split the To/Cc/Bcc fields into individual addresses manually, as I don't believe gmime is able to handle such malformed address lists; but we may still be able to feed it with each individual address, to avoid having to implement the whole parser. I'll look into this next weekend, and perhaps cook up a tentative patch. #### #5 Updated by Jim Nelson 10 months ago That all sounds reasonable, Tiago. Let us know if you have any other questions! #### #6 Updated by Jim Nelson 9 months ago * **Target version** changed from _0.3.0_ to _0.4.0_ #### #7 Updated by Jim Nelson 8 months ago * **Target version** changed from _0.4.0_ to _0.5.0_ #### #8 Updated by Adam Dingle 3 months ago * **Target version** changed from _0.5.0_ to _0.4.0_ Unfortunately this does not currently seem to be working in the autocompletion case: if I compose a message and use autocomplete to select an email address with a "last, first" name (without quotes around the name), the Send button remains disabled. I think we should solve this for 0.4 if possible. #### #9 Updated by Jim Nelson about 1 month ago * **Assignee** set to _Charles Lindsay_ #### #10 Updated by Charles Lindsay about 1 month ago * **Assignee** deleted (<strike>_Charles Lindsay_</strike>) This was assigned to me because I was optimistic this was part of some code I was already working on, but it turns out it isn't. From looking at this a little bit, it looks like we're already passing the addresses off to RFC822.MailboxAddresses to parse (I'm looking at email- entry.vala line 29, where we're determining whether the To address for the composer window is valid). Like Tiego says, we may need some more intelligent heuristic inside MailboxAddresses to "fudge" it a little bit, and treat strings like `last, first <email@example.com>` (which I believe would parse to one invalid address and one valid recipient in an RFC-compliant parser) as if they were instead `"last, first" <mail@example.com>` (which would parse to one valid recipient). #### #11 Updated by Charles Lindsay about 1 month ago * **Target version** changed from _0.4.0_ to _0.5.0_ --- Bug imported by chaz@yorba.org 2013-11-21 23:13 UTC --- This bug was previously known as _bug_ 5904 at http://redmine.yorba.org/show_bug.cgi?id=5904 Imported an attachment (id=261194) Unknown milestone "unknown in product geary. Setting to default milestone for this product, "---". Setting qa contact to the default for this product. This bug either had no qa contact or an invalid one. Resolution set on an open status. Dropping resolution
*** Bug 728346 has been marked as a duplicate of this bug. ***
*** Bug 730458 has been marked as a duplicate of this bug. ***
The partial fix alluded to above (6b4b56f0) switched from using RFC822.MailboxAddress.get_full_address() to .to_rfc822_string(). This latter method calls GMime.utils_quote_string() on the name, which will add quotes if necessary. However, this was undone by 502bb0d2, which did a bit of rearranging of the code. This explains the regression noted in #8. The simple fix for this would be to reimplement the change of 6b4b56f0. But I wonder if we should look at those two methods of RFC822.MailboxAddress again? Do we need both? Is there a time we need to give an unquoted version of the name? If so, we may want to go through and check that we're calling the right method each time. None of this solves the problem of the user typing in "Last, First <address>", but who does that?
(In reply to comment #3) > But I wonder if we should look at > those two methods of RFC822.MailboxAddress again? Do we need both? Is there a > time we need to give an unquoted version of the name? If so, we may want to go > through and check that we're calling the right method each time. Some times when displaying the address we didn't want quotes because they add clutter to the UI. to_rfc822_string() wasn't designed for user-facing strings but for (obviously) generating an RFC822 message. Because the composer's To:, Cc:, Bcc: lines are WYSIWYG (in the sense that those strings are pretty much used verbatim in the outgoing email), we need to use to_rfc822_string() there. But those widgets don't necessarily need to be WYSIWYG, which I think is the key to fixing this issue. > None of this solves the problem of the user typing in "Last, First <address>", > but who does that? Someone might copy-and-paste such a string into the address bar. Also, autocomplete will dump such a string into it. (I can reproduce this right now). Know that some clients send such a broken string as the From: line; I correspond regularly with a person whose client does just this. It gets harvested by Geary and now Geary's autocomplete will generate an invalid email address when I type their name. The fundamental problem I would like solved is that, to the user, all looks correct with [Last, First <address@host>] but the Send button remains insensitive. This causes real confusion and frustration. You need to know RFC822's rules in order to manually correct the problem. We can patch things here and there and hope the user doesn't encounter the situation, or we can fix it outright. The three possibilities I see: (a) The GtkEntry fixes-up input as it comes in to ensure that the above pattern is always quoted. This isn't as easy as it sounds and could cause other confusion as the user types or pastes something and gets something else. (b) Fix the autocomplete bug I mentioned. In all likelihood this corrects the 95% problem, but still allows the user to paste in (or, unlikely enough, type in) bogus addresses. (c) The validity parser is tweaked to allow the errant pattern and when the GtkEntry lines are parsed the quotes are added automatically. I like (c) but know that's not the easiest. Fixing (b) is the best bang for the buck -- and I would certainly take such a patch! -- but I wouldn't close this ticket quite yet with just that.
I went ahead and made the fix. Pushed to master, commit 1398cc This patch forces autocomplete to use quoted RFC822 strings when inserting an address into the entry bar, meaning that autocomplete won't generate an unusable address for the user. I believe this fixes the 95% case, mentioned above. It doesn't fully solve this problem though, so I'm leaving this ticket open. This fix will land in Geary 0.8. If we do a Geary 0.6.2 (I would need a more urgent bug to do so) I'll migrate this patch in as well, as it's low-risk and fairly annoying when encountered. I also did a code check for to_rfc822_string() vs. get_full_address() usage. It looks like the autocomplete code was the only place it was being misused. <Knocks on wood>
This seems to be working fine, so I'm resolving this as fixed. Please re-open if the problem still exists.