CSS aggregation attempts to process @import in comment
rfay - August 10, 2009 - 03:50
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Jump to:
Description
When CSS aggregation is turned on, the @import declaration of CSS is handled and processed.
However, the string "@import" within a comment or an @import without the appropriate argument is not handled correctly.
How to reproduce:
- Add a css file to your style (or anywhere) with a comment like /* try this with @import */
Turn on CSS aggregation- Examine the resultant aggregation using firebug or whatever technique
What I expect: The comment to be intact, along with the rest of the file.
What happened instead: The file was truncated starting with where the @ was in @import (after 'with' in my example).
My opinion is that we'll not be able to resolve all problems like this until we actually parse CSS in order to compress and aggregate it. Until that point, all aggregation and compression is hacky.
See #494498: Add CSS parsing capability to Drupal concerning building a CSS parser into Drupal.
#1
I should mention that this is not an innocuous bug... It not only replaces @import in the comment, but it eats the rest of tthe CSS file looking for the filename to import. Basically, any malformed @import, whether in the context of a comment or not, will destroy the semantic content of a large part of the file in which it occurs.
#2
Drupal's implementation for CSS aggregation totally sucks, see
http://drupal.org/node/448870#comment-2085438
Drupal should rather use a 3rd party lib like CSSTidy, instead of playing with overly simple regular expressions.
#3
See also #472820: drupal_load_stylesheet_content() removes whitespace improperly, creating invalid CSS, which will go into D7, IMO. A review with RTBC of that patch is probably in order. It only solves one of the many issues with CSS aggregation.
#4
This problem appears to have been corrected.
#5
I confirm that this seems to have been corrected.
#6
Automatically closed -- issue fixed for 2 weeks with no activity.