@Import issues #39

Closed
jhuriez opened this Issue Jan 29, 2013 · 13 comments

Projects

None yet

2 participants

@jhuriez

If i use Casset for CSS with "combine" => true, google fonts from @import are not loaded. Why ?

My line example :

@import url(http://fonts.googleapis.com/css?family=Open+Sans:400,700,300,400italic|Lobster);
@canton7
Owner

Probably a bug in the minification library. Have a look in the generated CSS file and see whether your @import is being mangled. I suspect it is. Try the latest version of CSSMin.php from https://github.com/mrclay/minify/blob/master/min/lib/CSSmin.php. If that fails, raise an issue with them. If not, I'll upgrade the version I'm using.

As a workaround, you can disable minification for that one file. In your call to Casset::css, pass the file again as the second param (e.g. Casset::css('myfile.css', 'myfile.css')), or, in your config:

...
'files' => array(
   array('myfile.css', 'myfile.css'),
),
...
@jhuriez

Ok i'll try that tomorrow,

But beware, it's work when i enable "minification" and disable "combine". It's only when i enable "combine" that doesn't work.
Even when I enable only the combination and disable the rest (inline, minification, etc..)

@canton7
Owner

Ah interesting! I wonder what's going wrong - combining is literally concatting the files together. Got a sample that's broken?

@jhuriez

Yes,

I have 2 files css :

"test1.css"

body {
    background: gray;
}

"test2.css"

@import url(http://fonts.googleapis.com/css?family=Open+Sans:400,700,300,400italic|Lobster);

h2 {
    font-family: Lobster;
}

PHP file :

<?php echo Casset::css('test1.css', false, 'css_test'); ?>
<?php echo Casset::css('test2.css', false, 'css_test'); ?>
<?php echo Casset::render_css('css_test'); ?>

Config casset.php file :

return array(
    'groups' => array(
        'css' => array(
            'css_test' => array(
                'files' => array(),
                'combine' => true,
                'min' => true,
                'inline' => false,
            ),
        ),
    )
);
  • If "combine" is false, and "min" is true : WORK
  • If "combine" is false, and "min" is false : WORK
  • If "combine" is true, and "min" is true : DOT NOT WORK
  • If "combine" is true, and "min" is false : DOT NOT WORK

It's just the combining is wrong.
If i do :

<?php echo Casset::css('test2.css', false, 'css_test'); ?>
<?php echo Casset::css('test1.css', false, 'css_test'); ?>
<?php echo Casset::render_css('css_test'); ?>

It's work. I think u need to place "@import" lines at the top of the file in the concates files procedure, i've right ?

@canton7
Owner

What's the result when those two test files are combined? Just want to make very sure nothing stupid is happening. I won't be near a computer with a php stack on it until this evening.

@canton7
Owner

Interestingly, looks like Assetic has the same issue: kriswallsmith/assetic#144

@jhuriez

Maybe use preg_replace_callback for get all line "@import" in static array. And at the end, implode the array at the top of the file ?

protected static $importLines = array();

preg_replace_callback('/@import+.*?\);/', array('TheClass', 'addImportLine'), $content);

protected static function addImportLine($m) {
            self::$importLines[] = $m[0];
            return '';
}

// At the end :
if (!empty(self::$importLines)) {
 $res = implode(PHP_EOL, self::$importLines) . $res;
}

return $res;

I will see that tomorrow

@jhuriez

Ok, i added these lines in "casset/classes/casset.php", in the function "combine", before the line "file_put_contents" :

                        // Import Lines
                        if ($type == 'css') {
                            preg_replace_callback('/@import+.*?\);/', array('Casset', 'addImportLine'), $content);
                            if (!empty(self::$importLines)) {
                                   $content = implode(PHP_EOL, self::$importLines) . PHP_EOL . $content;
                                   self::$importLines = array();
                            }
                        }

Add the array in static :

        public static $importLines = array();

And the function callback :

        protected static function addImportLine($m) {
            self::$importLines[] = $m[0];
            return '';
        }

It's work fine, but i don't know if it's a good practise ?

@canton7 canton7 added a commit that referenced this issue Feb 3, 2013
@canton7 Move CSS @import statements to top of combined file
Thanks to jhuriez for pointing this out and for the basis of this patch.

See #39.
0defa60
@canton7
Owner

Right! I finally have time to look at this properly. Thanks for being patient.

It looks like @import doesn't have to end with a ), so your regex is a little off. Since fuel requres PHP 5.3, we can use a lambda in the callback as well.

I've added a new commit to the develop branch - can you check it does what you expect? If so I'll merge it into master and cut a new release.

@canton7
Owner

I'm still not hugely happy with the regex, as it'll take a commented-out @import line and effectively uncomment it. That's probably rare enough that we can ignore it though... Maybe...

@jhuriez

Thank's for your commit, it's working fine.

Yep the uncommented @import line is an issue... We can delete comments before records @import lines, like csscompressor function (it's work if we set "minify" to true), but this is not really the purpose of the value "combine"

@canton7
Owner

Yeah, if we're minifying then comments are stripped by that point anyway, so it's a non-issue. If PHP's regex supports negative and positive lookbehinds it'll be easy enough to add, but I'm still not sure if it's worth it. I'll have a look at some point.

@canton7
Owner

OK, I've merged this into master and cut a new release. Thanks for your help!

@canton7 canton7 closed this Feb 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment