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

Wrong behavior with multiple escaped characters #10

Closed
alexander-akait opened this issue Apr 17, 2017 · 10 comments
Closed

Wrong behavior with multiple escaped characters #10

alexander-akait opened this issue Apr 17, 2017 · 10 comments

Comments

@alexander-akait
Copy link

Input:

.class { content: "\F10C \F10D" }

Output:

.class { content: "\F10C  \F10D" } /* Two space between escaped characters */
@mathiasbynens
Copy link
Owner

Why are you feeding already escaped input to cssesc? What output are you expecting?

Please provide a reproduction case in JS.

@alexander-akait
Copy link
Author

alexander-akait commented Apr 17, 2017

@mathiasbynens

const cssesc = require('cssesc');

console.log(cssesc('😀 😀'));

Output:

\1F600  \1F600

Two space between, but in source one.

@mathiasbynens
Copy link
Owner

mathiasbynens commented Apr 17, 2017

Your input has one space already:

const cssesc = require('cssesc');
cssesc('\u{1F600} \u{1F600}');

The additional space following \1F600 in the output is consumed as part of the first escape sequence. https://mathiasbynens.be/notes/css-escapes#trailing-whitespace

The input from your original post:

.class { content: "\F10C \F10D" }

…decodes to '\u{1F600}\u{1F600}', which is a different string (note the missing space between the emoji).

@alexander-akait
Copy link
Author

alexander-akait commented Apr 17, 2017

@mathiasbynens I'm not really sure then who is responsible for the problem, css-loader use css-selector-tokenizer for some node types and change their content, but css-selector-tokenizer use cssesc for escaping string https://github.com/css-modules/css-selector-tokenizer/blob/d315d8d2a77cab97d90dcac5ee9f36c1d7688245/lib/stringifyValues.js#L27

@alexander-akait
Copy link
Author

And we give some incorrect behavior with escaped characters, example above #10 (comment)

@alexander-akait
Copy link
Author

@mathiasbynens also you example contain two spaces in output

const cssesc = require('cssesc');
console.log(cssesc('\u{1F600} \u{1F600}'));
\1F600  \1F600

We do not need two spaces right?

@mathiasbynens
Copy link
Owner

@evilebottnawi Exactly, and the rest of my comment explains why that’s okay. Check out the link I posted for more info.

@mathiasbynens
Copy link
Owner

@evilebottnawi It seems like there is some kind of bug in one of the projects you mentioned — they shouldn’t escape already-escaped data. Are there open bug tickets I could look at?

@alexander-akait
Copy link
Author

@mathiasbynens I have not seen them, we are preparing to release a stable version css-loader and i accidentally stumbled upon this error, PR in css-loader webpack-contrib/css-loader#493 and line https://github.com/evilebottnawi/css-loader/blob/094b60bdd47473e51a171b7e691c45e53db604c4/test/simpleTest.js#L29. Just uncomment line and run test npm run test

@alexander-akait
Copy link
Author

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