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

Minifying bootstrap 4 beta #8

Closed
warrenspe opened this issue Sep 19, 2017 · 9 comments
Closed

Minifying bootstrap 4 beta #8

warrenspe opened this issue Sep 19, 2017 · 9 comments

Comments

@warrenspe
Copy link

Minifying this particular bootstrap 4 beta css rule produces the following output:

.navbar-light .navbar-toggler-icon {
  background-image: url("data:image/svg+xml;charset=utf8,%3Csvg viewBox='0 0 30 30' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath stroke='rgba(0, 0, 0, 0.5)' stroke-width='2' stroke-linecap='round' stroke-miterlimit='10' d='M4 7h22M4 15h22M4 23h22'/%3E%3C/svg%3E");
}
.navbar-light .navbar-toggler-icon{background-image:url("data:image/svg+xml;charset=utf8,%3CsvgviewBox=\'003030\'xmlns=\'http://www.w3.org/2000/svg\'%3E%3Cpathstroke=\'rgba(0,0,0,0.5)\'stroke-width=\'2\'stroke-linecap=\'round\'stroke-miterlimit=\'10\'d=\'M47h22M415h22M423h22\'/%3E%3C/svg%3E")}

Which is incorrect. For some reason, the string quotes are not being respected and their contents are being minified.

@ndparker
Copy link
Owner

Hmm. we had a similar issue some time ago and more or less agreed that urls cannot contain spaces. They have to be escaped.

@warrenspe
Copy link
Author

That is generally a fair assumption; though from the example above it isn't necessarily true in all cases. Regardless of the url() function wrapper, I would have expected the string's contents to not to be minimized.

Isn't the biggest issue, but it does make minifying bootstrap using rcssmin a bit unwieldy, as modifying library code potentially makes it harder to upgrade in the future

@madebr
Copy link

madebr commented Nov 29, 2017

Me too agrees that spaces in urls have to be escaped.

And so they are in the bootstrap sample. They are enclosed between double quotes.

@ndparker
Copy link
Owner

@madebr It's not about CSS Syntax, it's about URL quoting: https://tools.ietf.org/html/rfc2397. These data-URLs are plain wrong.

@jsma
Copy link

jsma commented Jun 10, 2019

Browsers appear to support spaces in data URLs (as long as they are quoted strings of course). The original data URLs from Bootstrap 4 renders just fine and is what is used in the compiled version of Bootstrap available on their CDN.

rcssmin is taking a URL, which browsers parse just fine, and rendering an invalid data URL, which browsers can't parse at all. csscompressor has the same issue (there is a workaround in the linked issue).

I realize this tool exists to strip whitespace but in this case, rather than stripping whitespace, could rcssmin instead leave as-is or alternatively simply replace with %20? Is there a reason to actively strip whitespace inside URLs? If this were a linting tool I'd expect it to complain about conformity to spec but it's not obvious to me why a minifier tool should quietly and selectively enforce the spec while breaking browser-valid/tolerated CSS in the process.

@ndparker
Copy link
Owner

ndparker commented Jun 11, 2019

Yes, WS inside URLs are supposed to be strippable (so you can insert them for readability).

Said that, I started working on the issue: https://github.com/ndparker/rcssmin/tree/issue8
the C implementation is missing yet, though.

@qwenger
Copy link

qwenger commented Oct 20, 2020

Hi,

Has there been any progress on this issue?

@ndparker
Copy link
Owner

No :-(

@ndparker
Copy link
Owner

Fix released with 1.1.0.

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

5 participants