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

data: URIs containing spaces break some CSS minifiers #25211

Closed
jameysharp opened this issue Jan 5, 2018 · 5 comments
Closed

data: URIs containing spaces break some CSS minifiers #25211

jameysharp opened this issue Jan 5, 2018 · 5 comments
Labels

Comments

@jameysharp
Copy link

As best I understand, this is not strictly a bug in Bootstrap, but I think you can work around it and make deployment easier for people without hurting anything.

The navbar-toggler-icon background-image is a url() with a data: URI that contains space characters. At least rCSSmin, which is the default minifier for django-compressor, assumes that url() can't contain spaces, according to ndparker/rcssmin#8.

If you wrap variables such as $navbar-dark-toggler-icon-bg in an additional str-replace(..., " ", "%20") then these minifiers work fine.

Since these variables are only used once each this isn't a big change to the generated CSS but makes Bootstrap easier to deploy, so I hope you'll consider making this change.

@XhmikosR
Copy link
Member

XhmikosR commented Jan 9, 2018

I don't get why the minifier you mention doesn't follow the specs.

If it's valid, the minifier should be fixed.

@wendt88
Copy link

wendt88 commented Jan 26, 2018

can anyone help me to set up an other (working) css loader/minifier in webpack, to get a working minified build?

my sass config is

            {
                test: /\.s[ac]ss$/,
                loader: ExtractTextPlugin.extract({
                    use: [
                        'css-loader',
                        {
                            loader: 'postcss-loader',
                            options: {
                                plugins: (loader) => [
                                    require('autoprefixer')()
                                ]
                            }
                        },
                        'resolve-url-loader',
                        'sass-loader?sourceMap'
                    ],
                    fallback: 'style-loader'
                })
            }

and I added a plugin to specify the loader options:

       new webpack.LoaderOptionsPlugin({
            minimize: isProduction,
            options: {
                context: __dirname,
                output: { path: './' }
            }
        })

or should I oberride the $navbar-dark-toggler-icon-bg variables?

@jameysharp
Copy link
Author

I haven't used webpack, but I found that clean-css is the minifier that the official Bootstrap distribution is built with, so if you can figure out how to make webpack use both postcss and clean-css you should be set. I can't help you do that though since my workflow is completely different.

Together with roots/sage#2017 it seems there's evidence that this is a pain point for people trying to use Bootstrap 4, so I still think it's worth including in upstream Bootstrap the small workaround of replacing spaces with %20 in these data: URIs, even if the minifiers are technically wrong here.

@sandrowuermli
Copy link

sandrowuermli commented Feb 18, 2018

Some additional ideas from webpack with roots/sage:

A 'Clean' Workaround

TL;DR

1.
In Your assets/styles/main.scss insert at the top of the file (yes, before @import "common/variables";, we need the functions for str-replace()):

@import "~bootstrap/scss/functions";
@import "~bootstrap/scss/variables";

2.
In Your styles/common/_variables.scss insert:

$navbar-dark-toggler-icon-bg:       url("data:image/svg+xml;charset=utf8,%3Csvg viewBox='0 0 30 30' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath stroke='#{str-replace(str-replace(#{$navbar-dark-color}, "(", "%28"), ")", "%29")}' stroke-width='2' stroke-linecap='round' stroke-miterlimit='10' d='M4 7h22M4 15h22M4 23h22'/%3E%3C/svg%3E");
$navbar-light-toggler-icon-bg:      url("data:image/svg+xml;charset=utf8,%3Csvg viewBox='0 0 30 30' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath stroke='#{str-replace(str-replace(#{$navbar-light-color}, "(", "%28"), ")", "%29")}' stroke-width='2' stroke-linecap='round' stroke-miterlimit='10' d='M4 7h22M4 15h22M4 23h22'/%3E%3C/svg%3E");

Detail

The Problem aren't the spaces. The brackets from the stroke='#{$navbar-dark-color}' causes the problem. So if we str-replace() the ( with %28 and the ) with %29 you can run yarn run build:production without removing your scss.

Reference:
Bootstrap 4 CSS error when minifying with webpack, causes loss of SCSS running build:production

@kennyeliason
Copy link

I know I know... who uses stylelint... but my OCD loves it. If you want this little patch to work and still validate with your stylelint, this is what you need to be putting in your /common/_variables.scss file:

$new-navbar-dark-color: str-replace(str-replace(#{$navbar-dark-color}, "(", "%28"), ")", "%29");
$new-navbar-light-color: str-replace(str-replace(#{$navbar-light-color}, "(", "%28"), ")", "%29");

$navbar-dark-toggler-icon-bg: url("data:image/svg+xml;charset=utf8,%3Csvg viewBox='0 0 30 30' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath stroke='#{$new-navbar-dark-color}' stroke-width='2' stroke-linecap='round' stroke-miterlimit='10' d='M4 7h22M4 15h22M4 23h22'/%3E%3C/svg%3E");
$navbar-light-toggler-icon-bg: url("data:image/svg+xml;charset=utf8,%3Csvg viewBox='0 0 30 30' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath stroke='#{$new-navbar-light-color}' stroke-width='2' stroke-linecap='round' stroke-miterlimit='10' d='M4 7h22M4 15h22M4 23h22'/%3E%3C/svg%3E");

The problem was that the str-replace inline in the data:image blob was causing stylelint to fail. Creating a new variable outside of that string and passing it into the blob worked like a charm.

Along with the rest of what @sandrowuermli recommends above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants