Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
We are using this module and the phpmailer module that is implemented by the http://drupal.org/project/messaging class. There are scenarios (especially during running cron) that the other module loads the PHPMailer class first.
Here is how to prevent that:
// Include the PHPMailer class (which includes the SMTP class).
if (!class_exists('PHPMailer')) {
require_once(drupal_get_path('module', 'smtp') .'/phpmailer/class.phpmailer.php');
}
Comment | File | Size | Author |
---|---|---|---|
#6 | smtp_libraries.diff | 2.78 KB | designerbrent |
#1 | smtp-541942.patch | 787 bytes | jdwfly |
Comments
Comment #1
jdwfly CreditAttribution: jdwfly commentedMade a patch for this against the dev version. Might work for regular version too.
Comment #2
markus_petrux CreditAttribution: markus_petrux commentedRelated issue in the messaging module queue: #541938: using PHPMailer twice crashes messaging_mailer
SMTP could also try first using libraries module. See the patch attached in the above mentioned issue.
That way, sites using SMTP with no other module loading PHPMailer class may take advantage of libraries. ie. the class could be installed out side the module directory, which makes upgrades a lot easier. And 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().
Comment #3
markus_petrux CreditAttribution: markus_petrux commentedtagging
Comment #4
ShutterFreak CreditAttribution: ShutterFreak commentedYes, it would be a great idea to install phpmailer under sites/all/libraries, like the Wysiwyg library already does for the 3rd party HTML editors.
Comment #5
z3cka CreditAttribution: z3cka commentedI agree with ShutterFreak. this library should be able to live in sites/all/libraries
The problem I had was when updating this module with drush up, it moves the phpmailer library with it to the backup directory along with the module files.
If it was in the libraries directory, we would not have this problem. I will look into writing a patch that would accomplish this.
Comment #6
designerbrent CreditAttribution: designerbrent commentedAttached is a patch that allows you to place phpmailer outside of the module folder. It looks in 3 locations:
Additionally this adds the hook_requirements function to cleanly warn the user that the phpmailer doesn't exist.
Comment #7
z3cka CreditAttribution: z3cka commentedThe patch works with phpmailer in sites/all/libraries ! Thanks designerbrent, nice job (now I don't have to TRY to write this patch). +1 for adding to the next release.
Tested on a fresh install with the following modules enabled:
Comment #8
esteewhy CreditAttribution: esteewhy commented+1 for using libraries.module and keep externals out of module dir.
Comment #9
akalata CreditAttribution: akalata commentedCan also confirm sites/all/libraries success using this patch.
Comment #10
fr34ck CreditAttribution: fr34ck commentedThis patch work perfectly!
Thank you!
Is possible to include this patch in the next release?
Comment #11
franzHi, nice job, looks great. I have a few questions before proceeding:
- Isn't there going to be a dependency on .info for libraries? If so, patch must include.
- SMTP had some issues with 5.x versions of PHPMailer, is it really working?
- There are patches that need to be applied to 2.x versions of PHPMailer. Instructions for these are missing.
Looking forward to those patches also for the D7 version
Comment #12
yettyn CreditAttribution: yettyn commentedThought I should add my thoughts to this as I privately been working on refactoring the module to work with D7 as current dev version is non-working as is.
I was also thinking of and in benefit of adding phpmailer as a library but while it may make sense to D6 alone, I've changed my mind regarding D7. The main reason for this, imho, is that a library like this shouldn't be needed by other modules, and they should really use this module instead if they have need for it. The further conclusion of this is also that there is no point in having several modules offering a smtp service, which is what phpmailer really have to offer D7. The other features are already present as I see, Mime Mail and Html Mail etc.
Futher more phpmailer nice but rather bloated with things we don't need, although alternatively one could do it as adding the full library and change other modules to use these other parts instead of what they use now. I haven't done any investigation here though which modules and what that may be. So adding the full library, just to use the smtp class is bulky in my view and it also has the side effect it cannot be stored in Drupal CVS.
Another small but rather important detail is that phpmailer need to be patched to work with smtp module, which makes it less suitable to live in sites/all/libraries.
The best solution in my view is to take the parts we need and re-release it as GPL (it's now LGPL, which allows to to be modified and re-released) and as a part of this module. I'm willing to take on that job and indeed already have a version that works with current D7 dev version. This way this module will work "out of the box" which I think is what most want to have.
Also, there is a high probability that phpmailer won't have any more updates, at least not for the parts that concerns us, so it shouldn't be any maintainability hog and we could streamline it to fit our needs. What Drupal really need is SMTP to work out the box, and in core, which won't happen before D8 so lets make this something that easily can be dropped into core when the time comes.
Comment #13
Simon Georges CreditAttribution: Simon Georges commentedClosing #919958: SMTP doesn't check if PHPMailer class is already loaded as a duplicate of this one.
Comment #14
polskikrol CreditAttribution: polskikrol commentedHas the patch mentioned above be applied to 6.x-1.0-beta6 when that comes out?
Comment #15
designerbrent CreditAttribution: designerbrent commented@yettyn: While I agree with you that there should be no use outside of this module for the PHPMailer library, the real problem for me was that EVERY time I updated the smtp mailer via Drush, it would delete the PHPMailer folder. Then I had to go out and find the right file to download (No small task in itself!) and get it back into place. When you use the libraries method, that is not a problem as you can update the module and the library remains.
Comment #16
franz@designerbrent
I think that yettyn suggested, actually, that the module should include the library itself on future releases (we're talking about D7 actually), so it would be 100% out-of-the-box. =)
But concerning D6, we could work on with libraries as well.
Comment #17
markus_petrux CreditAttribution: markus_petrux commentedMaybe there is no need to include the PHPMailer library itself. That's just a class that can be extended to suit custom extensions, but it is not a trivial class, it deals with something complex. Moving it here would duplicate the effort made by others. In fact, this is what the PHPMailer modules does, and it is a nice method to reuse the job made by others, so that each party can be specialized, one knows about SMTP stuff, and the other knows how to integrate that in Drupal.
Maybe these modules could even join forces so that code reuse can be enhanced even more.
Comment #18
Simon Georges CreditAttribution: Simon Georges commented@markus_petrux, there is #878334: SMTP Authentication vs. PHPMailer module regarding the merge / integration of these two modules.
Comment #19
franz@markus_petrux, the join suggestion is good. But both modules are supposed to be doing the same thing. One has the name of the library, the other doesn't. It makes no sense saying it would duplicate efforts, since PHPMailer duplicated the function of SMTP an year ago, and using the same library.
Both modules should integrate perfectly with Drupal, so you don't have to use both. Also, there is the possibility that SMTP become an in-core feature, and that doesn't go well with libraries. I really think that, for D7 the effort must be to include the library, and that's not hard to do.
Comment #20
designerbrent CreditAttribution: designerbrent commentedIs there anyway that patch #15 could get integrated in the 6.x version anytime soon? Would really help us out.
Comment #21
franzuh, there is no patch at #15...
Comment #22
designerbrent CreditAttribution: designerbrent commentedSorry! I meant the patch at #6.
Comment #23
franzAny idea whether an existing installation (phpmailer inside module folder) is also working without troubles? If someone can confirm this, please put issue on RTBC
Comment #24
akalata CreditAttribution: akalata commentedConfirmed, patch in #6 does not adversely affect an existing installation with phpmailer included.
Comment #25
franzCommited.
Comment #26
z3cka CreditAttribution: z3cka commentedhooray! thanks for the commit franz! oadaeh told me that this patch had been committed. I shall start using the dev version today!