Ignore commas inside quotes. Fixes #24

[Mottie]

Hopefully the test css I provided is sufficient for this change.

[Mottie]

It says build failed (it was fine locally), but I don't think it's my fault! :crying_cat_face:

> perfectionist@2.1.2 test /home/travis/build/ben-eb/perfectionist
> npm run test-unformatted | faucet
/home/travis/build/ben-eb/perfectionist/src/__tests__/api.js:3
import postcss from 'postcss';
^^^^^^
SyntaxError: Unexpected reserved word
    at Module._compile (module.js:439:25)
    at loader (/home/travis/build/ben-eb/perfectionist/node_modules/babel-tape-runner/node_modules/babel-core/node_modules/babel-register/lib/node.js:128:5)
    at Object.require.extensions.(anonymous function) [as .js] (/home/travis/build/ben-eb/perfectionist/node_modules/babel-tape-runner/node_modules/babel-core/node_modules/babel-register/lib/node.js:138:7)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at /home/travis/build/ben-eb/perfectionist/node_modules/babel-tape-runner/bin/babel-tape-runner:12:7
    at Array.forEach (native)
    at /home/travis/build/ben-eb/perfectionist/node_modules/babel-tape-runner/bin/babel-tape-runner:11:11

Sorry, I suck at npm, or I'd know what to do to fix this problem...

[ben-eb]

Think the failure is due to babel-tape-runner 1.3.0 (Babel 6 update). Separate issue.

[ben-eb]

Is the space correct here?

.no-quotes {
  background-image: url(data:image/gif;base64, R0lGOD.....);
                                              ^
}

[Mottie]

No, spaces after base64, break the data-uri.

[Mottie]

Opps, that is one without quotes.. so yeah, the change doesn't work for that test.

[Mottie]

What I mean is, yes, that is correct in that this change does not target commas outside of quotes. So yes, that specific data-uri is broken, but it is expected for the test since it isn't wrapped in quotes.

[ben-eb]

If the output is broken we probably shouldn't add the space there. :smile:

[Mottie]

LOL this pull request changes the regex to only target commas outside of quotes.

That is just a test I added to show that commas, not inside quotes, will be targeted.

Maybe I should work on a regular expression that only targets commas not inside of a url()... the quotes thing was painful already LOL.

[Mottie]

@denji I tried including the base64 in the regex (see #24), but @silverwind yelled at me :crying_cat_face:

[denji]

He's right, base64 is not required (rfc2397)

data:[<MIME-type>][;charset=<encoding>][;base64],<data>

MIME: text/plain
data:,GIF89a%01%00%01%00%80%00%00%FF%FF%FF%FF%FF%FF%21%F9%04%01%0A%00%01%00%2C%00%00%00%00%01%00%01%00%00%02%02L%01%00%3B
---
MIME: image/gif; Charset: ASCII (ISO-8859)
data:image/gif;charset=iso-8859-7,GIF89a%01%00%01%00%80%00%00%FF%FF%FF%FF%FF%FF%21%F9%04%01%0A%00%01%00%2C%00%00%00%00%01%00%01%00%00%02%02L%01%00%3B
---
MIME: image/gif; Encoding: base64
data:image/gif;base64,R0lGODlhAQABAIAAAP///////yH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==

[silverwind]

A inline svg filter makes a great test case:

data:image/svg+xml;charset=utf8,<svg xmlns=\'http://www.w3.org/2000/svg\'><filter id=\'i\'><feColorMatrix type=\'saturate\' values=\'0\'/><feColorMatrix color-interpolation-filters=\'srgb\' values=\'-1 0 0 0 1 0 -1 0 0 1 0 0 -1 0 1 0 0 0 1 0\'/></filter></svg>#i

They also even be in url-escaped form:

data:image/svg+xml;charset=utf8,%3Csvg%20xmlns%3D%27http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%27%20width%3D%2710%27%20height%3D%2710%27%3E%3ClinearGradient%20id%3D%27gradient%27%3E%3Cstop%20offset%3D%2710%25%27%20stop-color%3D%27%23F00%27%2F%3E%3Cstop%20offset%3D%2790%25%27%20stop-color%3D%27%23fcc%27%2F%3E%20%3C%2FlinearGradient%3E%3Crect%20fill%3D%27url(%23gradient)%27%20x%3D%270%27%20y%3D%270%27%20width%3D%27100%25%27%20height%3D%27100%25%27%2F%3E%3C%2Fsvg%3E

[silverwind]

Here are some more test cases for data-uri for you.

(Firefox fails the UTF-8 ones, but that bug was rejected by Mozilla :sob:).

[ben-eb]

Yeah, pinning babel-tape-runner to 1.2.0 has the tests running now on CI, at least. :smile:

[denji]

node.js RFC 2397:

• https://github.com/7anshuai/node-rfc2397
  • https://github.com/7anshuai/node-rfc2397/blob/master/index.js#L1-L3
• https://github.com/Mithgol/dauria

[silverwind]

@Mottie could also try this one.

[Mottie]

Nice find! I think that'll work!

[silverwind]

lgtm. @ben-eb can you merge this?

[ben-eb]

Awesome. Thanks @Mottie for your patience and @silverwind & @denji for your very valuable feedback. :+1:

[ben-eb]

Released in 2.1.3.