search

Heyvaert / wiquery

Automatically exported from code.google.com/p/wiquery
0 stars 0 forks source link

Wrong built URL in WiQueryMergedStyleSheetResourceReference #195

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. create a Wicket application with a non-root Wicket filter mapping 
(/myWicketApp/* for instance)
2. use a CSS file with a background: url("....")
3. activate WiQuery resource merging
4. the background: url("....") in the merged stylesheet file is wrong: it 
doesn't take into account the /myWicketApp/* (it's absolute)

What is the expected output? What do you see instead?
I expect to have a relative URL, which is always better than an absolute. If 
it's not possible, I expect a URL with /MyWicketApp/

What version of the product are you using? On what operating system?
1.2.3

Please provide any additional information below.
Here is a modification to the getCssUrl method in the class (sorry, I don't get 
time to make a patch on 1.2.3). The idea is to get a relative URL, using the 
original ResourceReference. The signature of getCssUrl changes to get a 
"baseReference" instead of a "baseUrl".
Worked for me.
/**
     * Convert local URL for the merging stylesheet( the url will be broken, so
     * we have to rewrite it !!)
     * @param url
     * @param baseReference the reference to the parent CSS file (containing the url("...") stuff)
     * @return
     */
    protected static String getCssUrl(String url, ResourceReference baseReference) {
        String cleaned = url.replace("'", "").replace("\"", "").trim();
        cleaned = cleaned.substring(4); // remove '('
        cleaned = cleaned.substring(0, cleaned.length() - 1); // remove ')'

        StringBuilder buffer = new StringBuilder();
        buffer.append("url(\"");

        if (UrlUtils.isRelative(cleaned)) {
            // the url is relative to the parent CSS, so we build its path from the parent
            String urlPath = new File(new File(baseReference.getName()).getParentFile(), cleaned).getPath();
            // we build a very new reference, so Wicket could give us the correct URL for the url("...") stuff
            ResourceReference urlReference = new ResourceReference(baseReference.getScope(), urlPath, baseReference.getLocale(), baseReference.getStyle());
            buffer.append(RequestCycle.get().urlFor(urlReference));
        } else {
            buffer.append(url);
        }

        buffer.append("\")");
        return buffer.toString();
    }

Original issue reported on code.google.com by guiom.mary@gmail.com on 27 Jun 2011 at 7:50

GoogleCodeExporter commented 9 years ago 
Hi,

Can you test with the last release, please ? (wiQuery-1.2.4). Because a patch 
was applied on the WiQueryMergedStyleSheetResourceReference

See issue 178.

Regards

Julien Roche

Original comment by roche....@gmail.com on 27 Jun 2011 at 7:53

GoogleCodeExporter commented 9 years ago 
I doubt the latest release correct this problem since the code which calculate 
the URL as absolute is still there (baseHost, getContextPath(), etc)

But I do it when I have time.

Original comment by guiom.mary@gmail.com on 27 Jun 2011 at 8:00

GoogleCodeExporter commented 9 years ago 
Hi guillaume,

I look on your method and I made some changes to fix some problems (bad urls if 
we put some internet link, of the cleaned url ...). I changed the unit tests to 
work with your method.

If you are agreed with the changes, I will apply it for the next release (1.2.5)

Here the method:
/**
     * Convert local URL for the merging stylesheet( the url will be broken, so
     * we have to rewrite it !!)
     * @param url
     * @param baseReference the reference to the parent CSS file (containing the url("...") stuff)
     * @return
     */
    protected static String getCssUrl(String url, ResourceReference baseReference) {
        String cleaned = url.replace(" ", "").replace("'", "").replace("\"", "");
        cleaned = cleaned.substring(4); // remove '('
        cleaned = cleaned.substring(0, cleaned.length() - 1); // remove ')'

        StringBuilder buffer = new StringBuilder();
        buffer.append("url(\"");

        if (UrlUtils.isRelative(cleaned)) {
            // the url is relative to the parent CSS, so we build its path from the parent
            String urlPath = new File(new File(baseReference.getName()).getParentFile(), cleaned).getPath();
            // we build a very new reference, so Wicket could give us the correct URL for the url("...") stuff
            ResourceReference urlReference = new ResourceReference(baseReference.getScope(), urlPath, baseReference.getLocale(), baseReference.getStyle());
            buffer.append(RequestCycle.get().urlFor(urlReference));

        } else {
            buffer.append(cleaned);
        }

        buffer.append("\")");
        return buffer.toString();
    }

And here the tests:
public class WiQueryMergedStyleSheetResourceReferenceTest extends
        WiQueryTestCase {
    /**
     * Test the
     * {@link WiQueryMergedStyleSheetResourceReference#getCssUrl(String, String) }
     */
    @Test
    public void testGetCssUrl() {
        ResourceReference baseReference = new ResourceReference("no_name");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(img.png)", baseReference),
                "url(\"resources/org.apache.wicket.Application/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(./img.png)", baseReference),
                "url(\"resources/org.apache.wicket.Application/.\\img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(../img.png)", baseReference),
                "url(\"resources/org.apache.wicket.Application/..\\img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(http://www.a.com/img.png)", baseReference),
                "url(\"http://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(https://www.a.com/img.png)", baseReference),
                "url(\"https://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(ftp://www.a.com/img.png)", baseReference),
                "url(\"ftp://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(file://img.png)", baseReference), "url(\"file://img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url('img.png')", baseReference),
                "url(\"resources/org.apache.wicket.Application/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url('./img.png')", baseReference),
                "url(\"resources/org.apache.wicket.Application/.\\img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url('../img.png')", baseReference),
                "url(\"resources/org.apache.wicket.Application/..\\img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url('http://www.a.com/img.png')", baseReference),
                "url(\"http://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url('https://www.a.com/img.png')", baseReference),
                "url(\"https://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url('ftp://www.a.com/img.png')", baseReference),
                "url(\"ftp://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url('file://img.png')", baseReference), "url(\"file://img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(\"img.png\")", baseReference),
                "url(\"resources/org.apache.wicket.Application/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(\"./img.png\")", baseReference),
                "url(\"resources/org.apache.wicket.Application/.\\img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(\"../img.png\")", baseReference),
                "url(\"resources/org.apache.wicket.Application/..\\img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(\"http://www.a.com/img.png\")", baseReference),
                "url(\"http://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(\"https://www.a.com/img.png\")", baseReference),
                "url(\"https://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(\"ftp://www.a.com/img.png\")", baseReference),
                "url(\"ftp://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url(\"file://img.png\")", baseReference), "url(\"file://img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "   url(   \"   img.png   \"   )   ", baseReference),
                "url(\"resources/org.apache.wicket.Application/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "   url(   \"./img.png\"   )  ", baseReference),
                "url(\"resources/org.apache.wicket.Application/.\\img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                " url( \"../img.png\"  )  ", baseReference),
                "url(\"resources/org.apache.wicket.Application/..\\img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                " url( \"http://www.a.com/img.png\"  )", baseReference),
                "url(\"http://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "  url   (\"https://www.a.com/img.png\"  )  ", baseReference),
                "url(\"https://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "   url (\"ftp://www.a.com/img.png\")", baseReference),
                "url(\"ftp://www.a.com/img.png\")");

        assertEquals(WiQueryMergedStyleSheetResourceReference.getCssUrl(
                "url     (\"file://img.png\")", baseReference),
                "url(\"file://img.png\")");
    }
}

