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

fix relative image path bug and Less support #22

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

benoit-ponsero
Copy link

Hello,

rewrite url() or src= to set the correct path when css are in different folders that the css generated.

@dirkmc
Copy link
Owner

dirkmc commented Dec 10, 2011

Hi Benoit,
Thanks for your interest in the plugin. I'll take a look at your pull request next week.
One thing I would suggest is to use play.libs.IO.readContentAsString() to read the contents of a less file from a stream. It should perform better than reading one byte at a time.
Also it looks like there's an extra file checked in - commands.pyc
Thanks again,
Dirk

@benoit-ponsero
Copy link
Author

Hello,

i change to read the full line whereas one byte.

I don't know why commands.pyc was committed. Maybe it was updated when i run build-module. You can ignore this file

@dirkmc
Copy link
Owner

dirkmc commented Dec 12, 2011

Hi Benoit,
I tried out your pull request. The relative path code doesn't seem to work for paths that don't have a .. in them. For example if I have a file called "/public/stylesheets/m/mytest.css" with the following line:
url(m/test.png)
it becomes
url(/public/stylesheets/m/mytest.css/m/test.png)

Could you please fix this code and include a set of tests that demonstrate that it works for all scenarios?
Thanks,
Dirk

@benoit-ponsero
Copy link
Author

hello,

i setup a test application with different test case, you can get it at : https://github.com/benoit-ponsero/testpress
just store this app in same folder than the forked press module, because i added in application.conf :

module.press=../press

@dirkmc
Copy link
Owner

dirkmc commented Dec 13, 2011

Ok thanks. Could you also push your code so that I can see the fix

@benoit-ponsero
Copy link
Author

i dit not fix anything. i think you missconfigured your url in css.

@dirkmc
Copy link
Owner

dirkmc commented Dec 13, 2011

I think maybe there's a bug in the repath method:

private static String repath(String cssFileUrl, String url) {
    String[] parts = url.split("\\.\\./");
    int len = parts.length;
    String finalurl = "";
    String[] urlparts = cssFileUrl.split("/");
    for (int n = 0; n < urlparts.length - len; n++) {
        finalurl += urlparts[n] + "/";
    }
    finalurl += parts[len - 1];
    return finalurl;
}

It looks to me like it takes the part of the css file path up to the last slash, and appends the part of the url after the last ../

So for example:
repath("/web/public/css/mobile/default/style.css", "../../../images/sprite.png")
will result in
"/web/public/css/mobile/default/images/sprite.png"
when it should actually be
"/web/public/images/sprite.png"

@benoit-ponsero
Copy link
Author

i just test it and the result is : /web/public/images/sprite.png

as it should be.

@dirkmc
Copy link
Owner

dirkmc commented Dec 15, 2011

Ok I think it's almost right. The only problem is when you have things like
url(../somedir/../sprite.png)
I wrote a class that you can use to test it:

public class Test {
    public static void main(String[] args) {
        String[][] tests = {
            {"/web/myproj/public/stylesheets/test.css", "test.png", "/web/myproj/public/stylesheets/test.png"},
            {"/web/myproj/public/stylesheets/test.css", "../up.png", "/web/myproj/public/up.png"},
            {"/web/myproj/public/stylesheets/test.css", "../../up2.png", "/web/myproj/up2.png"},
            {"/web/myproj/public/stylesheets/test.css", "../../../up3.png", "/web/up3.png"},
            {"/web/myproj/public/stylesheets/test.css", "../somedir/../../level1.png", "/web/myproj/public/level1.png"},
            {"/web/myproj/public/stylesheets/test.css", "../../somedir/../level2.png", "/web/myproj/level2.png"},
            {"/web/myproj/public/stylesheets/test.css", "../../somedir/../somedir/../level2.png", "/web/myproj/level2.png"},
            {"/web/myproj/public/stylesheets/test.css", "subdir/testsubdir.png", "/web/myproj/public/stylesheets/subdir/testsubdir.png"},
            {"/web/myproj/public/stylesheets/test.css", "subdir/subdir2/testsubdir2.png", "/web/myproj/public/stylesheets/subdir/subdir2/testsubdir2.png"},
            {"/web/myproj/public/stylesheets/test.css", "subdir/subdir2/../subdir2up.png", "/web/myproj/public/stylesheets/subdir/subdir2up.png"},
            {"/web/myproj/public/stylesheets/test.css", "subdir/subdir2/../../subdir2up2.png", "/web/myproj/public/stylesheets/subdir2up2.png"},
            {"/web/myproj/public/stylesheets/test.css", "subdir/subdir2/../../../subdir2up3.png", "/web/myproj/public/subdir2up3.png"},
        };

        for (String[] test : tests) {
            String cssFile = test[0];
            String url = test[1];
            String expected = test[2];
            String result = repath(cssFile, url);
            System.out.println("css: " + cssFile);
            System.out.println("url: " + url);
            System.out.println("=>  " + result);
            if(expected.equals(result)) {
                System.out.println("[OK]\n");
            } else {
                System.out.println("exp: " + expected);
                System.out.println("[Failed]\n");
            }
        }
    }

    private static String repath(String cssFileUrl, String url) {
        String[] parts = url.split("\\.\\./");
        int len = parts.length;
        String finalurl = "";
        String[] urlparts = cssFileUrl.split("/");
        for (int n = 0; n < urlparts.length - len; n++) {
            finalurl += urlparts[n] + "/";
        }
        finalurl += parts[len - 1];
        return finalurl;
    }
}

@lnbogen
Copy link

lnbogen commented May 31, 2012

I would love to see these changes merged to master. I'm having conflicts with less 0.9.1 and there is no way I'm giving up .less usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants