Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

"reply-to" instead of "from" #21

Closed
ghost opened this issue Jan 29, 2015 · 4 comments
Closed

"reply-to" instead of "from" #21

ghost opened this issue Jan 29, 2015 · 4 comments
Labels
Milestone

Comments

@ghost
Copy link

ghost commented Jan 29, 2015

In the sendForm method (line 223) wouldn't it be better to have the submitter's email address entered in a "Reply-To" line in the header as opposed to being used directly in the "From" line? The "From" line should actually be the domain the form is running on, no?

I was having some trouble with the plugin not reliably delivering some emails (especially when the submitter was using a gmail address, for instance). I think receiving servers were seeing a gmail "From" address and realizing the IP was not gmail, so they got suspicious and didn't deliver it. But I don't think this problem would necessarily be isolated to gmail.

fyi, the failed mail sent me to this link for more info: http://dhurl.org/20b

If I have some time, I may fork and test this. I'll keep you posted.

@ghost
Copy link
Author

ghost commented Jan 30, 2015

This is working better -- with a 'replyTo' line added into the params array right after the "from" line.

Mail is now seems to be being delivered reliably with this form -- including gmail from-addresses. And can still reply to the sender by clicking "reply". However, I'm not getting the 'name' to show up in either of these for some reason. I'm not super familiar with Kirby, so I'm not 100% sure how this is working. That would be preferable to get the send name to show in the "from" line with the host domain. I just hardcoded the hosting domain for a quick fix, but it would be best to make that dynamic obviously -- one of the kirby url functions maybe with preg replace or something?

'from'        =>   a::get($this->data, 'name', '') . ' <admin@the-hosting-domain.com>',
'replyTo'   =>   a::get($this->data, 'name', '') . ' <' . a::get($this->data, '_from') . '>',

Thanks for your great work on this plugin. I realize I may be missing something that caused me to have this problem in the first place -- and you've dealt with it in some way that I don't see here.

@mzur
Copy link
Owner

mzur commented Jan 30, 2015

I totally agree, replyTo would be a better field for the submitter's email address. I'd prefer not to generate the from address directly from the domain name since the user should decide which of their email adresses they want to take. It can be an additional (required) argument in the email array.

As to why the name does not appear I found that it is intentionally removed by the Email class. I guess we could open an issue at the toolkit repo and ask if this can be fixed or we could write our own email service that adds the name to the header again.

@mzur mzur added this to the v2 milestone Jan 30, 2015
@mzur mzur added the bug label Jan 30, 2015
mzur added a commit that referenced this issue Feb 1, 2015
The email address of the '_from' form field now will be used for the 'replyTo' field of the email to be sent. The 'from' field now must contain an email adress belonging to the domain that sends the email. Closes #21.
@mzur
Copy link
Owner

mzur commented Feb 1, 2015

Opened a new issure for the names.

@mzur mzur closed this as completed Feb 1, 2015
@ghost
Copy link
Author

ghost commented Feb 1, 2015

Thanks, Martin. This looks good to me -- handling the sender in the setup array to give users some flexibility.

This plugin is looking really good, by the way. Glad you renamed it in v.2, because it's potentially a lot more than a "contact form." :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant