WSOD: smtp_library variable is not removed when mimemail has been disabled

Bless you kenorb. Really.

Wow, just got bit hard by this bug, spent more time than I care to admit tracking this down!

It's actually worse than you think kenorb. There's also the case that the user had mimemail enabled (so smtp_library is set to mimemail), and then disables but does not uninstall mimemail. The variable still points to mimemail, and drupal (in mail.inc::drupal_mail_send()) just checks the smtp_library variable and to make sure the include file exists - it never checks to make sure the module is enabled.

So yeah, the upshot is, to remove the effects of this module, delete it. Or with kenorb's patch above completely uninstall it! Just disabling the module does not clean up enough.

Wow. Finally found this after working 3 days and embarrassing myself in front of the organization. Only deleting the folder cured the problem.

Not criticizing too robustly, though, because I have an un-deleted system variable in my module too! Thanks to the community for the lesson.

I tell ya what... I'll clean up mine if you clean up yours! And then I'll reinstall MimeMail!

Michael

Thanks for keeping this updated, I hadn't thought about this issue in months.

After rereading though, it seems obvious that Drupal's mail.inc has a bug in drupal_mail_send(). It should check to make sure the include file it's attempting to load is from an enabled module. This would be a good safety check against all contributed modules that don't handle the smtp_library variable properly, not just this one.

Created #678200: drupal_mail_send includes libraries from disabled/uninstalled modules

(edit fixed node link)

Would be enough to implement hook_disable() and delete the smtp_library variable on disable, since only a disabled module can be uninstalled.

Ran into this issue today.

Using mimemail-6.x-1.0-alpha2.tar.gz.

I configured mimemail - and set it for PHPMailer. I was using it with SMTP Authentication, so I needed to test with the other interface.

The PHPMailer was missing a lib, so it threw errors.

I saw the errors in Drupal, so I went to configure Mimemail back to SMTP Auth, but couldn't get past the error screen. Once it executed the changes for SMTP, it will not change back, even though I have uninstalled PHPMailer.

Is there a way to manually unset the SMTP settings so that I can get the server to a functioning state again?

@ernest.park: Delete the smtp_library variable from the variable table in the database.

Patches in #2 and #14 seem to do the trick. Not sure if it should go in hook_disable or hook_uninstall. If the variable gets blown away in hook_disable, the user will have to go back to the admin form to reset the variable when re-enabling the module, though having it in hook_disable would likely prevent a few headaches when wondering what has happened to Drupal's mail system while experimenting with modules.

@mitchmac: I am sure it should go in hook_disable(). If John wants to remove a module, it is not certain that he will uninstall it entirely, but he must and he will disable it, that's certain. As kscheirer wrote, if we don't handle this in the disable process the problem still persists. If we do, it's completely unnecessary in hook_uninstall().

IMHO if you tested the patch in #14 and found it working, this can be marked as RTBC.

@sgabe: Yep, I follow and agree with your logic. I just believed that there was a general convention to persist user generated settings until a module is uninstalled, though, since this is a variable not specific to the module and has annoying consequences if left around, all's fair.

On the topic of conventions, it seems hook_disable is usually implemented in the module's .install file.

@mitchmac: You're absolutely right. I'm attaching a new patch with hook_disable() implemented in the .install file.

you're right, typically hook_disable is not for deleting any data, the idea being a user can re-enable the module and be right back to where they started.

How about a hook_requirements() that will at least let the admin user know that even though the module is disabled, it is still being used as the smtp library, and include some instructions on how to resolve it (uninstall module or delete the variable manually).

OR

remove the variable on hook_disable() as planned, and set the variable again on hook_enable().

[...] even though the module is disabled, it is still being used as the smtp library [...]

I think it's nonsense. :) This is exactly what we need to prevent here. IMHO: Please, test the attached patch and set the issue to RTBC if it works.

It works and is obvious...

Changing version to HEAD.

Committed to HEAD.