Opened 9 years ago
Closed 8 years ago
#7024 closed defect (bug) (wontfix)
BP no longer filters `wp_mail_from_name` when using `wp_mail()`
Reported by: | boonebgorges | Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 2.5.0 |
Component: | Emails | Keywords: | |
Cc: |
Description
Prior to 2.5, BP filtered wp_mail_from_name
to replace with bp_get_option( 'blogname' )
. In 2.5, if you force the use of wp_mail()
, this filtering no longer takes place.
Was it taken out just to clean things up, or for some more principled reason? It's no biggie to replace it on client sites, but it involved some annoying debugging.
#6999 is related, but is not specifically about wp_mail()
.
Attachments (2)
Change History (19)
#2
@
9 years ago
@johnjamesjacoby - Your requests for retaining backward compatibility with existing filters are not without merit. But, just for clarity, my issue in this ticket is a mostly separate one: I am choosing to use wp_mail()
instead of the new email functionality, and BP has stopped filtering some values in wp_mail()
that it always filtered in the past.
#3
@
9 years ago
@boonebgorges - This is why I mention both the list of filters that are missing, and mention retaining wp_mail()
because then we wouldn't need to maintain our own separate whitelist of WordPress core filters in BuddyPress's API.
#4
@
9 years ago
OK. I just don't want my modest ticket, which is intended to be a questions as much as a bug report, to get buried by an extensive discussion about the architecture of the mail feature (or your legitimate concerns to get buried in a ticket about a minor bug).
#5
@
9 years ago
@boonebgorges I think it was just removed as part of tidying up. Re-instating it as it was is going to affect the new implementation after #6999. How about something like this?
- Put back bp_core_email_from_name_filter() to wp_mail_from_name filter.
- And then apply the patch I'm about to upload.
I'm on vacation for a few weeks so if you think this is close, you are very welcome to run with it. :)
#6
@
9 years ago
Thanks, @DJPaul. Your plan sounds OK, except I don't really understand your patch. What is it about reinstating the filter that means we have to run the title through wp_specialchars_decode()
? As far as I see, that's the only change you're proposing.
This can wait until after you're back :)
#7
@
9 years ago
https://github.com/paulgibbs/BuddyPress/commit/2a621518d4f2eb39c848c2b96999e952b853fefe was the commit for this during development.
As things stand, we use the site name (option blogname
) for the default value of the "From name" property. When you set blogname
in WordPress, it's sanitized with esc_html
via sanitize_option
before being stored in the database. Email from/reply to/to/bbc/cc "names" should not be HTML escaped otherwise you can end up with odd-looking values in the email, which in my testing sometimes caused deliverability issue.
In this patch, as we've fixed wp_mail_from_name
support for your edge case (IMO - it wasn't a use case I had considered during development), I figured falling back to use its default value of "WordPress" rather than blogname
made sense for consistency with wp_mail
.
And then the now-deprecated function you found which filters in the blogname
value for us, results in no change from current 2.5 behaviour, except we fix the problem you reported for people using wp_mail
on a BuddyPress site.
(The patch obviously needs to reinstate the old function, I was just looking at the filtering aspect).
#8
@
8 years ago
- Component changed from API to API - Emails
- Milestone changed from Awaiting Review to 2.5.4
#9
@
8 years ago
The updated patch is a full, tested patch for this issue on the 2.5, with the changes described previously. However, @boonebgorges, I am not sure about doing this because I expect most BP sites to use BP email APIs. This patch means we are messing with wp_mail's from name, even though we aren't using wp_mail. I do agree however that this has been a small backwards compatibility break in 2.5.
At any rate, I do not mind significantly either way (whether we commit it or not), so I'll leave it up to you to make the call.
This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.
8 years ago
#12
@
8 years ago
7024.02.patch looks OK to me, though we should remove the _deprecated_function()
call. I have not tested, so I'll leave it to someone who can test to do the committing :-D
An example from the forums:
https://buddypress.org/support/topic/change-the-email-from-name-buddypress-email-templates/
For anyone that's already familiar with customizing WordPress's
wp_mail()
function, BuddyPress introduces an entirely new set of conventions that do not inherit WordPress's.Here are the commonly used WordPress filters
wp_mail()
has that BuddyPress does not have:wp_mail_from
(Done #6999?)wp_mail_from_name
(Done #6999?)wp_mail_charset
wp_mail_content_type
wp_mail_failed
wp_mail_original_content
For reference, BuddyPress avoided using
wp_mail()
because of its pluggable nature. I'd like to suggest this blocker be revisited (here or elsewhere's.)wp_mail()
is widely used and relied upon, and not using it: