Problem/Motivation
The aim of this issue is to give e-mails from the site administrator a "from" title set to the website name and not just an e-mail address.
In Drupal 5 this was possible by configuring "Site Webmaster" <webmaster@site.com> in Site information, but since 6.x the additional validation prevents such an e-mail address configuration.
You are now limited to just the e-mail address, without a name. Hence site e-mails appear to be from just webmaster@site.com, which is just not as nice :)
Proposed resolution
Specify in the "From:" header the site name along with the site email address.
Remaining tasks
- Get RTBC.
- Commit the backported patch at #209672-120: Use site name in From: header for system e-mails to 7.x.
- Create patch and reroll for Drupal 6 if desired.
User interface changes
In your email, you'll see the Site name instead of the email address.
Screen capture attached of my email client:
API changes
None.
Original report by dshaw
In Drupal 5.x, I configure the site e-mail address to be something like "Site Webmaster" <webmaster@site.com> (at the site info page: example.com/admin/settings/site-information). The effect of this is that e-mails from the site have a nicely formatted "From" field. That is, they appear to be from "Site Webmaster", rather than webmaster@site.com. Which is a bit nicer and cleaner.
In Drupal 6.x, the additional validation prevents such an e-mail address configuration. You are now limited to just the e-mail address, without a name. Hence site e-mails appear to be from just webmaster@site.com, which is just not as nice :)
Very minor style issue, I know. But is it possible to retain nice addresses, while still validating the e-mail address?
(The field validation could be altered. Or a site name field could be added to site info. Mail could then be sent from "site name" )
Comment | File | Size | Author |
---|---|---|---|
#146 | 209672-mail-from-146.patch | 3.88 KB | jcisio |
#143 | site_name_in_emails-209672-143.patch | 621 bytes | Andrew Answer |
#127 | d7-site-name-in-emails-209672-128-testsonly.patch | 1.47 KB | naxoc |
#127 | d7-site-name-in-emails-209672-128-tests+fix.patch | 2.24 KB | naxoc |
#123 | 209672.png | 16.33 KB | penyaskito |
Comments
Comment #1
Anonymous (not verified) CreditAttribution: Anonymous commentedD6 uses the new function system_site_information_settings_validate, and specifically user_validate_mail to check the site_mail field, which in turns uses common.inc's valid_email function. So, there are three different ways of solving this problem, without adding a new field (and as such having to change even more code for mailing).
1) Modifiy system_site_information_settings_validate in system.admin.inc to execute a regex and split the string based on finding , and only validate the second portion which is the actual e-mail address.
2) Modify user_validate_mail to do the same stripping as above.
3) Modify the core valid_email function (not recommended, as this could have many ramifications).
Even though this was a "feature" in D5, I don't believe that it's a bug just because it's not in D6.
I'm changing the component to point the appropriate place and will leave it as a bug report until discussion can take place on whether this is a bug or a request.
Comment #2
dshaw CreditAttribution: dshaw commentedThanks for the follow-up. I'd lean towards option 2 (or maybe 1) as a good way to go.
Your point about whether this a bug is a fair one. My initial reaction to D6 was, "hey, I can't do what I could in D5", hence I chose bug. But you're right it was never an explicit feature, so this could become a feature request. Of course that would mean it wouldn't be fixed until D7, wouldn't it? :(
Comment #3
PanchoThis is not just a "nice to have" feature but a serious bug, as emails w/o sender name are often penalized by raising the SPAM index value, AFAIK.
To make this implicit D5-feature explicit, I'd prefer adding a separate name field to the form. We'll have to add or change one or two strings anyway. The fact that adding a string is the lesser crime during string freeze, would be an additional argument for a separate field.
Some response would be great before any work is done on this.
Comment #4
ruharen CreditAttribution: ruharen commentedIMO the best we can do is to add another field for sender's name. email is not sender, so no reason to have it on the same field.
Comment #5
theborg CreditAttribution: theborg commentedShold we follow RFC 2822 and match mailbox definition?:
http://www.faqs.org/rfcs/rfc2822.html
Note that a mailbox is composed of
mailbox = name-addr / addr-spec
Comment #6
cburschka+1 for using mailbox spec (with optional displayname angle-addr) rather than forcing addr-spec. However, such a change would have to be accompanied by a careful check that no other part of core relies on the addr-spec assumption.
If some code took it on itself to construct
"Name" <$email>
anywhere, allowing$email
to already be a mailbox specification would break it.Comment #7
theborg CreditAttribution: theborg commentedAgree with Arancaytar,
The document defines an address as:
So the sender field can be : phrase "<" addr-spec ">".
I vote for modifing the core function.
Comment #8
ScoutBaker CreditAttribution: ScoutBaker commented+1 for following the spec.
Comment #9
theborg CreditAttribution: theborg commentedThis patch modifies the core function valid_email_address to adapt it to the spec. The optional new format is:
"display name" <user@domain>
, the old format, without the first part, is still valid.
Extensive testing is needed.
Comment #10
theborg CreditAttribution: theborg commentedSame as #9 but using & l t ; and & g t ; instead of angle chars.
Comment #11
cburschkaWhy do you need to use entities in this validation - has the input already been through check_plain?
Comment #12
theborg CreditAttribution: theborg commented@Arancaytar: Entities are not needed, but I think that for #9 to work we need to modify drupal_send_mail function
Comment #13
cburschkaOh, so you actually have to *enter*
"Name" <mail@mail>
. Well... inconvenient, but I suppose that's better than nothing. Perhaps this could be improved in a later patch.Comment #14
cburschkaAngle addresses break badly.
Address entered:
"Arancaytar Ilyaran" <arancaytar.ilyaran>
Mail headers:
I'll try to get debug output too, in a bit.
Comment #15
cburschkaThis is the exact content that gets passed to the PHP mail() function as its fourth (mime-headers) parameter:
Somewhere along the line, it ends up as above. My guess is that the entities aren't working out...
Comment #16
cburschkaI have applied patch #9 instead of #10. Works.
Site mail entered:
"Arancaytar Ilyaran" <arancaytar.ilyaran@gmail.com>
Mail headers:
#9 (non-escaped angle characters) works fine for my test, without any other modification. #10 does not.
Unless there is some other issue, I guess #9 is the one we need to use.
I am shying away from RTBCing though, because this has to be tested on another PHP setup, and with other components than the site address. For further testing, these are the references to valid_email_address() and drupal_mail() and drupal_mail_send() in core.
Comment #17
theborg CreditAttribution: theborg commented@Arancaytar: Thanks!
I've tried to break it on all forms that a valid email is involved but I couldn't and the regex added is optional.
The angle address works ok for me, so I'm attaching #9 again with a comment about the two valid formats.
Comment #18
cburschkaOn reading that regex again...
Won't the following two be "valid" email addresses?
I may have a bit of a gap in my understanding of substring expressions, however. Some of those question marks look odd...
Comment #19
theborg CreditAttribution: theborg commentedNo, this addresses are not valid, the trick here is the code:
$angle = '(?(1)>)';
Is a conditional regex that *only* is validated when the (1) regex has been tested and is TRUE.
The (1) regex is the display name part including the white space and the first angle:
$name = '((?:"[^\"\f\n\r\t\v]+" <))?';
Here, we specifically look for quotes surrounding a phrase (one or more words, no control characters), but this is all optional so it can be FALSE, when not valid or nothing. Then $angle only is active whe this is TRUE.
Comment #20
cburschkaThanks for teaching me something new about regex! I think I remember dozens of places in my own code where I can clean up a regular expression or cut down from two to a single one using this particular trick.
In any case, I haven't found any errors on applying this patch. Perhaps another reviewer should test it, but as far as I'm concerned, it's ready.
Comment #21
Gábor HojtsyLooks like this is a feature addition (in email validation) and also an API break since those expecting that their validation is done on an email *address* will get names sneaking in where they don't want it (like on the user account email field).
Comment #22
cburschkaHowever, the "feature" it adds was in D5 and removed by regression. As the site email was allowed to contain a name in D5, any expectation of site email being a pure address would have been broken in D5, and will be broken in D6 for upgraded sites that retain their newly invalidated site email field. So at least this field can contain a name without adding any additional API problems. I'll readily admit that the other references to valid_email_address assume absence of names.
Can we at least make the mailbox spec optional and default it to false?
Comment #23
theborg CreditAttribution: theborg commentedOk, so this should be divided:
a) A feature request to include the validation on the core function: http://drupal.org/node/214114.
b) A patch to modify 'system_site_information_settings' function because this is the part affected by the issue.
This patch looks if there is a display name inside the mail parameter and extracts the address part to pass it to validation.
Comment #24
Gábor HojtsyHm, $allow_name sounds like reasonable for now.
Comment #25
cburschka$allow_name implemented.
As site-info uses user_validate_email for generating the appropriate form errors, user_validate_email takes the $allow_name = FALSE argument and passes it on to valid_email_address.
NOTE - this patch is racing against another one that fixes the "trailing period in hostname" bug. Both patches change the same lines of common.inc, and the losing patch will require a (mostly trivial) conflict-resolving re-roll.
Comment #26
cburschkaStatus update.
Comment #27
dshaw CreditAttribution: dshaw commentedI've tested this patch and it appears to work. I can now enter e-mail addresses with a sender name, a-la Drupal 5.x.
I also tried several almost-correct addresses with sender names and they were all flagged as errors. So I think what's there is good to go.
I think the use of extra variables in the functions and the defaults chosen should prevent any breaks to existing code.
My only concern is whether there are more places where support for a sender name could be enabled. I don't feel I can comment on the entire code base. Perhaps someone else ...
Comment #28
RobLoachI did a file search for "valid_email_address", and I believe there are a couple places where it is used:
I tested it in all these locations and they seem to be working nicely. You could use the sender's name in Drupal 5, so renaming this issue to a regression issue.
This attached patch adds code documentation for the new argument passed to the
valid_email_address
function.Comment #29
cburschkaPossible follow-up issue: The installer "recommends" using the same address for site contact and the root user. Javascript code copies the form value from one email field to the other automatically.
With this patch, the site contact address will allow values that a user address does not. It is usually a reasonable expectation that the suggested defaults actually pass validation.
However, that shouldn't be a show-stopper, because it's pretty clear to the user what is happening, and if they know what they're doing when they add a name to the first field, they should also know that the second field may not have a name.
Comment #30
Gábor HojtsyWhile I understand that this is a regression, I don't think that API change is a good idea now. Please revisit the issue in Drupal 7. For Drupal 6, the form and the mail is equally alterable, so there are multiple places to solve this. This regression appeared at all, because there was no email validation in Drupal 5 there.
Comment #31
cburschkaHm. Okay. Of course, if we had known this two weeks ago, we could have spent more time on critical issues instead of implementing the $allow_name argument to get this into 6.x... ;)
Comment #32
RobLoachSo, for DRUPAL-6, we remove the validation on the site information settings page, and for HEAD, we add $allow_name to
valid_email_address
. Does that sound reasonable?Comment #33
Gábor HojtsyIn D6, I'd rather use the site name as name and site email as email in the outgoing mail, if you insist on having a name by default. Checking the format of an email input field is IMHO an improvement not a bad thing to have.
Comment #34
cburschkaLike this?
This patch is for 6.x.
Comment #35
cburschkaNote: I don't know what characters the site name allows, and if it potentially needs to be check_plained or otherwise filtered before conforming to RFC 2822.
Comment #36
babbage CreditAttribution: babbage commentedSubscribing.
Comment #37
JamieR CreditAttribution: JamieR commentedAnyone know what happened here? Thanks!
Comment #38
Anonymous (not verified) CreditAttribution: Anonymous commented#34 patch works great, thanks. This should definitely be included in core IMHO.
Comment #39
mfbSubscribing.
Comment #40
GregoryHeller CreditAttribution: GregoryHeller commentedSo a i reading this right? you need to patch core in order to get the site email address to show up prefaced with an actual name like
"My Site"
Comment #41
cburschkaThis issue is one major release old and needs to be moved up.
Incidentally, on giving this some more thought I am wondering if it might not make more sense to drop support for this completely. Instead of allowing the email address to contain a specific name, just format outgoing From headers as
"$site_name" <$site_mail>
Comment #42
Anonymous (not verified) CreditAttribution: Anonymous commentedI disagree it should be dropped. That method might work but it's hardly user friendly!
Comment #43
Dries CreditAttribution: Dries commentedI'm not 100% either.
Comment #44
kprmca CreditAttribution: kprmca commentedpatch (#9) is working fine... i mean email field is accespting both name & mail address.... but while sending a mail from that mail address its giving problem like this
warning: mail() [function.mail]: SMTP server response: 501 5.5.4 Invalid Address in C:\wamp\www\drupal-6.11\includes\mail.inc on line 193.
Unable to send e-mail. Please contact the site administrator if the problem persists.
Comment #45
kprmca CreditAttribution: kprmca commentedpatch (#9) is working fine... i mean email field is accespting both name & mail address.... but while sending a mail from that mail address its giving problem like this
warning: mail() [function.mail]: SMTP server response: 501 5.5.4 Invalid Address in C:\wamp\www\drupal-6.11\includes\mail.inc on line 193.
Unable to send e-mail. Please contact the site administrator if the problem persists.
Comment #46
Damien Tournoud CreditAttribution: Damien Tournoud commentedI agree with Arancaytar on using the site name as the human readable part of the
From:
header.Comment #47
jcruz CreditAttribution: jcruz commented@GregoryHeller You could also create a custom module and do a hook_mail_alter
Comment #48
perceptum CreditAttribution: perceptum commentedHi guys
Personally I think this is quite important for professional sites. People just tend to overlook mails that are not formatted nicely.
I tried jcruz's module-hack and it works like the bizz! Would be easy to extend this further into core...
Bryan
Comment #49
VM CreditAttribution: VM commentedThis needs to stay at 7.x and back ported from there.
Comment #50
Matt V. CreditAttribution: Matt V. commentedAnother workaround, which I just tested, is to edit the variables table entry directly. I used the "Variable editor" link in the Devel module block, but presumably you could use PHPMyAdmin or another database client tool.
Comment #51
mrfelton CreditAttribution: mrfelton commentedThe custom module in #47 is a good approach and works well for me. I do feel that this feature should be in core though.
Comment #52
carlos8f CreditAttribution: carlos8f commentedMail 'from the system' ($from argument is omitted in drupal_mail) should use "Site name" <site mail> as its From: header. Any disagreements? Please review.
Comment #53
carlos8f CreditAttribution: carlos8f commentedI meant "Site name" <site mail> in #52 :)Comment #54
carlos8f CreditAttribution: carlos8f commentedTagging with usability, since I think seeing the name of the website in your inbox is less confusing than just seeing whatever the site_mail is set to.
Comment #55
Dave ReidComment #56
Gabriel R. CreditAttribution: Gabriel R. commentedBoing.
Comment #58
benone CreditAttribution: benone commentedSo how it looks now ? Is there any module for D6 which can give a simple site name to the email sender name ?
I use 6.15 and still have only option for email address. No name.
Subscribing
Comment #59
JaredAM CreditAttribution: JaredAM commented+1
Comment #60
coyotemojo CreditAttribution: coyotemojo commentedHello,
I tried the patches in
Hello,
I tried both patches #34 and #52 but I could not get the site emails to come "from" the site name.
Do I need to do anything else besides applying those patches?
Jason
Comment #61
pazzypunk CreditAttribution: pazzypunk commented#52: nice_from_header.patch queued for re-testing.
Comment #62
perkesame as #60, wonder if the method that includes module and not hacking core would work better
Comment #63
c960657 CreditAttribution: c960657 commentedIf site_name contains 8bit characters, the From header line becomes
From: =?UTF-8?B?InTDqXN0IiA8Zm9vQGV4YW1wbGUuY29tPg==?=
. My mail server changes this toFrom: =?UTF-8?B?InTDqXN0IiA8Zm9vQGV4YW1wbGUuY29tPg==?=@mail.example.org
, and my mail client displays it as “tést @mail.example.org”.
Comment #64
babbage CreditAttribution: babbage commentedToo late for this to get into D7 now, moving to D8...
Comment #65
shyam541 CreditAttribution: shyam541 commentedSubcribing
Comment #66
GuyPaddock CreditAttribution: GuyPaddock commentedWe'd better move it to Drupal 9, so this patch makes it out in time for Duke Nukem Forever.
Come on, guys, this can't be THAT hard to include in core already.
Comment #67
GuyPaddock CreditAttribution: GuyPaddock commentedFor now, it looks like the Personalized E-mails module will address this for D6.
Comment #68
Daniel Norton CreditAttribution: Daniel Norton commentedNot convinced that this can’t easily be addressed in D7 (and D6), I added this issue and a corresponding patch:
Comment #69
lukebrooker CreditAttribution: lukebrooker commentedMy solution to this problem is I was already using the Mime Module and by changing the site email in it's settings page to something like "Site Name" it bypasses the normal validation and also updates the email in the site settings page. I don't even have to have "send html emails" turned on in mime mail's settings, as long as the module is installed.
Comment #70
Cyberwolf CreditAttribution: Cyberwolf commentedSubscribing.
Comment #71
glitz CreditAttribution: glitz commentedMime Module did not work for me. Any other solutions?
Comment #72
glitz CreditAttribution: glitz commentedan anyone else confirm that http://drupal.org/project/pmail will take care of this without an issue?
Comment #73
TheodorosPloumisSubscribe
Comment #74
glitz CreditAttribution: glitz commentedVerified PMail will take care of this
Comment #75
g000fy CreditAttribution: g000fy commentedI just tested PMail. It works. Didn't even have to configure the module!
Comment #76
frankfarm CreditAttribution: frankfarm commentedI confirm that in Drupal 6.20 installing
http://ftp.drupal.org/files/projects/pmail-6.x-1.x-dev.tar.gz
results in the site name followed by the email address you specify in
/admin/settings/site-information
and the email address appears in angle brackets. I also think a better solution is desired for the future.
My concern as Pancho mentioned in #3 is that the email address alone appears spammy. Without Pmail, when importing users, our automated email invitations ended up in junk email and spam folders, so this was problematic for us until Pmail resolved it.
I also agree with carlos8f that this is to some extent a usability issue since a site name is more likely to be recognized than the email address specified in
/admin/settings/site-information
The dev release of the Personalized E-mail module seems to solve the problem nicely for D6, but it is not clear to me if this might also work as a workaround for D7.
Comment #77
Anonymous (not verified) CreditAttribution: Anonymous commentedI'm confused how such a seemingly simple D6 issue can remain open for over 3 years, and in the meantime get reassigned to 2 versions after the original?! Providing the site name in the From header is basic core functionality. It shouldn't require the addition of a contrib module to fix.
Comment #78
gdud CreditAttribution: gdud commentedI attached version of patch #34 that works wit non-ASCII characters in site_name. This patch is for Drupal 6.x.
Comment #79
pillarsdotnet CreditAttribution: pillarsdotnet commentedRe-rolled against 8.x using current patch standards.
Comment #80
pillarsdotnet CreditAttribution: pillarsdotnet commentedBah. regression should be "bug report" not "feature request".
Comment #82
pillarsdotnet CreditAttribution: pillarsdotnet commentedOops.
Comment #83
pillarsdotnet CreditAttribution: pillarsdotnet commentedd8 patch still applies.
Rolled d7 and d6 versions.
Note that this fixes a regression of functionality available in d5 (!!!), hence the backports.
Comment #84
pillarsdotnet CreditAttribution: pillarsdotnet commentedSlight improvement over #82:
$default_from
is both non-empty and valid, because an empty string is also a valid email address.From:
header twice.Comment #86
pillarsdotnet CreditAttribution: pillarsdotnet commentedsilly me. Corrected.
Comment #87
pillarsdotnet CreditAttribution: pillarsdotnet commentedComment #88
pillarsdotnet CreditAttribution: pillarsdotnet commented#86: drupal_mail-209672-86.patch queued for re-testing.
Comment #89
pillarsdotnet CreditAttribution: pillarsdotnet commentedComment #90
pillarsdotnet CreditAttribution: pillarsdotnet commentedComment #91
pillarsdotnet CreditAttribution: pillarsdotnet commented#86: drupal_mail-209672-86.patch queued for re-testing.
Comment #92
pillarsdotnet CreditAttribution: pillarsdotnet commented#86: drupal_mail-209672-86.patch queued for re-testing.
Comment #93
drupal_was_my_past CreditAttribution: drupal_was_my_past commented#86 still applies cleanly to 8.x. I think it just needs to be reviewed or is there another reason this issue has stagnated?
Comment #94
pillarsdotnet CreditAttribution: pillarsdotnet commentedYup; it needs review. I'd do it myself, but the person who authored or last modified a patch should not review their own work.
If you feel up to it, read the (updated) Patch review guidelines and feel free to submit your own review. It's a great learning experience.
Comment #95
Lars Toomre CreditAttribution: Lars Toomre commented#86: drupal_mail-209672-86.patch queued for re-testing.
Comment #96
trzczy0 CreditAttribution: trzczy0 commentedThis is for 7.8. It works like a charm.
Comment #97
pillarsdotnet CreditAttribution: pillarsdotnet commentedPlease read and understand the backport policy before changing the version, or assigning yourself to an issue. Thank you.
This issue has been stalled for five months while awaiting a review of the patch in #86.
If nobody is willing to provide such a review (and RTBC), this issue should be closed (won't fix).
Comment #98
pillarsdotnet CreditAttribution: pillarsdotnet commentedClosing for lack of interest. Please re-open only if you are willing to provide a patch review.
Comment #99
inkage CreditAttribution: inkage commentedJust go for mimemail projet http://drupal.org/project/mimemail
Comment #100
bismigalis CreditAttribution: bismigalis commented#34 is how people expect this should work
Comment #101
olamaekle CreditAttribution: olamaekle commentedUpdated #86 against the current 8.x-dev and tested it. It just gives you an empty "From" field in the mail.
Heres the new patch from #86.
Comment #102
pillarsdotnet CreditAttribution: pillarsdotnet commentedThis won't generate an empty From: header.
Comment #103
olamaekle CreditAttribution: olamaekle commentedI get an error when I try to apply the patch:
I tested the code and the site name is showing but not the site email. It did that with #101 too but Gmail does not display it when the email is missing (Mail.app does).
This is from the email source:
From: "Drupal" <>
I don´t know programing that well but I´ll try my best to help with testing.
Comment #104
pillarsdotnet CreditAttribution: pillarsdotnet commentedThat's a valid email address according to RFC 2822.
Comment #105
olamaekle CreditAttribution: olamaekle commentedStrange that Gmail does not recognize the "From:" field then. But how do I add the email address between the < and > signs?So that i get something like this:
From: "Drupal" <drupal@example.com>
OK... forget that seems to be working now. Going to reinstall and retest now.
Comment #106
olamaekle CreditAttribution: olamaekle commentedRight after I install drupal 8 with #102 patch I get this in the email:
From: "Drupal" <>
When i visit admin/config I get an error:
After that it works just fine:
From: "Drupal" <drupal@example.com>
Any idea?
Comment #107
naxoc CreditAttribution: naxoc commentedHere is a test and a re-roll of the patch.
mime_header_encode()
is called on all header fields inmail()
so I removed that function call. Made the patch smaller and simpler.Comment #108
ryan.gibson CreditAttribution: ryan.gibson commentedya lo tengo
Comment #108.0
ryan.gibson CreditAttribution: ryan.gibson commentedUpdated issue summary to match template standards.
Comment #108.1
ryan.gibson CreditAttribution: ryan.gibson commentedUpdated issue summary.
Comment #109
ryan.gibson CreditAttribution: ryan.gibson commentedAlright, I tested the patch from #107; it applied cleanly. Before applying the patch, emails appeared as from the site administrator's email account only. After applying the patch, the e-mail did have a "from" and displayed the site name. If this is what we're wanting then the patch works great.
Comment #110
ryan.gibson CreditAttribution: ryan.gibson commentedComment #111
xjmItsy-bitsy correction:
This should be one line of 80 characters or fewer. Reference: http://drupal.org/node/1354#functions
Patch looks great to me other than that, and the test coverage is correctly exposed in #107.
Comment #112
pillarsdotnet CreditAttribution: pillarsdotnet commentedAdjusted according to #111 and re-rolled with "git format-patch". No code changes, hence no change in status.
Comment #113
sunMissing blank line.
Comment #114
naxoc CreditAttribution: naxoc commentedReroll and fixed the missing blank line.
Comment #115
penyaskito#114: site-name-in-emails-209672-114-tests+fix.patch queued for re-testing.
Comment #117
penyaskitoThis should be a feature request, not a task, and it should be committed before the Feature Freeze deadline.
I tested the patch above and worked fine. Just rerolled it because the test moved from Common to Mail.
Comment #118
penyaskitoThis failed, but testbot didnt gave feedback.
Comment #119
penyaskitoSite mail variable is already converted to CMI. New try.
Comment #120
penyaskitoBackport for D7 once this is committed and marked as "patch(to be ported)".
Comment #121
penyaskitoNeeds work. The test is not well designed, it should have failed. This needs CMI in site.name too.
Comment #122
penyaskitoNew attempt, now with site.name converted to CMI too. Edited the test so it doesn't use variable_get, but asserts for the settings at the WebTestBase class setUp.
Comment #122.0
penyaskitoUpdated issue summary.
Comment #123
penyaskitoUpdated summary.
Comment #123.0
penyaskitoUpdated summary
Comment #123.1
penyaskitoimg
Comment #124
e0ipsoWorks for me.
Marking as RTBC.
Comment #125
catchCommitted/pushed to 8.x, thanks! Moving to 7.x for backport. This isn't really a new feature either, although not sure what David will think about backport.
Comment #126
geerlingguy CreditAttribution: geerlingguy commentedLatest D7 patches are in #120. (FYI)
Comment #127
naxoc CreditAttribution: naxoc commentedHere are the patch(es) from #120 for the testbot.
Comment #128
penyaskitoWorks for me and tests passed. Let's see what David thinks about it.
Comment #129
David_Rothstein CreditAttribution: David_Rothstein commentedWell, at a minimum I think the above could use more discussion. It's documented that if you explicitly pass in $from to drupal_mail(), the "From" header will get set to that, and I'm not sure there's any good reason to create exceptions to that.
As for the overall issue... hm. This change is probably what most sites would want. But 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. (It might be coming from a specific person, for example, and their personal e-mail address alone could make more sense than having it wrapped in the site name.)
Given that it should be very very easy to write a contrib module that adds this behavior (there are already code snippets above) wouldn't it be better to just leave it to contrib in Drupal 7?
Comment #130
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last thing that is required is more discussion. In a few days time, this issue will have been open for 5 years! That's ridiculous...
Comment #131
xjm@martinjbaker, such remarks are not helpful and are discouraging to the contributors who've donated their time to work on this issue.
We have to be careful what changes we backport to Drupal 7, because it is very important not to unexpectedly alter production sites in a stable release. See what @David_Rothstein said above:
So, if you want to see this patch get in, update the patch in #127 to remove the change @David_Rothstein pointed out in #129 and leave the other change.
Comment #131.0
xjmli's
Comment #132
ogomez78 CreditAttribution: ogomez78 commented127: d7-site-name-in-emails-209672-128-testsonly.patch queued for re-testing.
Comment #134
vasi1186 CreditAttribution: vasi1186 commentedMaybe we should enclose the name of the site in double quotes, so we would have
instead of
I had an issue with the From header when the site name contained characters like '>'. For example, like this: >>site_name>>
Comment #141
jochenf CreditAttribution: jochenf commentedI'm using Drupal 8.4 which already uses sitename in mail from:
When sitename contains UTF-8 characters (german umlauts), the whole from header gets encoded (base64?). According to RFC 2047/RFC 822 the mail address must be parsable and is not allowed to be encoded.
Instead of encoding the whole line like this:
From: =?UTF-8?B?QmFyZnXDnyBpbSBBbGxnw6R1IDxhZG1pbkBiYXJmdXNzLWFsbGdhZXUuZGU+?=
It should be
From: =?UTF-8?B?QmFyZnXDnyBpbSBBbGxnw6R1?=
Comment #142
Andrew Answer CreditAttribution: Andrew Answer as a volunteer commentedI found this issue because of now (in 2019) I see what in D8.7 no sender name (seriously?) How I can implement this?
Comment #143
Andrew Answer CreditAttribution: Andrew Answer as a volunteer commentedThis patch allow me to use site name as sender in From field.
Comment #144
jcisio CreditAttribution: jcisio at Axess Open Web Services commentedIt seems that the fix missed one place. I don't know the difference between
$message['from']
and$headers['From']
, but here is a patch which should fix the test in #143.Comment #146
jcisio CreditAttribution: jcisio at Axess Open Web Services commentedOk, now I fixed and tested locally.
Comment #149
sokru CreditAttribution: sokru as a volunteer commentedManually tested patch from #146, with multilingual site (different site name per language). No problems, so RTBC.
Comment #150
alexpottThis issue was about the backport of the fix to Drupal 7. The omission from the original patch discovered by @Andrew Answer needs a new issue. We shouldn't be piling on to this one years after it has been committed to Drupal 8. Also in the meantime Drupal 7 backport policy has changed to requiring a new issue to opened. So can someone:
Comment #151
alexpottComment #152
alexpottAlso fwiw I think we need to understand more about the problems @Andrew Answer is experiencing - because with the default Drupal 8 core mailer the from address on the email will come from the headers.
Comment #153
sokru CreditAttribution: sokru as a volunteer commentedCreated an issue for Drupal 8 #3098024: Use site name in From: header for system e-mails
Comment #154
alexpottI created the D7 issue - #3098058: [D7] Use site name in From: header for system e-mails - so now we can close this one as fixed.
Comment #155
alexpott