CSS comment/whitespace removal breaks rule

[GoogleCodeExporter]

Minify version:
PHP version: 2.1.2

What steps will reproduce the problem?
Consider the following css rule:
body{ background:#fff/*eef*/ url(/path/to/image.gif) repeat-y; }

Expected output:
body{background:#fffurl(/path/to/image.gif) repeat-y}

Actual output:
body{background:#fff url(/path/to/image.gif) repeat-y}
(not the space after the background colour)

The issue:
Spaces surrounding comments in the CSS are being removed along with the 
comments. This process should be independent.
1st - remove all comments
2nd - remove unnecessary spaces

Original issue reported on code.google.com by diego.a...@gmail.com on 23 Apr 2009 at 3:16

[GoogleCodeExporter]

Sorry, I meant to enter...
Minify version: 2.1.2
PHP version: 5
...but I don't think that will matter in this case.

Original comment by diego.a...@gmail.com on 23 Apr 2009 at 3:18

[GoogleCodeExporter]

I'll be able to look more at this over the weekend. In the meantime, open up 
this:
http://code.google.com/p/minify/source/browse/tags/release_2.1.2/min/lib/Minify/
CSS/
Compressor.php#83

Try changing: '@\\s*/\\*([\\s\\S]*?)\\*/\\s*@'

To: '@/\\*([\\s\\S]*?)\\*/@'

Original comment by mrclay....@gmail.com on 23 Apr 2009 at 3:55

• Changed title: CSS comment/whitespace removal breaks rule
• Changed state: Accepted

[GoogleCodeExporter]

Thanks, for the mean time I've just removed the comment - it's the most obvious 
easy 
solution - but of course, it's best to actually fix the issue when you can to 
stop it 
happening again as it breaks the css and can be tricky to identify the source 
of the 
problem.

Original comment by diego.a...@gmail.com on 23 Apr 2009 at 4:06

[GoogleCodeExporter]

Fix in R325. We lose a few bytes of compression here and there, but worth it to 
break less stuff.

Original comment by mrclay....@gmail.com on 1 May 2009 at 2:55

• Changed state: Verified
• Added labels: Release-2.1.2

[GoogleCodeExporter]

I see what you've done with the fix, but there shouldn't be any need to lose 
any 
bytes in compression, if you replace every comment with a space, then replace 
every 
sequence of spaces (ie.: \s+) with a single space. That way the compression 
will 
still be just as efficient but spaces will be left where required.

Yoy may be doing this already, but I couldn't tell from the changes in...
http://code.google.com/p/minify/source/detail?r=325

Original comment by diego.a...@gmail.com on 1 May 2009 at 6:56

[GoogleCodeExporter]

Replacing \s+ with " " led to Issue 49, and, while no user ran into that 
problem, 
I'd rather not break valid CSS strings. Other ideas?

Original comment by mrclay....@gmail.com on 1 May 2009 at 11:30

• Changed state: Fixed

[GoogleCodeExporter]

I have my own ASP/JS/CSS used in my CMS (although now I use minify for JS/CSS) 
and I 
have to overcome a similar issue. In my case, I was satisfied that my code will 
never 
require 2 consecutive spaces and where it does, I can simply change my code 
accordingly... I know that's not the case for minify so here is my suggestion.

I use a technique in my compressor that goes like this: before making any 
changes to 
the code, I remove blocks of the code that I do not wish to be affected.

So, in the case of minify, you could
1. identify string definitions and store them in an array
2. replace them with a place holder - some string pattern
3. compress, remove multiple white-spaces, etc...
4. find string definition place holders
5. replace them sequentially in the order they appear in the array

More specifically, I use this technique to highlight search terms on a HTML 
page. 
Rather than searching for "word" and replacing with "<strong>word</strong>" in 
small 
chunks of text as I build the HTML, I take the entire (finished) string and 
remove 
all HTML definitions and apply the highlighting using the steps described above.

I hope that makes sense - and perhaps help in some way...

Original comment by diego.a...@gmail.com on 1 May 2009 at 2:40

[GoogleCodeExporter]

The biggest hurdle I see is identifying string definitions. CSS has some wacky 
string syntax so I think it'd require building a tokenizer, but I haven't 
looked 
into it deeply. There are some other CSS min techniques I'd like to port before 
trying this:
http://regexadvice.com/blogs/mash/archive/2008/04/27/Update-to-CSS-Minification.
aspx

I'm going to mark this Verified and maybe open a separate ticket about 
whitespace 
later.

Original comment by mrclay....@gmail.com on 1 May 2009 at 5:42

• Changed state: Verified

[GoogleCodeExporter]

I agree. For the record, I think it would be a matter of hiding all 
"expression:" 
definitions and :\s*"[^"]" and :\s*'[^']'

I don't know much about gzip'ing, but I'm guessing it would take care of white 
space 
repetition anyway...

Those CSS min techniques are pretty cool and will be much more beneficial than 
this 
anyway...

Original comment by diego.a...@gmail.com on 1 May 2009 at 5:52