There are just too many issues when trying to aggregate CSS with regular expressions. As already raised in #494498: Add CSS parsing capability to Drupal we need a real CSS parser!
As we are already in code freeze this probably won't happen in drupal 7 (When this patch however goes in I am happy to work real hard on this one). So we need to fix what is fixable in the current approach of CSS aggregation. And we need to make it as easy as possible to replace core CSS aggregation with a contrib module, which may result in a better version.
I suggest we move current aggregation into a seperate module.
* There already is a hook_css_alter(), so code in common.inc only gets cleaner.
* For users nothing changes but they have to enable the module.
* Performance wise I think this won't make much difference, it may even be faster.
* CSS aggregation isn't really a core task and shouldn't be in includes/common.inc. drupal_add_css() and drupal_get_css() are the only functions belonging there.
So to properly fix all issues involving CSS aggregation, it needs to be moved into a seperate module.
Patch contains a working css.module, check it out. I already changed some of the logic to allow better testing and added some extra tests.
Again, when this patch goes in I am happy to work real hard on a real parser to get killer CSS aggregation.
Comment | File | Size | Author |
---|---|---|---|
css_module.patch | 66.2 KB | casey | |
Comments
Comment #1
casey CreditAttribution: casey commentedProbably too late to get this in anyway, so I created a contrib module for this: CSS Preprocessor. Maybe Drupal 8...
Comment #2
casey CreditAttribution: casey commentedComment #3
casey CreditAttribution: casey commentedComment #4
catchThis is a good idea, we need to stop cramming stuff in common.inc when it's not needed. Tagging but patch will no longer apply.
Comment #5
Spelling Ninja CreditAttribution: Spelling Ninja commentedComment #6
JacineNice. Subscribe!
Comment #7
catchI'm wondering a bit if this couldn't be a class in /includes rather than a module - closer to cache.inc or lock.inc. We could then make it swappable. That's just a thought though - as long as we move it somewhere out of common.inc, that will make it easier to move somewhere else at a later date too if we ever wanted to.
Comment #8
Wim LeersInstead of making it a module, it just became pluggable: #352951: Make JS & CSS Preprocessing Pluggable. :)