Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combiner does not work for absolute urls in css files #4002

Closed
ofriedrich opened this issue Feb 25, 2012 · 15 comments

Comments

@ofriedrich
Copy link

commented Feb 25, 2012

If you have urls like this in your css definitions:

url(/tl_files/images/myimage.png)

the combiner will replace this with

url("../../system/scripts//tl_files/images/myimage.png")

which does not work. The bug is in line 189 combine.php:

$content = preg_replace('/url("(?!(data:|https?://))/', 'url("../../' . $strGlue, $content);

which should be replaced by

$content = preg_replace('/url("(?!(data:|https?://|/))/', 'url("../../' . $strGlue, $content);

@leofeyer

This comment has been minimized.

Copy link
Member

commented Feb 27, 2012

How do I reproduce?

@ofriedrich

This comment has been minimized.

Copy link
Author

commented Feb 27, 2012

Try this in CSS-File:
background: url(/tl_files/images/myimage.png);
Import CSS-File
Definition is ok
Append this CSS-File to a layout
Look at the combined CSS-File
Definition is wrong

@ofriedrich

This comment has been minimized.

Copy link
Author

commented Feb 28, 2012

The solution is quiet easy: Your regular expression replaces, if url starts with "data:", "http://" or "https://". But if an url starts with "/", the slash will be replaced by "../../system/scripts//". This is wrong. So my regular expression also considers the "/" at the beginning of an url as an expression, where nothing will be replaced.

@DyaGa

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2012

I also have some troubles with this.

Here is a structure of assets files:

assets\
   css\
       file.css
   images\
       image.png

1. Combiner adds an extra backslash symbol to relative path

Rule In file.css:

backgroud-image: url("../images/image.png")

And Combiner returns this:

file system/scripts/a17954b71423.css

background-image:url("../..//assets/images/image.png")

2. Combiner doesn't understand rules without the quotes

in file.css

backgroud-image: url(../images/image.png)

Combiner doesn't understand and do nothing with this:

in system/scripts/0c707ddfd124.css:

backgroud-image: url(../images/image.png)

so, in this case image not loaded.

@DyaGa

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2012

3. Combiner doesn't understand rules with single quotes

if in file.css we have:

backgroud-image: url('../images/image.png')

Combiner also will do nothing with this:

in system/scripts/f3f18ba5d331.css

backgroud-image: url('../images/image.png')
@ofriedrich

This comment has been minimized.

Copy link
Author

commented Feb 28, 2012

Hello DyaGa,

if you import your css-file into contao, all urls are generated with double quotes. So it looks like, the combiner only works with double quoted urls. I had a look at w3 and found out, that quotes oder double quotes are optional. It seems to be a good idea, to make the combiner work with or without quotes. This is the one thing.

The other thing is, that the detection of replacements within the url doesn't do, what it should do.

I'll have a look at the source code.

@DyaGa

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2012

Hi. I agree.

And import css into a Contao is not convenient solution. Combiner should understand also single quotes and rules without quotes.

@DyaGa

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2012

4. Combiner doesn't understand Microsoft filters src property

Example of rule taken from Fancybox jQuery plugin (version 1.3.4, http://fancybox.net/):

.fancybox-ie #fancybox-bg-n { filter: progid:DXImageTransform.Microsoft.AlphaImageLoader(src='fancybox/fancy_shadow_n.png', sizingMethod='scale'); }
@ofriedrich

This comment has been minimized.

Copy link
Author

commented Feb 28, 2012

Hi DyaGa,

This is a completely different thing. The src attribute in this old styled M$ filter is not relative to the css file, but relative to the site url! As the combiner cannot know the site url, the src attribute cannot be replaced.

For example:
site url: http://mydomain.com/mysite.html => src='fancybox/fancy_shadow_n.png'
site url: http://mydomain.com/en/mysite.html => src='../fancybox/fancy_shadow_n.png'

So you'll never get this working within combined css files.

@DyaGa

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2012

Yes, it is not a relative url. But Combiner could do this:

for example, we have Fancybox plugin files in plugins folder:

plugins\
   fancybox\
      jquery.fancybox-1.3.4.css

in this case the MS rule:

src='fancybox/fancy_shadow_n.png'

should be changed with Combiner to this:

src='http://mydomain.com/plugins/fancybox/fancy_shadow_n.png'

or to this:

src='http://mydomain.com/system/scripts/../../plugins/fancybox/fancy_shadow_n.png'
@ofriedrich

This comment has been minimized.

Copy link
Author

commented Feb 28, 2012

As the template can be used for different domains, this cannot be done.

@DyaGa

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2012

so, in this case for domain anotherdomain.com the image will be loaded from mydomain.com

@ofriedrich

This comment has been minimized.

Copy link
Author

commented Feb 28, 2012

And this will cause problems, when you are working with https => crossdomain!

@DyaGa

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2012

Ок, maybe.

I have not tried to run sites under https. So, I agree, there could be problems with this. It should be checked.

@leofeyer

This comment has been minimized.

Copy link
Member

commented Mar 2, 2012

Fixed in 59172c3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.