Skip to:

Opened 2 years ago

Closed 18 months ago

#7024 closed defect (bug) (wontfix)

BP no longer filters `wp_mail_from_name` when using `wp_mail()`

Reported by: Boone Gorges Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.5.0
Component: Emails Keywords:


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)

7024.01.patch (1.1 KB) - added by Paul Gibbs 2 years ago.
7024.02.patch (3.1 KB) - added by Paul Gibbs 22 months ago.

Download all attachments as: .zip

Change History (19)

#1 @John James Jacoby
2 years ago

An example from the forums:

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:

  • creates a negative developer experience
  • increases the learning curve of interacting with BP Email
  • increases our maintenance overhead of BuddyPress's codebase
  • duplicates a pretty significant portion of PHPMailer & friends
Last edited 2 years ago by John James Jacoby (previous) (diff)

#2 @Boone Gorges
2 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 @John James Jacoby
2 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 @Boone Gorges
2 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 @Paul Gibbs
2 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. :)

@Paul Gibbs
2 years ago

#6 @Boone Gorges
2 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 @Paul Gibbs
2 years ago 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).

Last edited 2 years ago by Paul Gibbs (previous) (diff)

#8 @Paul Gibbs
22 months ago

  • Component changed from API to API - Emails
  • Milestone changed from Awaiting Review to 2.5.4

#9 @Paul Gibbs
22 months 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.

#10 @Paul Gibbs
21 months ago

  • Milestone changed from 2.5.4 to 2.6.1

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.

21 months ago

#12 @Boone Gorges
21 months 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

#13 @David Cavins
21 months ago

  • Milestone changed from 2.6.1 to 2.6.2

This ticket was mentioned in Slack in #buddypress by mercime. View the logs.

20 months ago

This ticket was mentioned in Slack in #buddypress by mercime. View the logs.

20 months ago

#16 @David Cavins
20 months ago

  • Milestone changed from 2.6.2 to 2.7

#17 @Boone Gorges
18 months ago

  • Milestone 2.7 deleted
  • Resolution set to wontfix
  • Status changed from new to closed

I haven't heard any uproar about this, so let's not do it.

Note: See TracTickets for help on using tickets.