Comments

pillarsdotnet’s picture

StatusFileSize
new2.65 KB

Patch applies against mimemail master (7.x-1.x-dev) branch to add support for the Mail System module.

pillarsdotnet’s picture

StatusFileSize
new2.55 KB

Corrected patch. Had to rename some of the mailsystem functions to avoid clashing with Drupal hooks.

pillarsdotnet’s picture

Status: Active » Needs review
sgabe’s picture

The idea is very good (I developed a similar helper module for testing), but the module lack's the support for messages with different $key in the message ID, which looks like this: {$module}_{$key}

However I don't want to make this a requirement and use it in the installation process. Mime Mail can work without it just fine and I think Mail System can do it's job without patching up Mime Mail.

pillarsdotnet’s picture

StatusFileSize
new3.79 KB

Thank you for the feedback.

I have released version 7.x-1.4 which has support for {$module}_{$key} as you describe.

I understand your objections against relying on such a new project, but I am resubmitting a modified patch, anyway. My hope is that this functionality eventually gets included into core.

pillarsdotnet’s picture

Mail System is now available in both 6.x-1.x and 7.x-1.x branches. The 6.x version provides a Drupal-6 backport of the Drupal-7 MailSystemInterface and related code.

pillarsdotnet’s picture

pillarsdotnet’s picture

Title: Use the Mail System module » The Mime Mail module should use the Mail System module

An added advantage of the latest version of Mail System is that it enables using one class to format messages and another class to mail them.

This would enable Mime Mail to integrate with SMTP Authentication Support.

See #1124252: The SMTP module should use or support the Mail System module
and #1125778: Integration with Mailwire

Also see the Administrative U/I screenshot.

sgabe’s picture

Is your patch in #5 still valid?

pillarsdotnet’s picture

@sgabe -- Re-rolled.

You will have some complaints from users about the added dependency, because some of them will just upgrade in-place without reading release notes, installing dependencies, or even running update.php.

The following two patches are both against the master branch of your repository. One integrates Mail System with your 7.x version, and the other creates a 6.x backport.

sgabe’s picture

IMO if we use the Mail System module we can (or more like should) remove the "Use as default mail system." option from the settings page and let just Mail System handle this question to avoid confusion.

As for 6.x I think this will not get into the 1.x branch, we are too close to a stable release now.

pillarsdotnet’s picture

we can (or more like should) remove the "Use as default mail system." option from the settings page and let just Mail System handle this question to avoid confusion.

Yes; that is another benefit of using Mail System.

As for 6.x I think this will not get into the 1.x branch, we are too close to a stable release now.

You should create a 6.x-2.x branch that is a backport of the 7.x code. This will give 25,000 users of Mime Mail 6.x the option to add SMTP Authentication Support.

Also consider this:

  • Comparing the 6.x and 7.x branches after the above patches: 96% same, 4% different

  • Comparing the 6.x and 7.x branches without the above patches: 70% same, 30% different

Doing a backport makes it seven times easier to maintain future improvements across both branches.

sgabe’s picture

The 25,000 sites using your 6.x branch will lose the advantage of being able to use Mime Mail with SMTP Authentication Support.

You just said that the biggest advantage would be the comfort to maintain similar codes. Why would we lose the SMTP support if we don't backport this for 6.x? The Mime Mail API provides support for different mail engines and SMTP implements it just fine. I don't understand why would that happen?

pillarsdotnet’s picture

Why would we lose the SMTP support if we don't backport this for 6.x?

Mail system 6.x-2.x allows using one MailSystemInterface class to format() and another to mail(). For instance, it would allow using MimeMailSystem for formatting, and SmtpMailSystem for sending.

However, the unpatched Mime Mail 6.x cannot use this feature because it conflicts with Mail System.

Also see #1135262: drupal_mail() should support using different MailSystemInterface classes for format() and mail()

sgabe’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
Status: Needs review » Patch (to be ported)

Committed to the master branch. Thank you!

For now I leave this open for 6.x.

sgabe’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Status: Patch (to be ported) » Closed (fixed)

I am marking this as fixed for D7.

At this time there is no consensus about the support of Mail System on D6. We will see if there will be a 6.x-2.x branch of Mime Mail, but I would like to have a stable and final release for D6 and concentrate on D7.

