Spaces and Line Breaks are removed from CSS definitions

I might also add, that it looks like the code your fix is applied to is meant to strip out multiple white spaces, but actually strips out all whitespaces. Maybe the code should be more like:
$variables['css'] = str_replace('  ',' ', str_replace("\n", '', $css));?

I'm bumping this to critical, then... since this is a show-stopper bug and a fix is in this post.

me fail sorry about that

Which solution is the "very first solution" you are referring to? "Multiple spaces" are not part of CSS in any way.

I opened a different case, namely: #581142: truncated mail due to css

My original issue was not css space removement, but much more mail truncation.

Please please read my post and understand that removing any newline is NO solution for SMTP due to RFC limitation of 998 bytes line length.

So finally i see no gain in removing a few bytes such as newlines and spaces, except introducing an incredible amount of potential problems. Instead mimemail should implement line length checks and add them manually or do base64 encoding...

Please simply switch this to the full unmodified css content.

Please simply switch this to the full unmodified css content.

I'm ALL for that! Please, Allie Micka, or one of the other maintainers...

Here's the patch for the latest dev release (2009-Jun-08):

@gregarios. Here you go. I can't test the patch myself for now. Could you please verify that this works? Thanks, Moritz

gregarios:
This issue is NOT about mimemail_compress. It's about wrong behaviour of plain mimemail.module - without mimemail_compress activated.

After having this basic functionality fixed you might think about enabling mimemail_compress (and also look into depth of much other strange things it does ;-) ) .. You don't need mimemail_compress. I strongly recommend you to add an own mail.css file and get rid of other css definitions that way.

*whew* Thanks for testing @gregarios

Alas, this has not been fixed in the newest version... and the patch in #16 would appear to not work on this version as well.

The patch needs to be rewritten to work on the same BUG CODE which has been moved to the mimemail.theme.inc file now. Disappointing.

Here we go @gregarios. For future reference: http://drupal.org/patch/create ;)

Patch works as advertised. Let's move this one in.

Patch for comment #27:

#24 works here too. nice work guys

using patch from comment #28 and work great

First of all, I'm not on my usual drupal working location so i'm unable to generate a patch here.

If you replace

I'm rephrasing this post:

This issue was in no way fixed in this new beta, and it seems inexcusable.

@alexanderpas, in answer to the next post:

Your patch is a great fix for this module, however, the number needs to be reduced to around 700 or so, despite the "standard" because indent spaces that are introduced can take up quite a few spaces before the CSS lines begin. No need to push the envelope anyway IMO.

I changed the 990 to 700 in your patch and it worked very well. 990 caused some lines to be truncated by the server, introducing a "!" into the line endings.

See issue http://drupal.org/node/743150 as many of these issues are being addressed in a bootleg patch.

The patch in #35 does fix the issue of too long lines, but hardly compresses the css file. We can do more to compress the file. Reduce multiple spaces (3, 4, 5, 6) to a single; Remove comments; remove unnecessary spaces. I found some articles which may be helpfull: http://websitetips.com/articles/optimization/css/crunch/ and http://www.lateralcode.com/css-minifier/

The patch in #35 does fix the issue of too long lines, but hardly compresses the css file.

I find it easier to just save the mail.css file the way I like it, in a readable-compressed format. Its not like it'll have to be a huge CSS file anyway. Compression can be left to the Mime Mail Compress module, right?

@gregarios: I second that, just like miro_dietiker wrote too. In here we should leave the mail.css as it is. If we agree, can we stick with patch from #24 and mark this as RTBC?

Absolutely.

@alexanderpas: The step-by-step variant works as advertised, but the second variant with preg_replace() doesn't handle every new lines.

However, if we don't use the CSS as it is, we could do more (remove comment and unnecessary spaces, etc.) according to the articles found by Sutharsan in #40.

I tested the attached patch with a style sheet formatted like disaster and the result was very nice.

@miro_dietiker: As I wrote before I agree with you. IMO: lets stick with the patch from #24 and when we finally have this (among other patches) committed in a new release, we can discuss this in another issue.

Some explanation about my choice of wordwrap and the space removal method.

As wordwrap only allows breaks on spaces etc., I decided to change, tabs and newlines into spaces, and only remove duplicate spaces, leaving a single space where used to be a spacing area giving wordwrap enough points where to wrap.

So, basically, the only way you could break the wordwrap step, is when there is a 700+ byte long string in your original CSS without spaces, on a single line.

Advanced methods, like those in #46 and the articles from #40 can, and shouldprobaly, be implemented in the CSS compressor module.

the patch in #24 is not good, since it replaces every single space with a single space, in addition to duplicate spaces, which is completely unneeded and causes slowdown.

reuploading the step-by-step variant of #45, as this seems to be the patch giving the best results, without advanced techniques.

The situation about memory and runtime is much worse than one might expect.

Look at this:
I've wrote some scripts. One setup for time/memory measurement and one script for each replace method.
Input is always a 10*1024*1024*' ,' (30M) sized text containing "space-space-comma" 10M times.
So finally 10M replacements are getting into place. The third is only to compare what happens if we use trivial str_replace which doesn't work for 2+ spaces.
A measurement script that gets run (in this case the first) looks like:

So the trivial preg match that matches also single spaces is faster in case of all-double-spaces. That's the cost of the more complex preg.
Now if we make 50% double spaces and 50% single spaces (5M times " , ,,"), see this:

