[D7] Use site name in From: header for system e-mails

If we're considering backporting this to D7, we should also directly backport #2936032: Sites named with special characters cannot send mail. Otherwise we'll be introducing possible bugs for some websites (which we don't want since D7 should be stable).
We should check whether similar bugs of this patch were reported and backport the fixes.

I'm working on backporting #2936032: Sites named with special characters cannot send mail.

I've backported #2936032: Sites named with special characters cannot send mail.
Please, review.

Reviewed.
"Sites named with special characters cannot send mail" - solved
Observation:
I assume that email should be replaced by site name.
Is the current behaviour expected?

Nikolas, it seems like it's Gmail's special behavior with long (or weird) from texts.

I've tried to change a site's name to a shorter one: 'diymus, LTd (x-fac)' and now it's showing at GMAIL UI.

I've also tried to send an email to yopmail with a longer name "Mycompanyname ()<>[]:;@\\,.\"" and it was ok.

Screenshots are attached.

Ok.
Let's summarize:
* Site name used instead of email: Done
* Site name encapsulate special characters: Done

Great work!

Thanks for working on this.

How do we address the concerns expressed by @David_Rothstein / @xjm in the parent issue, but specifically about introducing this to D7 now:

we'd basically be changing it for every Drupal 7 site (at least ones that don't specifically override it) in the middle of a stable release, and I think some sites might be quite happy with just the email address being used here.

This might be an unexpected change for some sites that'd break things like e-mail filters / forwarding set-ups that might have been in place for years.

Nice catch, @mcdruid!

I've modified the patch so that default behavior isn't changed.
Now to enable showing a site name in "From" header one needs to set 'site_mail_display_sitename' variable to true.
For example, by executing:

variable_set('site_mail_display_sitename', TRUE); 

or changing settings.php

I've also added tests that cover both cases.

Please, review

The patch does what expected, and it adds tests. To me, this is RTBC.

Tested. The results in attached files

Looks like this was an unrelated testbot snafu; a retest came back green.

I'd like to get this in (we could just about squeeze it into the next release due 2012-12-02 so in one week).

However, I have one or two fairly small tweaks I'd like to suggest:

1) The new drupal_format_mail_name() function is a fairly straight backport of formatDisplayName(). Let's also make the function name match more closely (e.g. drupal_mail_format_display_name() even though that's getting fairly long).

2) Thank you for adding the variable in #23 - however, let's add an entry to default.settings.php for this which explains what it does, and sets it to TRUE for new sites (although we could ask whether those are actually "a thing" for D7 ;).

I think maybe the variable should be called mail_display_name_site_name although I'm not sure there's a "right answer" for that.

It would also be good to add a Change Record / Notice for this pointing out that existing sites might want to enable this feature by setting the variable e.g. in settings.php

Leaving at "Needs work" for those changes.

I'll also add this to #3179845: [meta] Priorities for 2020-12-02 bugfix release of Drupal 7.76 / 7.77 though as I think we can get it into the next release if we work quickly.

Implemented suggestions from #27

Oops. This time without the typo.

My mistake; didn't rename the variable in the new tests.

Oh, interesting - I hadn't spotted the short array syntax in the calls to str_replace().

I've also fixed a typo in a comment a few lines below those - out of scope but annoying and not really worth a new issue.

Back to RTBC

RTBM, +1 from me for merge

I like that it is opt-in and that new sites opt-in automatically.

Should add some release notes and a CR so people know that they can easily enable it now.

Thank you everybody!