GNOME Bugzilla – Bug 791230
Should allow choosing sending address always when account has multiple addresses configured
Last modified: 2018-06-26 15:12:02 UTC
I'm trying to send a mail to the Fedora devel list, but getting rejected. Geary is sending the mail with my personal Posteo address, but I need it to be sent using my @gnome.org alias as that's the address I'm subscribed with. I have that configured in Geary as an "Additional email address" on my Posteo account. There's no easy way for Geary to guess this. If we want to be really smart, it could look at the last (i.e. the first/original) Delivered-To header in the file and use that as a heuristic to choose the right sending address. But really, we should always display From and let the user choose when an account has multiple email addresses enabled. Otherwise, users will always be mad when Geary fails to be omniscient.
Created attachment 365106 [details] [review] Always allow changing from address if account has multiple addresses
Created attachment 365107 [details] [review] Never enter inline compact mode when account has multiple emails If the current account has multiple emails, we must never enter inline compact mode, otherwise the user has no chance to pick the right email. Geary currently tries to guess which email to send the mail from, but it can't always be right.
Hey, thanks for these patches. I think I tend to agree with the rationale, will have a look at them on the weekend.
*** Bug 738902 has been marked as a duplicate of this bug. ***
This also sounds a bit like Bug 778764. Does detaching the composer let you change the email in this case? I'm wondering if a slightly smarter version of this is to look at the being-replied-to recipients, and if at least one matches those configured for the account then use that as the from address and display a compact composer, otherwise do as these patches do and show a full-blown one? We also probably want to allow the user to select a from address if multiple are configured in the inline-but-not-compact case.
(In reply to Michael Gratton from comment #5) > This also sounds a bit like Bug 778764. Does detaching the composer let you > change the email in this case? Um, yes, I never noticed that... of course, it would be nice to not need to remember to detach the composer before sending my mails. ;) > I'm wondering if a slightly smarter version of this is to look at the > being-replied-to recipients, and if at least one matches those configured > for the account then use that as the from address and display a compact > composer, otherwise do as these patches do and show a full-blown one? Nope, that doesn't work for mailing lists. I think Geary already does that to decide which sending address to select... if that worked, I wouldn't be filing this bug. > We also probably want to allow the user to select a from address if multiple > are configured in the inline-but-not-compact case. My first patch is supposed to accomplish that; does it not work for you?
(In reply to Michael Gratton from comment #5) > I'm wondering if a slightly smarter version of this is to look at the > being-replied-to recipients, and if at least one matches those configured > for the account then use that as the from address and display a compact > composer, otherwise do as these patches do and show a full-blown one? Ah yes, sorry, I misunderstood. Yes, that would work!
Related: bug #747893 (set a default From address for an account having multiple email addresses).
Created attachment 366469 [details] [review] tlsclientconnection: Update use-ssl3 documentation The property documentation correctly indicates how this code works nowadays, but the function documentation is obsolete and misleading. Update it. https://bugzilla.gnome.org/show_bug.cgi?id=745637
Created attachment 366470 [details] [review] tlsclientconnection: Deprecate ssl3 property and functions I originally planned to introduce a new property and functions to replace these, with the same behavior but less-confusing names. But that might not be the best approach in the long run. Instead, let's just deprecate them without replacement. TLS 1.2 intolerance is no longer a thing in the wild, and no known GTlsBackend supports TLS 1.3 yet. But you might need to use this property in the future, even though it's deprecated, if your GTlsBackend has added support for TLS 1.3 and you need to talk to a server that is TLS 1.3 intolerant. Independently of all that, these APIs simply no longer do what their names suggest, so deprecation is sensible regardless. https://bugzilla.gnome.org/show_bug.cgi?id=745637
Sorry, wrong bug. This was my second attempt. I just attached these patches to two wrong bugs, twice in a row. Grrr.
Created attachment 366473 [details] [review] Always allow changing from address if account has multiple addresses
Created attachment 366474 [details] [review] Never enter inline compact mode when account has multiple emails If the current account has multiple emails, we must never enter inline compact mode, otherwise the user has no chance to pick the right email. Geary currently tries to guess which email to send the mail from, but it can't always be right.
Created attachment 366475 [details] [review] Improve heuristic to decide whether to show From selection The previous two commits ensured that we do not hide the From selection when an account has multiple addresses, and that we do not enter inline compact mode in this case either (which would also result in the From selection not appearing). But it's actually OK to hide From if Geary is going to be able to guess the right address to use for the reply, which is generally the case in the absence of mailing lists or automatic forwarding. If exactly one of this account's addresses appears in the email headers of the mail being replied to, then that is probably the account the user will want to use to reply. To implement this heuristic, we obviously have to check the To, CC, and BCC fields of the mail being replied to, to see if the mail was directly addressed to one of this account's emails. But we also have to check From, because the user might have sent the last mail in the conversation. I'm not sure where the code that actually picks the address to use lives, but wherever it is, it seems to do a good job whenever this heuristic says to hide the From selection.
(In reply to Michael Gratton from comment #5) > I'm wondering if a slightly smarter version of this is to look at the > being-replied-to recipients, and if at least one matches those configured > for the account then use that as the from address and display a compact > composer, otherwise do as these patches do and show a full-blown one? OK, I was able to implement this. Does it look OK? > We also probably want to allow the user to select a from address if multiple > are configured in the inline-but-not-compact case. That's already handled by my original patches.
Review of attachment 366474 [details] [review]: Looks good, thanks. Pushed to master as commit 77d8516.
Review of attachment 366473 [details] [review]: Looks good, thanks. Pushed to master as commit 77d8516.
(In reply to Michael Gratton from comment #16) > Review of attachment 366474 [details] [review] [review]: > > Looks good, thanks. Pushed to master as commit 77d8516. Actually, the second patch was commit dec4416.
Review of attachment 366475 [details] [review]: This in general looks good, but there's some some stuff I'd like to see tidied up before committing this: ::: src/client/composer/composer-widget.vala @@ +367,3 @@ private bool is_closing = false; + // The mail that is being replied to Nit: This should probably read "replied to or forwarded" @@ +386,2 @@ public ComposerWidget(Geary.Account account, + Geary.Email? referred, This param duplicates the the referred param for the ComposerWidget::load() method, further down in the class. Since these should always refer to the same object, it would be good to remove the dup param from load() and just refer to the instance property in that method. @@ +542,3 @@ } + public ComposerWidget.from_mailto(Geary.Account account, Geary.Email? referred, This ctor only gets used for external and internal mailto URI links, so unless there some way we could work out what the referrnt is in those cases, the referred param should be removed here and just set to null when calling the default ctor.
(In reply to Michael Gratton from comment #19) > This param duplicates the the referred param for the ComposerWidget::load() > method, further down in the class. Since these should always refer to the > same object, it would be good to remove the dup param from load() and just > refer to the instance property in that method. I don't know; that implies removing the call to email_stores.get() at the bottom of GearyController.create_compose_widget_async(), since the retrieved Geary.Email would no longer be needed to be passed to ComposerWidget.load(). I don't know if it's important or not. Is it needed to "update" the email somehow? It's easy to change, I'm just not sure whether it's right or not.
Bump tickets to 0.14 that aren't going to make 0.13.
This is already solved. The last patch never made it in, but I don't have time for it anymore unfortunately, and it's really a separate issue.