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

Do not preload unused variants in the JavaScript-based polyfill #97

Merged
merged 1 commit into from
Jun 30, 2020

Conversation

papandreou
Copy link
Collaborator

@papandreou papandreou commented Jun 7, 2020

Fixes #92

Addresses the problem discussed here: https://gitter.im/assetgraph/assetgraph?at=5ece6c29a91f120a6cc72bca

Looks like we only weed out the non-subfont-introduced fonts when adding the <link rel="preload">s, not the JavaScript-based polyfill.

The User-Agent thing I mentioned is a separate problem, since it means that the unused variants copied into the __subset font families will end up as .ttfs because they're based on the generic CSS from Google Web Fonts. We could fix that by providing woff and woff2 alternatives as well. Let's discuss and try to address it separately.

This polyfill keeps biting us, maybe the state of web browsers is so that we can remove it soon?

@papandreou papandreou self-assigned this Jun 7, 2020
@papandreou papandreou requested a review from Munter June 7, 2020 14:50
@coveralls
Copy link

coveralls commented Jun 7, 2020

Pull Request Test Coverage Report for Build 340

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 89.474%

Totals Coverage Status
Change from base Build 334: 0.05%
Covered Lines: 995
Relevant Lines: 1065

💛 - Coveralls

@papandreou
Copy link
Collaborator Author

Regarding the .ttf format that ends up in the @font-face declarations of __subset for the unused variants. It seems like the best would be if we used the exact fonts.gstatic.com urls for the different font formats so that it matches the async loaded CSS fallback instead of introducing a second copy of the fonts? However, it's hard to consolidate the User-Agent-dependent font sources that come out of Google Web Fonts (see below) since our CSS has to be static.

Maybe we should just introduce another copy of the fonts and provide truetrype, woff, and woff2 format served out of /subfont/?

$ curl -H 'User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; Trident/7.0; .NET4.0C; .NET4.0E; .NET CLR 2.0.50727; .NET CLR 3.0.30729; .NET CLR 3.5.30729; rv:11.0) like Gecko' 'https://fonts.googleapis.com/css?family=Noto+Serif:400'
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFcWbg.woff) format('woff');
}
$ curl 'https://fonts.googleapis.com/css?family=Noto+Serif:400'
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFcWbQ.ttf) format('truetype');
}
$ curl -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.129 Safari/537.36' 'https://fonts.googleapis.com/css?family=Noto+Serif:400'
/* cyrillic-ext */
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFoWaCi_.woff2) format('woff2');
  unicode-range: U+0460-052F, U+1C80-1C88, U+20B4, U+2DE0-2DFF, U+A640-A69F, U+FE2E-FE2F;
}
/* cyrillic */
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFMWaCi_.woff2) format('woff2');
  unicode-range: U+0400-045F, U+0490-0491, U+04B0-04B1, U+2116;
}
/* greek-ext */
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFsWaCi_.woff2) format('woff2');
  unicode-range: U+1F00-1FFF;
}
/* greek */
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFQWaCi_.woff2) format('woff2');
  unicode-range: U+0370-03FF;
}
/* vietnamese */
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFgWaCi_.woff2) format('woff2');
  unicode-range: U+0102-0103, U+0110-0111, U+0128-0129, U+0168-0169, U+01A0-01A1, U+01AF-01B0, U+1EA0-1EF9, U+20AB;
}
/* latin-ext */
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFkWaCi_.woff2) format('woff2');
  unicode-range: U+0100-024F, U+0259, U+1E00-1EFF, U+2020, U+20A0-20AB, U+20AD-20CF, U+2113, U+2C60-2C7F, U+A720-A7FF;
}
/* latin */
@font-face {
  font-family: 'Noto Serif';
  font-style: normal;
  font-weight: 400;
  src: local('Noto Serif'), local('NotoSerif'), url(https://fonts.gstatic.com/s/notoserif/v8/ga6Iaw1J5X9T9RW6j9bNfFcWaA.woff2) format('woff2');
  unicode-range: U+0000-00FF, U+0131, U+0152-0153, U+02BB-02BC, U+02C6, U+02DA, U+02DC, U+2000-206F, U+2074, U+20AC, U+2122, U+2191, U+2193, U+2212, U+2215, U+FEFF, U+FFFD;
}

@Munter
Copy link
Owner

Munter commented Jun 30, 2020

I think we should merge this as-is.

The current state of preload makes me feel we still need it. Firefox doesn't even have preload support unflagged yet (WTF).
When Firefox enabled preload support I'm all for ripping out that javascript polyfill. It just adds complexity.

About the TTF's in out unused fonts declarations, I think the right way forward is to switch behavior to self-host google fonts. I need to find the time to concentrate on that. Maybe it's a hack night opportunity :)

@Munter Munter merged commit 85335ae into master Jun 30, 2020
@Munter Munter deleted the fix/doNotPreloadUnusedVariants branch June 30, 2020 08:01
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

Successfully merging this pull request may close these issues.

TTF preload despite woff2 being available
3 participants