From 6be09eec093b4832aaa2558a408afbd7514ccc8f Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 31 May 2023 22:13:55 -0400 Subject: [PATCH] Remove STIXGeneral from the font stack (#10980) * Remove STIXGeneral from the font stack STIXGeneral was originally added to our font stack to work around a bug in Chrome (https://bugs.chromium.org/p/chromium/issues/detail?id=591346) which caused some obscure combining marks to render as tofu. However, because STIXGeneral unexpectedly has glyphs for a handful of common Japanese characters, it's ended up making Japanese text in Element look patchy. I previously attempted to fix this by prioritizing sans-serif over STIXGeneral, but as is evident from our screenshot tests and user reports, this is still not enough on some systems to get Chrome to pick a consistent font for Japanese. On the basis that i18n is more important than supporting a few mathematical diacritics, I propose we remove the font. STIXGeneral is deprecated anyways, so if we want to get these diacritics back there's always the option of looking at its successor, STIXTwo. * Remove STIXGeneral installation from Cypress workflow --- .github/workflows/cypress.yaml | 6 ------ res/themes/legacy-light/css/_legacy-light.pcss | 12 +++--------- res/themes/light/css/_light.pcss | 12 +++--------- 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/.github/workflows/cypress.yaml b/.github/workflows/cypress.yaml index c4d23ae2da..df737c6ab4 100644 --- a/.github/workflows/cypress.yaml +++ b/.github/workflows/cypress.yaml @@ -120,12 +120,6 @@ jobs: - uses: browser-actions/setup-chrome@c485fa3bab6be59dce18dbc18ef6ab7cbc8ff5f1 - run: echo "BROWSER_PATH=$(which chrome)" >> $GITHUB_ENV - - uses: tecolicom/actions-use-apt-tools@ceaf289fdbc6169fd2406a0f0365a584ffba003b # v1 - with: - # Our test suite includes some screenshot tests with unusual diacritics, which are - # supposed to be covered by STIXGeneral. - tools: fonts-stix - # There's a 'download artifact' action, but it hasn't been updated for the workflow_run action # (https://github.com/actions/download-artifact/issues/60) so instead we get this mess: - name: 📥 Download artifact diff --git a/res/themes/legacy-light/css/_legacy-light.pcss b/res/themes/legacy-light/css/_legacy-light.pcss index 310ef4e59e..9ce9c28d31 100644 --- a/res/themes/legacy-light/css/_legacy-light.pcss +++ b/res/themes/legacy-light/css/_legacy-light.pcss @@ -1,23 +1,17 @@ /* Nunito lacks combining diacritics, so these will fall through to the next font. Helevetica's diacritics sometimes do not combine nicely (on OSX, at least) and result in a huge horizontal mess. - Arial empirically gets it right, hence prioritising Arial here. - We also include STIXGeneral explicitly to support a wider range - of combining diacritics (Chrome fails without it, as per - https://bugs.chromium.org/p/chromium/issues/detail?id=1328898). - We should never actively *prefer* STIXGeneral over the default font though, - since it looks pretty rough and implements some non-LGC scripts only - partially, making, for example, Japanese text look patchy and sad. */ + Arial empirically gets it right, hence prioritising Arial here. */ /* We fall through to Twemoji for emoji rather than falling through to native Emoji fonts (if any) to ensure cross-browser consistency */ /* Noto Color Emoji contains digits, in fixed-width, therefore causing digits in flowed text to stand out. TODO: Consider putting all emoji fonts to the end rather than the front. */ $font-family: "Nunito", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Arial", "Helvetica", sans-serif, - "STIXGeneral", "Noto Color Emoji"; + "Noto Color Emoji"; $monospace-font-family: "Inconsolata", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Courier", monospace, - "STIXGeneral", "Noto Color Emoji"; + "Noto Color Emoji"; /* unified palette */ /* try to use these colors when possible */ diff --git a/res/themes/light/css/_light.pcss b/res/themes/light/css/_light.pcss index 60fd0196d1..58ab893058 100644 --- a/res/themes/light/css/_light.pcss +++ b/res/themes/light/css/_light.pcss @@ -1,23 +1,17 @@ /* Nunito and Inter lacks combining diacritics, so these will fall through to the next font. Helevetica's diacritics sometimes do not combine nicely (on OSX, at least) and result in a huge horizontal mess. - Arial empirically gets it right, hence prioritising Arial here. - We also include STIXGeneral explicitly to support a wider range - of combining diacritics (Chrome fails without it, as per - https://bugs.chromium.org/p/chromium/issues/detail?id=1328898). - We should never actively *prefer* STIXGeneral over the default font though, - since it looks pretty rough and implements some non-LGC scripts only - partially, making, for example, Japanese text look patchy and sad. */ + Arial empirically gets it right, hence prioritising Arial here. */ /* We fall through to Twemoji for emoji rather than falling through to native Emoji fonts (if any) to ensure cross-browser consistency */ /* Noto Color Emoji contains digits, in fixed-width, therefore causing digits in flowed text to stand out. TODO: Consider putting all emoji fonts to the end rather than the front. */ -$font-family: "Inter", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Arial", "Helvetica", sans-serif, "STIXGeneral", +$font-family: "Inter", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Arial", "Helvetica", sans-serif, "Noto Color Emoji"; $monospace-font-family: "Inconsolata", "Twemoji", "Apple Color Emoji", "Segoe UI Emoji", "Courier", monospace, - "STIXGeneral", "Noto Color Emoji"; + "Noto Color Emoji"; /* Colors from Figma Compound https://www.figma.com/file/X4XTH9iS2KGJ2wFKDqkyed/Compound?node-id=559%3A120 */ /* ******************** */