press icon indicating copy to clipboard operation
press copied to clipboard

fix relative image path bug and Less support

Open benoit-ponsero opened this issue 14 years ago • 10 comments

Hello,

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

benoit-ponsero avatar Dec 09 '11 09:12 benoit-ponsero

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

dirkmc avatar Dec 10 '11 13:12 dirkmc

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

benoit-ponsero avatar Dec 12 '11 08:12 benoit-ponsero

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

dirkmc avatar Dec 12 '11 19:12 dirkmc

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

benoit-ponsero avatar Dec 13 '11 12:12 benoit-ponsero

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

dirkmc avatar Dec 13 '11 13:12 dirkmc

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

benoit-ponsero avatar Dec 13 '11 13:12 benoit-ponsero

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"

dirkmc avatar Dec 13 '11 13:12 dirkmc

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

as it should be.

benoit-ponsero avatar Dec 13 '11 14:12 benoit-ponsero

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;
    }
}

dirkmc avatar Dec 15 '11 15:12 dirkmc

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.

lnbogen avatar May 31 '12 07:05 lnbogen