fix: Used font-families discarded due to mixed quotation use

[GreenGremlin]

Currently, postcss-discard-unused is improperly discarding @font-face definitions when their font-family's quotation style differs from its use in font and font-family styles.

Example:

@font-face {
    font-family: 'FontAwesome';
    src: url('/fonts/fontawesome-webfont.woff2?v=4.7.0') format('woff2'),
         url('/fonts/fontawesome-webfont.woff?v=4.7.0') format('woff');
}

.icon {
    font: 14px/1 FontAwesome;
}

When executed on the above css, the postcss-discard-unused plugin will improperly detect the FontAwesome font as unused. This happens because its font-family definition in the @font-face rule is wrapped in quotes but the use of the font is not. The css specifications does not require that the font-family definition and rule match in their quotation style.

This change updates the postcss-discard-unused plugin to normalize font names before checking them against a "normalized" list of used fonts.

This required refactoring the way fontCache.values is built. Rather than just concatenating font and font-family values, it is now a bit smarter in that font values are first split by spaces, then commas, then flattened.

Lastly, this change simplifies the way font-family properties inside @font-face blocks are parsed. Since a valid @font-face should only ever contain a single font-family definition and that font-family should only have a single value, we can use find instead of filter and we also don't need to split the font-family value by commas. This makes the filterFont logic much more simple. This change is not necessary for the fix, and I would be ok reversing it if that would help get this MR merged.

Fixes #726

[coveralls]

Coverage increased (+0.006%) to 99.011% when pulling 2a92e3c on GreenGremlin:normalized-fonts into 8d4610a on cssnano:master.

[alexander-akait]

Looks like a breaking change, maybe better we do this under option? Also some notes about implementation

[alexander-akait]

Bad idea, it is wrong, you can find how normalize font-name in other our plugin

[GreenGremlin]

What plugin is that? There are quite a few in this repo.

[alexander-akait]

Here right solution https://github.com/cssnano/cssnano/blob/a520e6906e7fa17951a64769f030ed7b6f44c38a/packages/postcss-minify-font-values/src/lib/minify-family.js, it is really not easy parse font-family, maybe we should move this utils to separate package

[GreenGremlin]

In what scenario do you see this failing? The only scenario I can think of would be if there were a font-face with a name that matched a non font-family value in a font property, something like bold or 10px. That would be a very unusual edge case, and likely an invalid font name. Even then, the failure would be that the poorly named font-face would not be discarded.

[alexander-akait]

It it not poor, some font can have very weird characters in name, so we should parse this as w3 say

[GreenGremlin]

According to the w3's font shorthand documentation, the font shorthand must always include a font-size and font-family/line-height as the last two properties. Knowing this, it is actually pretty simple to extract the font-family value from a font shorthand. All we have to do is identify the last matching font-size/line-height property and strip it and everything before it out of the font value. I added a method getFontFamilyFromShorthand which does just that. I thin the current iteration of this change is pretty solid. Let me know what you think.

[alexander-akait]

@GreenGremlin we should not use regexp for normalize, problem not in shorthard, problem what font name can contains unicode (no u flag and we can't use this flag right now) characters and you regexp is broken in this case, we should use parser for this as i describe above

[GreenGremlin]

Maybe "normalize" isn't the best name for what this fix does. All this fix is trying to do is strip out wrapping quotes and escape characters from font names so that they can be compared in a base format. We really shouldn't need to fully "normalize" the fonts, like is done in the postcss-minify-font-values. I did rework my changes using postcss-value-parser, which makes the parsing a bit more clean. I also broke the font name parsing logic out to its own module, with its own tests. If you still think there are some unicode characters that will break this, please give me an example and I'll add a test for it.

[alexander-akait]

Thanks, now it is looks good

[GreenGremlin]

Thanks, now it is looks good

Great!! Is there anything else I can do to help get this merged?

[alexander-akait]

@GreenGremlin thanks for PR, right now i rewrite postcss-calc, after this we do release

[GreenGremlin]

I merged in the latest changes, resolved the conflicts, and ran prettier. It would be nice to get these changes merged before there's more conflicts.

[ludofischer]

I have tested on the latest 5.0 RC and I think the issue has already been fixed.