Best regards

Julien

Original comment by roche....@gmail.com on 4 Jul 2011 at 7:01

GoogleCodeExporter commented 9 years ago 
Hi Julien,

If you use replace(" ", "") then you lose spaces in the name of the resource 
(the name of an image, for instance, "my logo.png"), that's why i prefered to 
use trim() which is less corruptive.
You should add a test for it.

All the rest is fine.

Original comment by guiom.mary@gmail.com on 5 Jul 2011 at 12:06

GoogleCodeExporter commented 9 years ago 
Hi,

The use of trim will be not work on some tests (for example: "  url   
(\"https://www.a.com/img.png\"  )  "). 

I will try to clean only what we need and add the code for the next release 
(and add your test too).

Regards

Original comment by roche....@gmail.com on 5 Jul 2011 at 12:21

GoogleCodeExporter commented 9 years ago

Original comment by hielke.hoeve on 16 Aug 2011 at 5:21

GoogleCodeExporter commented 9 years ago 
Hi Guiom,

Using spaces in filenames is very very bad practice, I think (as in wiquery 1.5 
merging is no longer supported) Juliens proposition is good enough.

Original comment by hielke.hoeve on 17 Aug 2011 at 7:26

GoogleCodeExporter commented 9 years ago 
Hi Hielke,

World is made of filenames with spaces, even if you think that's a bad practice 
(and it is a long debate ...), you should'nt disallow it.
That's maybe complicated for you, but I'm really convinced that Wiquery should 
support this kind of filename. For instance, we use Wiquery for an application 
where users can put their own CSS files in the system, upload necessary image 
files, etc... in order to have the application have the look they want. 
Disallow spaces in filenames means for us to disallow our customer to use 
spaces in their own image files. From a client point of view, it is not a good 
point. Go ahead and think that my customer ask their own designer to create a 
skin for our application. The designer must know that he's not allowed to use 
spaces, whereas they are usually artists, far from technical problems.
So, from our point of view, this is a bad news.

Original comment by guiom.mary@gmail.com on 17 Aug 2011 at 7:50

GoogleCodeExporter commented 9 years ago 
Ok, how then do you propose to check for spaces in the filenames while removing 
all the other spaces?

I committed Juliens proposition with a small change to account for spaces more 
specifically. Please review.

Original comment by hielke.hoeve on 18 Aug 2011 at 3:18

GoogleCodeExporter commented 9 years ago 
I wasn't proposing anything about checking for spaces in the filenames, since I 
think we should let them.
My suggestion was only about using the trim() method instead of replaceAll(" ", 
"") (see comment #4), and I see that's what you've done ! :)
Thanks.

Original comment by guiom.mary@gmail.com on 18 Aug 2011 at 4:35

GoogleCodeExporter commented 9 years ago

Original comment by hielke.hoeve on 29 Mar 2012 at 5:10