Posted by Sutharsan on March 14, 2007 at 11:22pm
| Project: | Mime Mail |
| Version: | 6.x-1.0-alpha2 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Created the attached patch for adding attachments when plain text is selected.
| Attachment | Size |
|---|---|
| mimemail.inc_.plainattach.patch | 2.09 KB |
Comments
#1
I'm working on the cooperation between simplenews and mimemail. Further investigated the mimemail behaviour with plain text mails.
* Fixed bug in mimemail accepting the plain/html settings in newsletter, user mail preference and mimemail general settings. E-mails are now sent in plain text if any of the three is set to plain. (in mimemail.module)
* Mail body for mail with and without attachment corrected (in mimemail.inc).
#2
Southarson:
+1 the patch will allow attachment of files in newsletter which is a great feature.
@1 the patch was made to mimemail HEAD which isn't the best idea, IMO. I think if you tried patching a stable version of mimemail that will be great. I usually doubt the stability of HEAD, since it's where the developer commits changes thus it might be buggy.
Thanks for the great patch anyways.
#3
Here's an updated patch Mimemail version 1.0 patch which now handles correctly (hopefully without side-effects) the sending of plain text email with attachments *without* any HTML content.
Use this patch together with the updated Simplenews version 1.4 patch.
I also had to override the theme_mimemail_message function as follow (in template.php) :
function phptemplate_mimemail_message($body, $mailkey = null) {
return $body;
}
#4
Sutharsan,
I've commited the portion of this patch which allows for attachments when plain-text is selected.
If you could, please explain a bit further the other changes relating to user preference for plain text, and re-submit a patch just for that portion for review.
Thanks!!
Jer
#5
From reading my issue and the code I reconstruct the following:
* Whether or not to send plain text is determined by 3 parameters:
- Mime mail setting
- User setting
- Parameter in the mimemail() function call
If any of these is set to plain the mail should be plain
In the original code the user setting is ignored if mimemail()'s plain parameter is set (not NULL). My code is not good either, but I don't have the time now to rewrite. So I assign it to myself, but feel free to pick it up instead.
#6
Is this code submitted to 6.x as well? I notice a problem that html simplenews to the test email is sent with plain text content-type in the header. Could this be fixed too?
#7
@com2, make sure you use the latest 6.x-1.x-dev of simplenews. A number of fixes where committed last weeks in the html/plain handling.
#8
Sorry but the dev version of simplenews does not deliver at all in combination with mimemail. See this thread for information: http://drupal.org/node/153291#comment-1150871
#9
The latest 6.x version of the module sill has this problem. It sends out the html alternate content when the email is set to plain text and there is an attachment. I checked out the dev source code also and that too has this problem.
So what really happens is that when the user tries to send out plain text messages without HTML formatting the content gets squashed into one line because the mail has the HTML alternate part.
I was using the module along with simplenews module.
#10
We have prepared a patch to fix this issue for 6.x-1.0-alpha1 release. The same patch or the same strategy should work fine on the dev branch as well.
#11
I applied and tested zyxware's patch on 6.x-1.0-alpha1 release and it works fine. With the fix the plain text messages don't contain any HTML part, which reduces the message size too, so I think this is an important fix.
#13
When the message is plain text the
mimemail_multipart_body()call as function return is missing the correct content type parameter, which causes the header information shows up in the messages. I am attaching a revised patch, which fixes that.<?php...
if ($plaintext) {
// Plain mail without attachment
if (empty($attachments)) {
$content_type = 'text/plain';
...
}
// Plain mail has attachement
else {
$content_type = 'multipart/mixed';
...
}
}
// Include HTML versions of content only if plaintext is not explicitly set
else {
$content_type = 'multipart/alternative';
...
$content = mimemail_multipart_body($content, $content_type, TRUE);
...
}
...
return mimemail_multipart_body($parts, "$content_type; charset=utf-8");
}
?>
#14
...and marking as needs review.
#15
The patch works fine for me! Thanks for your work!
Because of the wrong line numbers in the patchfile itself (due to changes in mimemail.inc from alpha1 to alpha2) the patch process won't work with alpha2.
I made up a new patchfile for alpha2 which does exactly the same thing the mimemail.127876_01.patch does but with changed line numbers.
#16
Forgot to add the $content_type sry!
#17
@capulux: Thanks for testing and rerolling the patch against 6.x-1.0-alpha2, the attached patch applies and works just fine.
Marking this issue as RTBC.
#18
Patch works fine except one thing:
When you send some mails within a loop with the same attachment only the first mail will contain the attachment data.
You just forgot to set "$filenames = array();" at the end of _mimemail_file().
Patch includes #16
#19
#20
@haggins:
IWe didn't forget anything, that is another issue #358439: Images are only in the first message. Patch in #16 is still our solution and this issues is still RTBC. However, thanks for testing!#21
sgabe, is there any response by Allie Micka on the burst of activity last weeks?
#22
@Sutharsan: I couldn't reach Allie, but I have spoken to jerdavis on irc: They are aware of the last weeks events and not happy with the state of things either. They need to take some time to review the current state and patches and get another release put together. They wanted to get some of it in with the security release but time was to short. I have suggested some issues to take first place as most beneficial and well tested solutions and he mentioned the possibility of co-maintaining in the future. I think an important part of the work is done, it's only a matter of time now.
#23
Committed to HEAD
#24
Automatically closed -- issue fixed for 2 weeks with no activity.