However, pillarsdotnet thanks for your work on Mail System!

anybody’s picture

I strongly recommend to use "phpmailer.module" for Drupal 7. After lots of WTF's I decided to change from SMTP to PHPMailer and everything worked fine ad hoc.

jwilson3’s picture

Assigned: Unassigned » jwilson3
Issue summary: View changes
Status: Closed (fixed) » Patch (to be ported)

At this time there is no consensus about the support of Mail System on D6.

Mail System for D6 is the de-facto solution for handing mail in a stable way; HTMLMail for d6 (this module's competitor) uses it. I'm reopening this as "Patch to be ported" and am working on an actual working patch. The D6 patch on comment #10 above is essentially a patch that takes the Drupal 7 version and poorly converts it to Drupal 6. There were many issues that I had to resolve and so I finally decided that that methodology is not sustainable, and I'd rather write a maintainable patch that adds Mail System support to the 6.x-1.x-dev branch, to maintain backward compatibility, instead of breaking a bunch of extra stuff and doubling or tripling my workload.

If desired this could be used as a basis for starting a 6.x-2.x branch, but I seriously doubt this will happen since d6 is losing momentum and official support in a matter of months.

jwilson3’s picture

Version: 7.x-1.x-dev » 6.x-1.x-dev
jwilson3’s picture

Status: Patch (to be ported) » Needs review
StatusFileSize
new11.42 KB

Patch for D6 to integrate MailSystem with MimeMail.

Note: If you've copied mimemail-message.tpl.php to your own theme to override the default HTML, and your messages are showing up with the default template HTML, try out my patch on #2115299-6: Theme to render the emails settings does not apply anymore.

jwilson3’s picture

StatusFileSize
new14.07 KB

Whoops, forgot to include the new file mimemail.mail.inc in the patch!

Here is a fix.

The last submitted patch, 21: mimemail-mailsystem-1088914-21-D6.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 22: mimemail-mailsystem-1088914-22-D6.patch, failed testing.

jwilson3’s picture

Status: Needs work » Needs review
StatusFileSize
new272 bytes
new13.97 KB

Patch was prevented from applying due to the stuff in the info file added automatically by Drupal.org.

This should fix it.

jwilson3’s picture

StatusFileSize
new13.97 KB
new672 bytes

Looks like the constant FILTER_DEFAULT_FORMAT is actually FILTER_FORMAT_DEFAULT.

Status: Needs review » Needs work

The last submitted patch, 26: mimemail-mailsystem-1088914-26-D6.patch, failed testing.

jwilson3’s picture

Status: Needs work » Needs review
StatusFileSize
new0 bytes
new699 bytes

It looks like the check_markup functionality will strip the message body and replace it with 'n/a' if the email is triggered by an anonymous user, if anonymous users are not allowed to use the input format specified by the mimemail_format variable.

To fix this we should run through check_markup but don't do the access check (pass the third parameter FALSE).

Also, unrelated it looks like the test functionality was switched over to ci.drupal.org and tests are currently failing, but has nothing to do with this code.

jwilson3’s picture

StatusFileSize
new13.98 KB

Fixed the patch in 28 it was empty. The interdiff-26-28.txt is still relevant.

The last submitted patch, 28: mimemail-mailsystem-1088914-28-D6.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 29: mimemail-mailsystem-1088914-29-D6.patch, failed testing.

The last submitted patch, 29: mimemail-mailsystem-1088914-29-D6.patch, failed testing.

  • sgabe committed 29f952e on 8.x-1.x
    Issue #1088914 by pillarsdotnet: Use the Mail System module.
    
    
agorgoy’s picture

Hi I was trying to send email with attachments and when tried to install Mime Mail I got the following error.
Mime Mail requires Drupal version 6.29 or higher. (Currently using Drupal 6.22)
there is any way to get those modules working without update my drupal version ??

jwilson3’s picture

Sorry, I doubt you'll find an answer to your question, but why would you *not* update to the latest version of Drupal 6? There are known security holes in older versions of Drupal.

tr’s picture

Status: Needs work » Closed (won't fix)

Patches have already been committed to 8.x-1.x and 7.x-1.x for this issue.

Drupal 6 is no longer supported, for more than two years now.

Marking this D6 issue as won 't fix.