google-code-export / wiquery

Automatically exported from code.google.com/p/wiquery
MIT License
1 stars 1 forks source link

Single line JavaScript comments are striped out in CSS-Files #128

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?

1. In a CSS file use e.g.
background: url(http://example.com/img.png)
(or use the included fusion theme)
2. The expression will be replaced with
background: url(http:

What is the expected output? What do you see instead?
// shouldn't be striped out.

What version of the product are you using? On what operating system?
1.1 and 1.1.1-SNAPSHOT

Problem is the double slash, which starts a comment in JavaScript. But in a CSS 
file only /* comments */ are allowed.

Original issue reported on code.google.com by czap83@gmail.com on 11 Nov 2010 at 6:26

GoogleCodeExporter commented 9 years ago
Hi,

Can you give us an eclipse project with a sample of your problme please ? What 
kind of options did you use ? Have you enabled the wiquery merging option ?

Thank you

Regards

Julien Roche

Original comment by roche....@gmail.com on 12 Nov 2010 at 7:41

GoogleCodeExporter commented 9 years ago
Hi Julien,

well...I'm a NetBeans user. :)

But I hope the attached maven quickstart project demonstrates the issue.

I've enabled resource merging, minified resources and set fusion as theme. On 
the page i've added a dialog.

A generated line in the CSS file looks like that:

.ui-widget-content { border: 1px solid #aaaaaa; background: #ffffff 
url(http:.ui-widget-content a { color: #222222; }

Hope it helps. :)

cheers,
Witi

Original comment by czap83@gmail.com on 12 Nov 2010 at 9:18

Attachments:

GoogleCodeExporter commented 9 years ago
Ok, I will look on this problem.

If you disable the merging option, does it work ?

Regards

Julien Roche

Original comment by roche....@gmail.com on 13 Nov 2010 at 3:03

GoogleCodeExporter commented 9 years ago
Hi Witi,

That's really odd, because I can't reproduce your problem (even 
enabling/disabling merging and/or compressing option).

I change your quickstart example to put a css with 
"url(http://example.com/img.png)
" and each time, I've got the right url. 

I put too a screenshoot of firebug with the result (and we can see that we use 
the merging option with the name of the stylesheet).

I used Windows XP pro with Eclipse Helios, jdk 1.6 and tomcat 6.

Can you send me your netbeans project (and netbeans version), I will try to 
install it and test it the next week.

Regards

Julien Roche

Original comment by roche....@gmail.com on 13 Nov 2010 at 4:11

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Julian,

You have to run it in deployment mode. In development mode I have the same 
output as you. But in deployment mode your example is even nicer, the whole 
line is gone. :)

cheers,
Witi

Original comment by czap83@gmail.com on 15 Nov 2010 at 6:59

Attachments:

GoogleCodeExporter commented 9 years ago
Sorry...I meant "Hi Julien". ;)

Original comment by czap83@gmail.com on 15 Nov 2010 at 7:00

GoogleCodeExporter commented 9 years ago
Interresting.

I will look on this problem.

Thank you very much Witi !

Cheers

Julien Roche

Original comment by roche....@gmail.com on 16 Nov 2010 at 7:47

GoogleCodeExporter commented 9 years ago
Hi Witi,

I have found the problem. We must use quotes when we compress stylesheets. 
Otherwise, we will have the bug that you found. I fix it into the r559

I have done some changes into the org.odlabs.wiquery.core.commons.compressed to 
external the Hielke's IResourceStream for the merging process. you can retrieve 
the current source code from the trunk and test it into your applications.

Many thanks for your feedbacks.

Regards

Julien Roche

Original comment by roche....@gmail.com on 17 Nov 2010 at 10:57

GoogleCodeExporter commented 9 years ago
Julien, your patch broke the tests. Did you forget to run the tests before 
committing?

Original comment by hielke.hoeve on 18 Nov 2010 at 8:40

GoogleCodeExporter commented 9 years ago
Gasp, yes, I forgot it !!

I will fix these tonight.

Sorry for that....

Original comment by roche....@gmail.com on 18 Nov 2010 at 8:42

GoogleCodeExporter commented 9 years ago
The new unit test:

public class WiQueryMergedStyleSheetResourceReferenceTest extends TestCase {
    /**
     * Test the {@link WiQueryMergedStyleSheetResourceReference#getCssUrl(String, String) }
     */
    @Test
    public void testGetCssUrl() {
        String baseUrl = "http://localhost/test/";

        Assert.assertEquals(
                WiQueryMergedStyleSheetResourceReference.getCssUrl("url(img.png)", baseUrl), 
                "url(\"http://localhost/test/img.png\")");

        Assert.assertEquals(
                WiQueryMergedStyleSheetResourceReference.getCssUrl("url(./img.png)", baseUrl), 
                "url(\"http://localhost/test/./img.png\")");

        Assert.assertEquals(
                WiQueryMergedStyleSheetResourceReference.getCssUrl("url(../img.png)", baseUrl), 
                "url(\"http://localhost/test/../img.png\")");

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

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

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

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

Original comment by roche....@gmail.com on 18 Nov 2010 at 8:45

GoogleCodeExporter commented 9 years ago
No problem, I have set our googlegroups to be emailed when our companies CI has 
build wiquery, dont know if it can email to the big bad outside world but we'll 
see.

Hielke

Original comment by hielke.hoeve on 18 Nov 2010 at 8:45