I'm working on a custom accounting module for a client, and the client wants to be able to email out the same statements that appear on the web site. The problem is that template_preprocess_mimemail_message doesn't include module CSS files in the email message, only theme CSS files. That means that the statement isn't formatted correctly.
I've discovered that simply commenting out the $type == 'theme' test in template_preprocess_mimemail_message is enough to fix the problem.
So I'm wondering why mimemail is intentionally excluding these CSS files. Unless there is some other mechanism for a module to include a custom CSS file, it seems they should be included.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | mimemail_1175378_05.patch | 2.81 KB | sgabe |
| #5 | mimemail_1175378_04.patch | 2.76 KB | sgabe |
| #4 | mimemail.patch | 2.16 KB | samalone |
| #3 | mimemail.patch | 2.16 KB | samalone |
| #1 | mimemail_1175378_01.patch | 593 bytes | sgabe |
Comments
Comment #1
sgabe commentedBecause otherwise the style sheet would be way too huge - not to mention absolutely needlessly in 99% - to be included in the message. I would propose to add a "mail" media type to the statement. This is not a new idea anyway.
Comment #2
sgabe commentedChanging status.
Comment #3
samalone commentedOK, I buy the argument that automatically including all module CSS files is overkill.
But I think that if a module goes to the trouble of explicitly registering a "mail" CSS file, it should be included before theme CSS files (since theme styles can traditionally override module styles), and should be included even if the theme has a mail.css file.
So here is an alternative patch that has these features. It adds a helper function and converts the $styles variable from a newline delimited string to an array, so that styles can more easily be added in the proper order.
Comment #4
samalone commentedOops, my previous patch was testing $type rather than $media like yours did.
I'm also thinking that we might want to use the media type "email" rather than "mail", since "email" is being used as a media type by MailChimp:
http://blog.mailchimp.com/tag/css-email-support/
I've revised my patch with these changes.
Comment #5
sgabe commentedI would rather not needlessly duplicate code. Please, name your patches according to the issue, see Submitting patches in the handbook.
Comment #6
samalone commentedThanks for the link to the Submitting patches page. I'll get that right next time.
Your patch #04 works for me and solves my problem.
Comment #7
sgabe commentedFixed support for Fusion based themes and other minor changes. Looks good to me, but needs testing.
Comment #8
sgabe commentedCommitted, thanks samalone!
Comment #9
sgabe commented