ActiveCampaign / postmark-wordpress

The Official Postmark Wordpress Plugin
GNU General Public License v2.0
18 stars 17 forks source link

Plugin not working - because it is overwritting the wp_mail() function #70

Closed andrejarh closed 3 years ago

andrejarh commented 3 years ago

The Postmark plugin overwrites the native WordPress wp_mail() function. Tthis is a bad practice. You should use the wp_mail filter instead, and use unique names for your functions.

The Postmark plugin now relies on loading the modified wp_mail function before the official WordPress function wp_mail is loaded.

This solution works OK, but only if some other plugin does not require the official WordPress wp_mail() function before...

But plugins do this sometimes. For Example the Redux Framework plugin, which a lot of theme autors use for their Theme Options. In order to load Redux extension like the Demo import Redux extension, they require pluggable.php file (The file where WP core function wp_mail is defined). Example code: require_once ABSPATH . '/wp-includes/pluggable.php';

This causes the wp_mail function to be registered before it would normally be and before the Postmark plugin overwrites the wp_mail function with its own version.

And in this case, the Postmark is not loaded at all:

if ( ! function_exists( 'wp_mail' ) ) {
  $postmark = new Postmark_Mail();
    ...
  }
}

If you would use the wp_mail filter instead of overwriting the wp_mail function, you would still get access to all the WordPress mail data, but without the need for overwriting the WordPress functions. Your custom wp_mail function should be restructured as separate function with unique naming.

I know fixing this issue requires a lot of refactoring, but it will solve many potential issues.

pgraham3 commented 3 years ago

This is great info - thanks @andrejarh!

While it would require some substantial refactoring, like you mentioned, I do think it would be worth it in the long run. I'll see if I can get this slotted into my work sometime over the next couple quarters.

mgibbs189 commented 3 years ago

The wp_mail function is meant to be overwritten, hence the comment at the very top of pluggable.php:

 * These functions can be replaced via plugins. If plugins do not redefine these
 * functions, then these will be used instead.

If you look at the wp_mail function, you'll see that it's heavily tied to the PHPMailer class, and there are no hooks to control this. This was by design. The WP core team did not think that there would be a need for multiple plugins to handle wp_mail in much the same way that only a single plugin can use db.php, advanced-cache.php and other drop-ins.

@andrejarh mentions the wp_mail filter... but if you actually look at it, you'll see that it only affects the email itself, not the actual transport layer (e.g. how it's sent):

$atts = apply_filters( 'wp_mail', compact( 'to', 'subject', 'message', 'headers', 'attachments' ) );

So until some hooks get added that allow for PHPMailer to get swapped out, I don't see any way around how the Postmark plugin currently handles it.

pgraham3 commented 3 years ago

Thanks @mgibbs189!

I'll go ahead and mark this as wontfix then with this new information.