Oops again. It gets almost the same speed - even if #1 has no more matches! So the regexp interpretation complexity is by far higher than the replacement overhead - if however something like that is measurable.

Interprete it on your own...

Ok, so in the case of normal emails going out, considering each email is going to have maybe 1/100th of those spaces, each email would save around .001 seconds? lol

Ok, I'm having issues with the reformatted CSS now in the emails. SOME email clients do NOT like that the CSS does not have a line break after each CSS entry. Removing the stock line breaks then adding line breaks at designated intervals is definitely NOT the way to go. The module needs to leave the CSS alone, so a person's mail.css file remains intact. Place a line-break after each semicolon in the CSS if you must have line breaks.

We need to go back to the way it was done in the patch from comment #24.

gregarios,
you've simply confirmed that the patch #24 we've considered RTBC was right.
There's no objection and even no need to reactivate the topic.

So, once again:
Back to #24, back to unchanged newlines, back to RTBC.

however, gregarios, can you please provide more details and exact examples about which clients don't accept multiple css statements on one single line? What we really should start is a data collection with information about mail clients and compatibility information. Sending quality mime mails is heavily related to best practice and incompatibility workarounds. Understanding the standards and strictly implementing them is by far not enough to be mass compatible...

Talked with sgabe about this a bit and I'd like to see us keep some basic optimization of css. The patch in #24 is simple enough, but parts of #46 seem like a more correct approach. This is a proposed compromise between the two.

This still performs some css optimization, based on the example provided by Drupal core's css aggregation functions:
http://api.drupal.org/api/function/drupal_load_stylesheet/6

It then also calls the wordwrap function in case there are still long lines from a default css file.

I'd like to commit this ASAP, so I'd appreciate any assistance with testing you can provide! This patch is rolled against HEAD.

#60 is confirmed to be working as advertised. Descendant selectors, child selectors and properties with multiple values are intact. Long lines wrapped at the given width. I can't see any problem.

I am confirming that some email clients, like some older versions of Outlook (there are about 10) do NOT like having missing line breaks before each CSS selector.

(As an example — width of code shortened for emphasis)
Some clients do not like:

While the same clients will show the CSS correctly if this is how the same CSS looks:

I say leave all line-breaks in. Break it further if you want the "RFC821" compliance, but please don't remove line breaks that are put into the "mail.css" manually.

Committed to HEAD, thanks sgabe, gregarios and everyone else!

gregarios,

Can you please re-verify this? I'm not seeing this in testing. Are you using a mail.css?

Jeremiah

Attached is my mail.css file. (with the .css extension replaced with .txt so you could read it from this thread as text.)

This is most definitely not fixed. :-\

Can't we just leave the CSS alone? Its not like the admin isn't the one who created it -- it shouldn't be 90000 characters long anyway!

I don't see how this will change anything. "Well Formatted" is a matter of opinion, too. I always do my CSS the way you see mine, and I can read it very well that way. So, you're saying that my CSS is to blame for this module stripping out all the line breaks?! Bah.

If the user don't use custom mail.css and the CSS aggregation is on (hopefully is), Drupal core will do this for us

I cannot use the aggregation, because I only need the 20+ lines of CSS that are in my emails, not the entire CSS file from my website. (over 700 lines, formatted the way I DO it -- it would be over 10,000 lines the way your "well formatted" idea is)

Ok, how about this: Have the function strip out ALL line-breaks of any kind. Then have it insert a line-break after every "}" in the CSS. Then have it put in a line-break after every 700 or so characters as a safety as well. Wouldn't that work?

The way I do my CSS is perfectly acceptable and is actually "well formatted" in some circles, too. Wouldn't you rather have compatibility, instead of forcing people to code CSS exactly like you want them too?

I guess I'm missing something here... isn't CSS aggregation the job of the "mimemail_compress" module? I don't use it, and my site's CSS are not included in the emails at all. I just want my mail.css file to be included with as little destruction of it as possible in the main module's emails. :-)

The patch to HEAD in comment #73 works perfectly. Thank you.

Here is what my original CSS looks like with it:

It has been committed.

hi,

where can I find a version of mimemail, where this patch or solution is included?

The only version I can find is
6.x-1.0-alpha2 Download (19.6 KB) 2010-Mar-24
and the patch above is from May 2010.

@DrupalFan2: There is a new alpha3 release - which includes this patch - listed on the project page.

Hallelujah! Thanks for your help!

I use this code

// Perform some safe CSS optimizations. (derived from core CSS aggregation)
// Regexp to match comment blocks.
$comment = '/\*[^*]*\*+(?:[^/*][^*]*\*+)*/';
// Regexp to match double quoted strings.
$double_quot = '"[^"\\\\]*(?:\\\\.[^"\\\\]*)*"';
// Regexp to match single quoted strings.
$single_quot = "'[^'\\\\]*(?:\\\\.[^'\\\\]*)*'";
$css = preg_replace(
"<($double_quot|$single_quot)|$comment>Sus", // Strip all comment blocks
"$1", // but keep double/single
$css); // quoted strings.
$css = preg_replace(
'<\s*([@{}:;,]|\)\s|\s\()\s*>S', // Remove whitespace around separators,
'\1', $css); // but keep space around parentheses.
// End the file with a new line.
$css .= "\n";

and my CSS code will have a clean view