Closed (fixed)
Project:
Mime Mail
Version:
6.x-1.0-alpha1
Component:
Miscellaneous
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
15 Mar 2010 at 18:55 UTC
Updated:
1 Oct 2010 at 15:28 UTC
Jump to comment: Most recent file
I think Mime Mail is a great and very important module. Nowadays almost every site needs it's functionality, so we need to help the maintainers as much as we can to support it's development. In the last few days (heh...almost a week) I dedicated long hours to process the issue queue, filter out the duplicates, create new patches and review exist ones. For now I think I read almost every issue - at least in the 6.x branch - and we have some solutions to the most nerve-racking issues, without breaking anything.
I gathered these all together to create a potential new release. I made some modifications in order to the posted patches could work together. I have tried to provide patches individually by issues too, if you are interested you can check them out on the issue's page, but they are not the same as this.
For testing, I used a clean Drupal 6.16 installation with Simplenews 1.0 on a live site with multiple subscribers. I tested HTML and plain text e-mail with/without embedded images and attachments (with both of them too). If you would like to help with further testing, please contact me in a private message and you can use my test site, if you want.
The attached files:
- Patch applying to 6.x-1.0-alpha1.
- Zipped module with the patch applied.
The following issues are committed in the attachments:
Hopefully fixed bugs
- #567594: $mailkey is not properly set in drupal_mail_wrapper()
- #740856: Check if the file part is set in the body
- #698794: Attachment Content-Type-fix
- #567594: $mailkey is not properly set in drupal_mail_wrapper()
- #634210: Images missing from repeated mail message elements
- #448670: Spaces and Line Breaks are removed from CSS definitions
- #315239: Bad address syntax on Windows
- #642800: [Patch] Enforce requirement of PHP 5.x for mimemail_compress
- #127876: Plain text with/without attachment
- #304476: PHP Error when Stylesheets don't exist
- #456242: Use proper operators in if statements with strpos()
- #710116: Wrong implementation/namespace conflict of mimemail_prepare
- #448996: Wrong implementation of hook_mail_alter()
- #513138: Undefined variables in mimemail.inc
- #583920: Can't override mimemail.tpl.php
- #443964: Skip style sheets with print media
- #319229: src="Array" if path to image is broken
New features
- #629038: Attachements dont respect ‘list’ setting.
- #729658: Allow better integration with Domain Access module
- #685574: Optional site's css embedding
- #501722: HTML mail actions for Rules
- #319384: Add $mailkey to body tag as CSS class
Tasks
Please, do not open any new issue referring to this unofficial release! Search among the existing ones in the list and in the queue instead. I am pretty sure you will find something. However, any feedback and help is welcomed. If you find a bug, you can point to the corresponding issue in a comment and we can further investigate there. If you find something new, just post a comment here. We shall try to keep this issue traceable as possible.
I would really appreciate some feedback on this, it would be even better if this could be a new release someday.
Anyway, I will continue and share my progress on this here, according to the issues built-in.
| Comment | File | Size | Author |
|---|---|---|---|
| #31 | mimemail-6.x-1.0-beta1-07.patch | 63.79 KB | sgabe |
| #31 | mimemail-6.x-1.0-beta1-07.tgz | 18.12 KB | sgabe |
| #22 | mimemail-6.x-1.0-beta1-06.patch | 63.48 KB | sgabe |
| #22 | mimemail-6.x-1.0-beta1-06.tgz | 16.9 KB | sgabe |
| #18 | mimemail-6.x-1.0-beta1-05.patch | 56.28 KB | sgabe |
Comments
Comment #1
sgabe commentedAnd the attachments...
Comment #2
rkdesantos commentedExcellent... any chance you could roll this in too?
http://drupal.org/node/261028
I may try your beta with the above fix added and report back.
Comment #3
rkdesantos commentedTested module. Attempts to send e-mail on the site using contact pages fail with a WSOD. Nothing in the log to help.
Comment #4
sgabe commented@rkdesantos: I tried now with Contact module and I got a Call to undefined drupal_mail_wrapper() function error. Attaching new files which should fix that. I hope this will work for you too now.
Comment #5
rkdesantos commentedYep, looks like it is working now. Email went and no errors from the module I could detect. Tried it with the above return-path fix in and that worked, too.
However, I am getting another strange PHP warning:
# warning: include_once(sites/all/modules/flag/includes/flag.actions.inc) [function.include-once]: failed to open stream: No such file or directory in /home/coolsite/public_html/sites/all/modules/flag/flag.module on line 86.
# warning: include_once() [function.include]: Failed opening 'sites/all/modules/flag/includes/flag.actions.inc' for inclusion (include_path='.:/usr/lib/php:/usr/local/lib/php') in /home/coolsite/public_html/sites/all/modules/flag/flag.module on line 86.
This is the code in the flag module and line 86 is noted with the "<<"
Comment #6
sgabe commentedI don't think this error is Mime Mail related... Did you check the include path and the file permissions?
I will take a look on the issue you suggested and test it on safe_mode to be sure. If it's fine, I will include that too and update the attachments.
Comment #7
sgabe commentedIncluded #261028: SMTP Return-Path Setting as requested and follow up of #710116: Wrong implementation/namespace conflict of mimemail_prepare. Read the issues for more information.
Comment #8
rkdesantos commentedIf I remove the beta version of Mimemail and restore the older version the flag module PHP error goes away. Nothing else was changed. They are inter-related somehow I just don't know how yet. It could be we've exposed a flaw in the other module but why / where is the question.
Thanks for the latest update! Much appreciated.
Comment #9
sgabe commentedI installed Flag module, and did some testing. Seems fine to me. If you can dig up some information about this, maybe we could investigate further, but I don't think Mime Mail causes this error... Are you using this right now? I can see the error message permanently on your site.
Comment #10
rkdesantos commentedI'll keep investigating and may disable or reinstall the flag module. It isn't be used much so the loss isn't significant. Reading through issues for Flag there are quite a few having to do with how it does includes though none seemingly relevant. Thanks for looking into it.
Comment #11
rkdesantos commentedI've fixed the problem and it wasn't Mimemail at fault. Rather Mimemail just exposed the problem. There was a problem with a mislocated file and a PHP issue. All well now. Thanks for the help.
Comment #12
sgabe commentedMinor changes and fixes.
Comment #13
gregarios commentedIs there an official maintainer for this module? Fixes seem VERY rare and slow to implement, even when a fine patch is included like the one in post #12.
Thank you for your hard work sgabe. Will your patch apply to beta2? Maybe you should apply to become a co-maintainer for this?
Comment #14
plachsubscribe
Comment #15
sgabe commentedMime Mail has active maintainership, however the maintaining is not that simple as people may think, see #675950: Offering to maintain Mime Mail for more information. I totally agree with Allie in this question.
We can't commit anything without deep testing, because most patches and changes break almost as many things as they fix. I experienced this myself in some issues, see #372710: HTML emails are text-only in Hotmail or #583920: Can't override mimemail.tpl.php or #443964: Skip style sheets with print media for example, but we could continue the list.
In these cases the so called "solutions" handle just the symptoms for one particular issue and don't care about the side effects elsewhere. I created this issue and attached a bootleg patch to help test these changes and patches together, to see and fix the side effects.
The important thing what we need most is testing, testing again, and finally little more testing. I can assure you, this is very much for just one, two or three people. I spend hours everyday just sending e-mails to different addresses and reading them in different e-mail software just to make sure a fix is a fix indeed. This would be much easier with the contribution of the community.
I hope this will be useful for a new release, but I won't apply for co-maintainership because that wouldn't help a thing here. Instead, I would really like to see the referring issues here gone to green (RTBC). I'm sure in that case Allie would happily create a new release.
Please step in the process, review patches and give feedback!
Comment #16
dboulet commentedReally appreciate all the hard work sgabe, I will try to help out and review as many issues as I can.
Comment #17
gregarios commentedI guess this patch just needs to be updated to be applicable to beta2, then I'll apply it and test it for awhile as well.
Comment #18
sgabe commentedAlthough we have a new (security) release, still the patch will apply to 6.x-1.0-alpha1. The changes in alpha2 are very small, while my beta patch with all these fixes included is huge. It's much easier to commit those changes in my patch, than reversely.
The following have been integrated:
See the listed issues for more information. Any feedback is welcome.
Comment #19
rkdesantos commentedI'm seeing the following odd results when a test e-mail is sent with beta5:
with the title inserted after the "> in the body
Comment #20
sgabe commented@rkdesantos: Did you clear the caches? The orders of the parameters changed in the
theme()call, so you got the$subjectas$body. If you clear the caches, this should be fine. However, I discovered another bug in the mail sending alteration, so Mime Mail is not used for all messages, when it should be. I think I will update tomorrow with this fix.Comment #21
rkdesantos commentedDuh. Yep, that fixed it. Thanks.
Comment #22
sgabe commentedChanges:
Comment #23
roball commentedAny chance to get a comment to these efforts from an official maintainer?
Comment #24
perarnet commentedsgabe: I get errors when trying to apply this patch to alpha1
|diff -u -r1.1 mimemail_compress.info
|--- modules/mimemail_compress/mimemail_compress.info 22 Feb 2009 21:01:14 -0000 1.1
|+++ modules/mimemail_compress/mimemail_compress.info 28 Mar 2010 11:39:26 -0000
My folder lists:
-rw-r--r-- 1 foo foo 3766 Jul 16 2009 mimemail_compress.inc
-rw-r--r-- 1 foo foo 409 Oct 23 17:31 mimemail_compress.info
-rw-r--r-- 1 foo foo 572 Feb 23 2009 mimemail_compress.module
Comment #25
sgabe commented@perarnet: I think your mimemail_compress.info file is modified, so different than alpha1. Did you try with a clean alpha1 checked out from CVS?
Comment #26
sgabe commented@roball: I sent a message to Allie and asked her to take a look and leave a comment if she has some spare time. Though I can't say when we will hear from the maintainers, however we have a lot of issues with patches and workarounds waiting to be tested and confirmed, so there is no reason to sit back and wait for them to right themselves.
Comment #27
vatavale commented(subscribe)
Comment #28
muschpusch commentedGreat job! I will help testing the releases... Wouldn't it be a lot easier when someone get CVS access and commit the patches to a dev release?
Comment #29
sgabe commented@muschpusch: Thanks! I will update this issue soon with the latest progress.
Comment #30
ikeigenwijs commentedsubscribe
Comment #31
sgabe commentedAttaching the latest version with the following changes:
Please test, any feedback welcome!
Comment #32
gregarios commentedI can verify that the
mimemail-6.x-1.0-beta1-07.tgzsetup works perfectly well for me, and can verify that the #448670 issue is fixed. I cannot test for the #456260 or #768794 issues. Thanks for all your work, sgabe! :-)Comment #33
rkdesantos commentedLooking good here w/ beta1-07.
Comment #34
ikeigenwijs commentedSorry to inform
Included images with fck editor are still broken. 1 image is included and works
I coped the dir of mimemail-6.x-1.0-beta1-07.tgz 18.12 KB
and overwrote the running dir of mime
Is that the correct way if doig this, i did not do the incremental patches.
source of the newsletter the image
there is only 1 image included the last one coincidence?, all images are using the same dir.
please advice if i need to test something.
mail source:
Comment #35
sgabe commented@ikeigenwijs: Thanks for your feedback! However the source in your post is useless in this format. You should use code tags to post source code. Please send me a test message to this sgabe[at]freemail[dot]hu address.
Comment #36
sgabe commented@ikeigenwijs: Got your test message, thanks! Is this newsletter edition available on the web, somewhere I can take a look at the original source? I would like to try reproduce your message on my test environment.
Comment #37
ikeigenwijs commentedI ll publish it just for you ;-) the url is:
http://www.revaki.ugent.be/drupal_multi/?q=nl/test-nieuwsbrief/test-2-ei...
there are images in the header(simplenews template)
body witch you see only on the site,
footer(simplenews template)
Its the same with the real newsletter witch does not use the template and the header and footer
Comment #38
sgabe commented@ikeigenwijs: Thanks! ;) I think you are experiencing this #114312: Embedded images when Drupal is in a subdirectory issue. There is a patch in #23, would be awesome if you could test it and give feedback there. (Note: the patch won't apply, but you just have to modify one line in mimemail.inc.)
Comment #39
attiks commentedtesting
Comment #40
ikeigenwijs commentedI added the line, wher i think it belongs ;-)
But no joy. still no images.
We dont have the private file setting its just http.
I want to emphasis it does include 1 image , the last one of the newsletter.
Comment #41
ikeigenwijs commentedCan i do anything more?
Comment #42
ikeigenwijs commentedComment #43
sgabe commented@ikeigenwijs: I tried but I couldn't reproduce this. You should try to debug with the Devel module to see what's happening when Mime Mail creates the attachments. Print out the message and attachment variables in
_mimemail_file,mimemail_html_body, try to locate the problem.Please leave this issue in NR status. Your problem is not related with this experimental snapshot, something else causes that...
Comment #44
gooddesignusa commentedsubscribing. using this version and seems to make mime mail and simple news finally work correctly.
Comment #45
spidersilk commentedThanks so much for this - beta1-07 fixed this problem for me, and now MIME Mail and SimpleNews are playing nicely together again.
Comment #46
plachAny chance to address #729334: Flawed CSS to XPath conversion for class selectors in Mime Mail CSS Compressor and #417462: Language prefix is not taken into account here?
Comment #47
sgabe commented@plach: I have tested #417462: Language prefix is not taken into account. It looks fine and will be in the next unofficial beta release soon. I will take a look at #729334: Flawed CSS to XPath conversion for class selectors in Mime Mail CSS Compressor too, but can't promise.
Comment #48
plach@sgabe: Thanks! :)
Comment #49
gooddesignusa commentedwhen would all this be pushed to release or at least the dev release?
Comment #50
sgabe commented@gooddesignusa: I can't say. However, still would be great help if any of you could review some issues or confirm any rtbc issue. That would definitely bring us closer to a new release.
Comment #51
sgabe commentedTalked with Allie and she added me to the project as co-maintainer. Since we have some backlog, we agreed to roll in patches slowly and get them well tested by the larger community before we move forward. So, I think there will be at least 2-3 alpha release before we get to beta stage. In these alpha releases we should kill all existing bugs first, then we can add new features in beta stage.
Now I would like to focus on the RTBC bug reports we have and get them committed in the next alpha4 release at the end of the month. Then I would like to move toward the NR bug reports.
Would be really appreciated, if you could provide feedback about the committed and RTBC issues to confirm them as stable fixes and/or help test the issues which need review.
Comment #52
rkdesantos commentedGreat news @sgabe. Ive been running the last version here (07) for weeks now on 2 sites with no problems. So I consider the bugs corrected in it to be "stable" fixes for my purposes. Will be happy to test the first official "alpha" when it is posted.
Comment #53
doughold commentedI think I found a bug @ line 83 of mimemail.rules.inc, it should be:
mimemail_rules_action_mail_user(NULL, $settings);I got a call to undefined function mimemail_rules_action_mail_to_user before I changed it.
Comment #54
quicksketch@doughold you should file a separate issue for your bug.
Comment #55
sgabe commented@doughold: The beta7 is outdated (except the new features) compared to the alpha5 release or the HEAD branch. You should use alpha5 with the latest patch in #501722: HTML mail actions for Rules.
Comment #56
sgabe commentedI am closing this issue, since the module has almost caught up with itself.