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

Breakage when minifying at-import with unquoted urls containing semicolons #45

Closed
zytzagoo opened this issue Jan 11, 2018 · 3 comments
Closed
Assignees

Comments

@zytzagoo
Copy link

Here's a testcase that fails with current master:

@import url(//example.org/foo.css?a&b);
@import url(//example.org/bar.css);

Expected output:

@import url(//example.org/foo.css?a&b);@import url(//example.org/bar.css);

Actual output:

@import url(//example.org/foo.css?a&@import url(//example.org/bar.css);b);

Diff highlight:

Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'@import url(//example.org/foo.css?a&b);@import url(//example.org/bar.css);'
+'@import url(//example.org/foo.css?a&@import url(//example.org/bar.css);b);'

When urls are quoted, the problem doesn't occur (because processStringsCallback() captures and later restores the url "tokens").
However, I cannot control the inputs being given to Minifier.

Do you have any pointers on how to best approach fixing this?
Is this something you think needs fixing at all?

I have a branch ready for a PR over at my fork (with the test suite passing), as this is currently a showstopper for getting the latest version integrated into Autoptimize, however, there's probably a better/smarter way to go about fixing this -- I just needed to get something that works quickly...

@tubalmartin
Copy link
Owner

Thank you for reporting the issue extensively. I will review this for sure :)

@tubalmartin tubalmartin self-assigned this Jan 15, 2018
@tubalmartin
Copy link
Owner

Umm yes this is a valid issue. A semicolon is a reserved character in an URI and CSSmin should support it in import at rules. I'll fix this.

tubalmartin added a commit that referenced this issue Jan 15, 2018
@tubalmartin
Copy link
Owner

Fixed in v4.1.1

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

No branches or pull requests

2 participants