Contact Form: Add display names to addresses in the To header#6657
Contact Form: Add display names to addresses in the To header#6657
To header#6657Conversation
|
It looks like the test failures in Travis are unrelated. |
|
I'm rerunning the failed 7.0 test edit: rerun tests worked fine. |
|
|
||
| foreach ( $to as $to_key => $to_value ) { | ||
| $to[ $to_key ] = Grunion_Contact_Form_Plugin::strip_tags( $to_value ); | ||
| $to[ $to_key ] = self::add_name_to_address( $to_value ); |
There was a problem hiding this comment.
Would it be worth attempting to see if there is a "Name" field, and if so, use that instead of the email address for the name portion?
There was a problem hiding this comment.
Ah, yeah, that's a good idea. I'll add that :)
There was a problem hiding this comment.
Er, nevermind, now I remember why I didn't do that. We don't actually have a good source for the $to name, and it seems unlikely that users would take advantage of the ways it could be made available. It comes from these sources:
- The
[contact-form to=""parameter. The form builder will remove anything wrapped in<or>fields when inserting the shortcode, so it would break an address with a name part. I guess somebody could manually add it a name to the parameter, but that seems like a pretty small edge case, and it'd break if they ever used the form builder to modify the shortcode.add_name_to_address()does make sure that it only adds a name if it's passed a raw address, so it should work if anyone tries it, as long as they don't use the form builder. get_option( 'admin_email' ). We could try to fetch anyWP_Useraccounts with a matching email address, then pull that user's name, but it won't always work sinceadmin_emailis independent of user accounts. It wouldn't hurt anything, it could just fall back to using the address as the name if it didn't, but I'm not sure it's worth it. I'm happy to add it if you feel like it is, though.- The
contact_form_tofilter, which also seems like a small edge case. I don't think many people will interpret the filter docs as saying that a name part would be allowed, but it's possible.add_name_to_address()wouldn't modify the address if it were included, so this should already work.
Right now the To message header is the only place that's using add_name_to_address(), since the From and Reply-To headers are already built with names.
samhotchkiss
left a comment
There was a problem hiding this comment.
Looks fine to me-- kind of silly, but if it gets us better deliverability, then I'm all for it.
|
a505b8b merges |
3a5215e to
b6067c8
Compare
|
Rebased to fix conflicts and avoid merge commits. |
|
Tested, works well for me! |
* Changelog: initial commit for 4.9 release. * Changelog: add #6929 * Changelog: move old changelogs to changelog.txt * Readme: restore deleted release post link. The post is now live. * Changelog: add #6853 * Changelog: add #6856 * Changelog: add #6857 * Changelog: add #6884 * Changelog: add #6885 * Changelog: add #6892 * Changelog: add #6894 * Changelog: add #6898 * Changelog: add #6899 * Changelog: add #6900 * Changelog: add #6909 * Changelog: add #6927 * Changelog: add #6947 * Chagelog: add #6958 * Changelog: add #6961 * Changelog: add #6963 * Changelog: add #6965 * Changelog: add #6986 * Changelog: add #7000 * Changelog: add #7013 * Changelog: add #7015 * Changelog: add #7019 * Changelog: add #7028 * Changelog: add #6998 * Changelog: add #6999 * Changelog: add #7044 * Changelog: add #6881 * Changelog: add #6922 * Changelog: add #6940 * Changelog: add #6962 * Changelog: add #6942 * Changelog: add #6959 * Changelog: add #7018 * Changelog: add #6948 * Changelog: add #6657 * Changelog: add #7030 * Changelog: add #7048 * Changelog: add #7031 * Changelog: add #6990 * Changelog: add #6957 * Changelog: add #7027
This avoids SpamAssassin's
TO_NO_BRKTS_HTML_ONLYrule.See #6114
Testing instructions:
master, fill out a[contact-form]and copy the email source to http://spamcheck.postmarkapp.com/TO_NO_BRKTS_HTML_ONLYrule