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

Having a non-breaking space in css leads to error #1006

Closed
blackholegalaxy opened this issue Feb 20, 2018 · 27 comments · Fixed by dalton-0x0/cohort3#38 or grechut/watchdog#41 · May be fixed by kosiakMD/progressive_optimistic_ui#1, JuliusMorkunas/liker#16 or m0rphtail/Teleport#4

Comments

@blackholegalaxy
Copy link

blackholegalaxy commented Feb 20, 2018

Environment

Using clean-css through Angular CLI 1.7.0 which moved to clean-css. clean-css is imported through webpack plugin as it can seen here: https://github.com/angular/angular-cli/blob/master/packages/@angular/cli/plugins/cleancss-webpack-plugin.ts

Configuration options

 const cleancss = new CleanCSS({
          compatibility: 'ie9',
          level: 2,
          inline: false,
          returnPromise: true,
          sourceMap: this._options.sourceMap,
        });

Input SCSS

Image, since non-breaking space would not show up properly (screenshot taken on VSCode on faulty scss code, with Highlight bad characters VSCode plugin):

bad_char

This code was transpiled to CSS, which still contains the non-breaking space. Resulting in following error.

Error

/project/node_modules/clean-css/lib/reader/input-source-map-tracker.js:37
if (originalPosition.line === null && line > 1 && selectorFallbacks > 0) {

TypeError: Cannot read property 'line' of undefined
at originalPositionFor (/project/node_modules/clean-css/lib/reader/input-source-map-tracker.js:37:24)
at originalMetadata (/project/node_modules/clean-css/lib/tokenizer/tokenize.js:486:43)
at intoTokens (/project/node_modules/clean-css/lib/tokenizer/tokenize.js:435:68)
at tokenize (/project/node_modules/clean-css/lib/tokenizer/tokenize.js:74:10)
at fromStyles (/project/node_modules/clean-css/lib/reader/read-sources.js:147:12)
at fromString (/project/node_modules/clean-css/lib/reader/read-sources.js:48:10)
at doReadSources (/project/node_modules/clean-css/lib/reader/read-sources.js:33:12)
at readSources (/project/node_modules/clean-css/lib/reader/read-sources.js:24:10)
at /project/node_modules/clean-css/lib/clean.js:99:12
at _combinedTickCallback (internal/process/next_tick.js:131:7)
at process._tickCallback (internal/process/next_tick.js:180:9)

Expected behavior

Presence of non-breaking space should not block the production of css since these kind of spaces should be deleted in the minification process.

Potential fix

We could add a marker for no-break space and replace the character with standard space.

NO_BREAK_SPACE: /\u00A0|\u202F|\uFEFF/g,

then in tokenize:

isNoBreakSpace = Marker.NO_BREAK_SPACE.test(character);
...
else if (isNoBreakSpace) {
      // Replace any kind of no-break-space with standard space
      character = Marker.SPACE;
      buffer.push(character);
    }

Can't open a PR as I don't know how to test that in your test suit.

@blackholegalaxy blackholegalaxy changed the title Having a non-breaking space in scss leads to error Having a non-breaking space in css leads to error Feb 20, 2018
@joaopslins
Copy link

+1

@tx8821
Copy link

tx8821 commented Feb 27, 2018

Please fix this! Angular CLI cant build because of this!

@FirassKoubaa
Copy link

+1

1 similar comment
@renatomrcosta
Copy link

+1

@barocsi
Copy link

barocsi commented Mar 1, 2018

+1 this is so funny in a sad way.

@scote
Copy link

scote commented Mar 2, 2018

Any new when it will be fixed?

@markgoho-EDT
Copy link

@jakubpawlowicz are any contributors able to help with this issue?

@vithubati
Copy link

any update on this?

@barocsi
Copy link

barocsi commented Mar 3, 2018

if it helps just scan all of your files for non breaking space using Notepad++ or regex in IntelliJ / VS whichever tool you use to spot malformed locations.
regex this:

\xA0

@blackholegalaxy
Copy link
Author

blackholegalaxy commented Mar 3, 2018

I gave a code snippet on the main issue post. By patching clean-css with it everything works properly. The fact is I don't know how to write test on clean-css context, so I can't open a PR.

@scote
Copy link

scote commented Mar 3, 2018

@barocsi @blackholegalaxy for me the error came from a third party lib (materialize-css) i'm using in my project. I can't update those css or scss file.

@jakubpawlowicz
Copy link
Collaborator

Sure, will fix it in 4.1.10 coming out (hopefully) today.

@jakubpawlowicz jakubpawlowicz added this to the 4.1.10 milestone Mar 5, 2018
@jakubpawlowicz
Copy link
Collaborator

Hmmm, I took a look at the source code and we read all files as utf-8 not utf-16, so it will be rather tricky to fix at this point. Could you at least provide me with a test CSS file to test on?

@enavarro222
Copy link

@jakubpawlowicz the issue appears with materializecss (tested with v0.100.2) cf https://github.com/Dogfalo/materialize/blob/master/dist/css/materialize.min.css

btw, as mention by @scote, as this issue is raised by a third party lib, this is really annoying ! I hope you could find a solution quickly... Thank you very much to address this issue !

@jakubpawlowicz
Copy link
Collaborator

Thanks @enavarro222 for a quick reply. I've fetched the file but can't replicate the issue yet - since the it shows up when reading input source maps, can someone please post materialize.css transpiled from SCSS with source map?

@enavarro222
Copy link

@jakubpawlowicz not sure is it could help but here is a minimal setup to reproduce the issue with angular cli :

$ npm install -g @angular/cli
$ ng new testproject
$ cd testproject
$ npm install materialize-css

Then you have to add materialize.min.css in .angular-cli.json:

...
      "styles": [
        "styles.css",
        "../node_modules/materialize-css/dist/css/materialize.min.css"
      ],
...

And then:

$ ng build --prod
 92% chunk asset optimization/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/reader/input-source-map-tracker.js:37
  if (originalPosition.line === null && line > 1 && selectorFallbacks > 0) {
                      ^

TypeError: Cannot read property 'line' of undefined
    at originalPositionFor (/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/reader/input-source-map-tracker.js:37:23)
    at originalMetadata (/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/tokenizer/tokenize.js:486:43)
    at intoTokens (/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/tokenizer/tokenize.js:435:68)
    at tokenize (/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/tokenizer/tokenize.js:74:10)
    at fromStyles (/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/reader/read-sources.js:147:12)
    at fromString (/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/reader/read-sources.js:48:10)
    at doReadSources (/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/reader/read-sources.js:33:12)
    at readSources (/Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/reader/read-sources.js:24:10)
    at /Users/navarro/workspace/tmp/testproject/node_modules/clean-css/lib/clean.js:99:12
    at _combinedTickCallback (internal/process/next_tick.js:95:7)
    at process._tickCallback (internal/process/next_tick.js:161:9)

@scote
Copy link

scote commented Mar 5, 2018

@angular/cli version must be greater than 1.6.8.

@blackholegalaxy
Copy link
Author

blackholegalaxy commented Mar 5, 2018

@enavarro222 thx for repro.

You could also simply add any no breaking space anywhere in any css. It will break. Here is the character: " " (reference: http://www.fileformat.info/info/unicode/char/00a0/browsertest.htm)

This character exists in UTF-8 @jakubpawlowicz

The error occurs when any of the three kind of no-break space is present:

\u00A0
\u202F
\uFEFF

@jakubpawlowicz
Copy link
Collaborator

I've been able to reproduce the issue locally, however it's not entirely clean-css bug but also sass building incorrect source map. I found couple cases when line:column data inside it is invalid (e.g. column being negative causing source-map to explode when verifying it).

However I'll add some code to handle such cases gracefully.

@blackholegalaxy
Copy link
Author

Great! Thanks @jakubpawlowicz

@jakubpawlowicz
Copy link
Collaborator

It's fixed in 4.1.10.

@scote
Copy link

scote commented Mar 5, 2018

@blackholegalaxy will you reopen your issue on angular/cli?

@renatomrcosta
Copy link

Great work @jakubpawlowicz! Thanks!

@blackholegalaxy
Copy link
Author

blackholegalaxy commented Mar 5, 2018

@cote since it will be fixed by version 4.1.10 here angular CLI issue should solve by itself since they have a dependency set to "clean-css": "^4.1.9",

Since the 4.1.10 is out, we can test with no-break space in css code and reinstall angular CLI to check if the builds now succeeds. If it doesn't, let me know, i'll may reopen issue.

@blackholegalaxy
Copy link
Author

blackholegalaxy commented Mar 5, 2018

Just a quick real project test with invalid char on scss source. Removed node_modules and let npm install latest clean-css. Tested under 1.6.8, 1.7.0, 1.7.1, 1.7.2:

works

So the fix works. Thanks @jakubpawlowicz

@jakubpawlowicz
Copy link
Collaborator

Boom, cheers! 🍺

@enavarro222
Copy link

thanks a lot @jakubpawlowicz !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment