Skip to content

Commit

Permalink
Merge pull request #908 from assetgraph/feature/caseInsensitiveFontFa…
Browse files Browse the repository at this point in the history
…milyMatching

Match font-family property values case insensitively
  • Loading branch information
Munter committed Jul 19, 2018
2 parents c12e37b + 7dd69eb commit a409baa
Show file tree
Hide file tree
Showing 8 changed files with 121 additions and 9 deletions.
8 changes: 5 additions & 3 deletions lib/transforms/subsetFonts.js
Expand Up @@ -1078,7 +1078,7 @@ These glyphs are used on your site, but they don't exist in the font you applied
)
.reduce((result, current) => {
for (const familyName of current) {
result[familyName] = `${familyName}__subset`;
result[familyName.toLowerCase()] = `${familyName}__subset`;
}
return result;
}, {});
Expand Down Expand Up @@ -1124,7 +1124,8 @@ These glyphs are used on your site, but they don't exist in the font you applied
const fontFamilies = cssListHelpers
.splitByCommas(cssRule.value)
.map(unquote);
const subsetFontFamily = webfontNameMap[fontFamilies[0]];
const subsetFontFamily =
webfontNameMap[fontFamilies[0].toLowerCase()];
if (subsetFontFamily && !fontFamilies.includes(subsetFontFamily)) {
cssRule.value = `${cssQuoteIfNecessary(subsetFontFamily)}, ${
cssRule.value
Expand All @@ -1136,7 +1137,8 @@ These glyphs are used on your site, but they don't exist in the font you applied
const fontFamilies =
fontProperties && fontProperties['font-family'].map(unquote);
if (fontFamilies) {
const subsetFontFamily = webfontNameMap[fontFamilies[0]];
const subsetFontFamily =
webfontNameMap[fontFamilies[0].toLowerCase()];
if (
subsetFontFamily &&
!fontFamilies.includes(subsetFontFamily)
Expand Down
11 changes: 7 additions & 4 deletions lib/util/fonts/injectSubsetDefinitions.js
Expand Up @@ -2,7 +2,9 @@ const postcssValuesParser = require('postcss-values-parser');
const unquote = require('./unquote');

function injectSubsetDefinitions(cssValue, webfontNameMap) {
const subsetFontNames = new Set(Object.values(webfontNameMap));
const subsetFontNames = new Set(
Object.values(webfontNameMap).map(name => name.toLowerCase())
);
const tokens = postcssValuesParser(cssValue).tokens;
let resultStr = '';
let isPreceededByWords = false;
Expand All @@ -29,10 +31,11 @@ function injectSubsetDefinitions(cssValue, webfontNameMap) {
}
if (possibleFontFamily) {
// Bail out, a subset font is already listed
if (subsetFontNames.has(possibleFontFamily)) {
const possibleFontFamilyLowerCase = possibleFontFamily.toLowerCase();
if (subsetFontNames.has(possibleFontFamilyLowerCase)) {
return cssValue;
} else if (webfontNameMap[possibleFontFamily]) {
resultStr += `'${webfontNameMap[possibleFontFamily].replace(
} else if (webfontNameMap[possibleFontFamilyLowerCase]) {
resultStr += `'${webfontNameMap[possibleFontFamilyLowerCase].replace(
/'/g,
"\\'"
)}', `;
Expand Down
3 changes: 2 additions & 1 deletion lib/util/fonts/snapToAvailableFontProperties.js
Expand Up @@ -77,7 +77,8 @@ function snapToAvailableFontProperties(fontFaceDeclarations, propsToSnap) {
// Naively assume that the first defined font family is the one we are looking for. If it's a webfont it should be likely
const familyMatches = fontFaceDeclarations.filter(
fontFaceDeclaration =>
fontFaceDeclaration['font-family'] === fontFamilies[0]
fontFaceDeclaration['font-family'].toLowerCase() ===
fontFamilies[0].toLowerCase()
);

// No match for font-family. Probably not a web font. Early exit
Expand Down
32 changes: 32 additions & 0 deletions test/transforms/subsetFonts.js
Expand Up @@ -3086,6 +3086,38 @@ describe('transforms/subsetFonts', function() {
});
});

it('should tolerate case differences in font-family', async function() {
httpception();

const assetGraph = new AssetGraph({
root: pathModule.resolve(
__dirname,
'../../testdata/transforms/subsetFonts/local-font-family-case-difference/'
)
});
await assetGraph.loadAssets('index.html');
await assetGraph.populate();
const { fontInfo } = await assetGraph.subsetFonts({
inlineSubsets: false
});

expect(fontInfo, 'to satisfy', [
{
fontUsages: [
{
texts: ['Hello, world!', 'Hello, yourself!'],
props: { 'font-family': 'Open Sans' }
}
]
}
]);
expect(
assetGraph.findAssets({ type: 'Css' })[0].text,
'to contain',
"font-family: 'Open Sans__subset', oPeN sAnS;"
).and('to contain', "--the-font: 'Open Sans__subset', OpEn SaNs;");
});

it('should handle HTML <link rel=stylesheet> with Google Fonts', function() {
httpception(defaultLocalSubsetMock);

Expand Down
31 changes: 30 additions & 1 deletion test/util/fonts/injectSubsetDefinitions.js
Expand Up @@ -30,6 +30,14 @@ describe('injectSubsetDefinitions', function() {
);
});

it('should match the font-family case sensitively', function() {
expect(
injectSubsetDefinitions('Times new rOman', webfontNameMap),
'to equal',
"'times new roman__subset', Times new rOman"
);
});

it('should tolerate multiple spaces between words', function() {
expect(
injectSubsetDefinitions('times new roman', webfontNameMap),
Expand All @@ -54,7 +62,7 @@ describe('injectSubsetDefinitions', function() {
);
});

it('should not inject the subset into a value that already has it', function() {
it('should not inject the subset into a value that already has it, same casing', function() {
expect(
injectSubsetDefinitions(
"'times new roman__subset', times new roman",
Expand All @@ -65,6 +73,27 @@ describe('injectSubsetDefinitions', function() {
);
});

it('should not inject the subset into a value that already has it, case difference in existing value', function() {
expect(
injectSubsetDefinitions(
"'TIMES new roman__subset', times new roman",
webfontNameMap
),
'to equal',
"'TIMES new roman__subset', times new roman"
);
});

it('should not inject the subset into a value that already has it, case difference in webfontNameMap value', function() {
expect(
injectSubsetDefinitions("'times new roman__subset', times new roman", {
'times new roman': 'TIMES new roman__subset'
}),
'to equal',
"'times new roman__subset', times new roman"
);
});

it('should escape singlequotes in the subset font name', function() {
expect(
injectSubsetDefinitions('"times new roman"', {
Expand Down
17 changes: 17 additions & 0 deletions test/util/fonts/snapToAvailableFontProperties.js
Expand Up @@ -127,6 +127,23 @@ describe('snapToAvailableFontProperties', function() {
});
});

it('should return a case insensitive match', function() {
var snapped = snap(
[
{
'font-family': 'Foo'
}
],
{
'font-family': 'foO'
}
);

expect(snapped, 'to satisfy', {
'font-family': 'Foo'
});
});

it('should unquote quoted values', function() {
var snapped = snap(
[
Expand Down
Binary file not shown.
@@ -0,0 +1,28 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title>Document</title>
<style>
@font-face {
font-family: 'Open Sans';
font-style: normal;
font-weight: 400;
src: url(OpenSans.ttf) format('truetype');
}

h1 {
font-family: oPeN sAnS;
}

h2 {
--the-font: OpEn SaNs;
font-family: var(--the-font);
}
</style>
</head>
<body>
<h1>Hello, world!</h1>
<h2>Hello, yourself!</h2>
</body>
</html>

0 comments on commit a409baa

Please sign in to comment.