using PHPMailer twice crashes messaging_mailer

I don't have any other modules using phpmailer installed, so I can't test that this solves the problem, but I can at least say that it doesn't break anything. Thanks for the report and fix.

I wonder if a better solution in the long-run wouldn't be to make a separate module just to contain PHPMailer, so that administrators don't have to maintain multiple copies of it in multiple modules (and to avoid problems such as this).

http://drupal.org/project/phpmailer

- #563112: PHPMailer support for Libraries API!

[EDIT] I have posted a patch to the above mentioned issue, and aside from depending on Libraries API, the PHPMailer class is loaded on demand. Maybe other modules such as messaging_phpmailer in need of the PHP Mailer class could depend on PHPMailer module?

Anyway, if messaging_phpmailer cannot depend on PHPMailer module, then maybe it could depend on Libraires API. This would allow site administrators place the PHPMailer class on a common location that can be shared with other modules in need for this library. And it makes it a lot easier to upgrade Messaging package versions because the PHPMailer class would located on a separate location, which is the point of the Libraries API.

Ok to use libraries and also checking for PHPMailer. However I'd like it to fallback to current location so people with existing installs don't see them breaking after upgrade. Some chained if-then will be fine.

if (!class_exists('PHPMailer')) {
  if (module_exists(...)) { 
    ....
  } else {
     include_once './'. drupal_get_path('module', 'messaging_phpmailer') .'/PHPMailer/class.phpmailer.php';
  }
}

How about this?

PS: Before trying the patch, line endings in messaging_phpmailer.install need to be fixed in the repository. It is now using CRLF!

Oops! fixing typo in watchdog message: s/Could/Could not

For the record: here's a good reason to keep 3rd party libraries off the modules directories: #546584: Modules or themes with too many files kill drupal_system_listing performances. ie. less subdirectories under modules directory means less job for drupal_system_listing().

This looks very good. Just about the logic, could be cleaned up a little bit

  if (!class_exists('PHPMailer')) {
    // First, try using libraries module.
    $phpmailer_library =  libraries_get_path('phpmailer') .'/class.phpmailer.php';
    if (module_exists('libraries') && file_exists($phpmailer_library)) {
        include_once $phpmailer_library;
    }
    else {
      // If PHPMailer is not already loaded, then try from module subdirectory.
     $phpmailer_library = drupal_get_path('module', 'messaging_phpmailer') .'/PHPMailer/class.phpmailer.php';
      if (file_exists($phpmailer_library)) {
        include_once $phpmailer_library;
      }
    }
  }

Also note we remove './.' from paths. Does this look good / works?

Thanks for taking the time to review the patch.

1) Removing './.' from paths will work, but PHP will first look at include_path directories. The './' prefix makes PHP ignore include_path altogether. So this is used for perfomance reasons.

http://www.php.net/include

2) We cannot invoke libraries_get_path() if we haven't checked first if libraries module exists.

I tested the patch in #7 with and without PHPMailer class, with and without libraries module. And worked here, but it would be nice if someone else (other than me) can confirm this.

tagging

Fair enough, applied patch in #7

Thanks.

Noob question: where would I put that code?

Thanks
A

has this been committed?