Make WordPress Core

Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#46169 closed enhancement (fixed)

Use system fonts for the block editor

Reported by: garrett-eclipse's profile garrett-eclipse Owned by: garrett-eclipse's profile garrett-eclipse
Milestone: 5.7 Priority: normal
Severity: normal Version: 5.0
Component: Editor Keywords: has-patch
Focuses: ui, css, administration, performance, privacy Cc:

Description

Following from a discussion in #core-privacy I'm opening this to move on the Google Fonts issue with Gutenberg.
Discussion - https://wordpress.slack.com/archives/C9695RJBW/p1549043771637800

There's two main concerns;

  1. From a privacy standpoint Google Fonts tracks users potentially collecting PII
  2. From a performance standpoint the assets should be served locally to take advantage of modern font-loading optimization, etc. The cross-domain request slows down the site even with pre-fetching.

This was originally raised in Gutenberg Github and closed as it's now a core issue;
https://github.com/WordPress/gutenberg/issues/11648

This was perviously done in 4.6 to remove Open Sans as done here;
https://core.trac.wordpress.org/ticket/36753

Thank you

Attachments (3)

system-fonts.patch (1.9 KB) - added by Joen 3 years ago.
First stab at removing Noto Serif
system-fonts.2.patch (1.8 KB) - added by Joen 3 years ago.
46169.diff (1.8 KB) - added by hellofromTonya 3 years ago.
Restores $style->add() code for backwards compatibility, but adds notes for no longer used (similar to Open Sans).

Download all attachments as: .zip

Change History (49)

#1 @garrett-eclipse
5 years ago

Specifically, this is for the replacement of Noto Serif the Google Font used by the block editor.

Core reference - https://github.com/WordPress/WordPress/blob/3c7a623f85f4648eded5cbcf340ccb5c9cc3b2fb/wp-includes/script-loader.php#L1929-L1938

#2 @joyously
5 years ago

This would be better for when the styles are being overridden also.

This ticket was mentioned in Slack in #core-privacy by garrett-eclipse. View the logs.


5 years ago

#4 in reply to: ↑ description @pento
5 years ago

  • Keywords needs-design added

Replying to garrett-eclipse:

  1. From a privacy standpoint Google Fonts tracks users potentially collecting PII

Is there a reference for this? Reading through this discussion and the related FAQ, it seems like Google logs minimal data from Google Fonts requests.

  1. From a performance standpoint the assets should be served locally to take advantage of modern font-loading optimization, etc. The cross-domain request slows down the site even with pre-fetching.

Google Fonts is pretty fast. The first request to load the block editor from my site takes ~8s, of which 80ms is Noto Serif. Subsequent loads are 4s for the page load, and 0ms for Noto Serif, thanks to caching. This seems like a premature optimisation.

I'll defer to the design team as to whether system fonts are a better (or at least equivalent) writing experience compared to Noto Serif.

#5 @aristath
5 years ago

Is there a reference for this? Reading through this discussion and the related FAQ, it seems like Google logs minimal data from Google Fonts requests.

Google claims it logs minimal data, however there is no mention whatsoever of what data gets logged, for how long the logs remain and what kind of processing google then does on the logged data. Google's FAQ only states that "The Google Fonts API is designed to limit the collection, storage, and use of end-user data to what is needed to serve fonts efficiently" without providing any specifics.
The vagueness of their docs, the lack of information and specificity is what makes them a serious GDPR concern.

Google Fonts is pretty fast.

In general yes, they're pretty fast. But that's only because your connection and mine are fast. But a few days ago when there were heavy rains in my area for example and connection speeds were suffering, gfonts were a constant pain. We should not assume that everyone has a decent network or that they have the luxury of an 80ms request for webfonts.

I'll defer to the design team as to whether system fonts are a better (or at least equivalent) writing experience compared to Noto Serif.

Note Serif is a better experience, there's no doubt about that. It's a beautiful font. But it's not what themes use on the frontend so the editor should not assume anything and should not use an opinionated font. If a theme uses Noto Serif then it should enqueue it on its own in the editor.

#6 @joyously
5 years ago

Note Serif is a better experience, there's no doubt about that. It's a beautiful font.

Only in certain languages. If you use system fonts, there is no lag, no privacy concern, and the user gets the font they've already chosen as very readable. If this is just for the editor chrome, it's the best choice to use system fonts (like sans-serif). If it's for the content, the editor should not be specifying anything at all.

This ticket was mentioned in Slack in #core-privacy by dejliglama. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-privacy by postphotos. View the logs.


5 years ago

#9 @garrett-eclipse
5 years ago

Thanks @pento @joyously & @aristath for the input.

Google Fonts seems to be a recurring discussion in the #core-privacy channel as well as the privacy community at large as Google is so ambiguous with the disclosure of data usage that they become a concern. At minimum Google collects your IP address and the website/page source from which the call was made this assist them in understanding your browsing habits and potentially enables them to serve more targetted ads towards you. Google may not be doing this right now but their privacy policies enable them if they would like. Google Fonts along with all Google APIs are bound by the generic API terms of service which includes this clause;
"For the sole purpose of enabling Google to provide, secure, and improve the APIs (and the related service(s)) and only in accordance with the applicable Google privacy policies, you give Google a perpetual, irrevocable, worldwide, sublicensable, royalty-free, and non-exclusive license to Use content submitted, posted, or displayed to or from the APIs through your API Client. "Use" means use, host, store, modify, communicate, and publish."
Reference - https://developers.google.com/terms/
Google Privacy Policy (https://policies.google.com/privacy) enables it to collect large amounts of user data through its services including log data (browser version) and location data (IP Address). Sites that use Google Fonts are feeding data back to Google. It's possible that Google doesn't actively collect or use the data from Google Fonts but there's no assurance or disclosure which prompts those who are privacy-conscious to seek alternatives.

That being said there's no definitive source on how much data Google Fonts feeds to Google or how they utilize that data. But with the Core Privacy team trying to move WordPress to a Privacy by Design methodology removing the possibility of undisclosed data collection is a huge benefit to the community.

To my understanding though WordPress has been moving away from the use of CDN's and external resources and another approach could very well be bundling the Noto Serif font directly into WP which would allay any privacy and performance concerns while still providing the same experience.

Another point on avoiding external resources is local development while offline is hindered by the use of external resources. I myself have 1-2 hour bus commute both ways to/from work and I often spin up WP instances on my machine with no internet connection. Avoiding external resources assists those with minimal or no internet connection.

In my opinion I'd be happy with either switching out the gFont for a system font or bundling it with WordPress to remove the external call and potential data leakage.

All the best

This ticket was mentioned in Slack in #core-editor by youknowriad. View the logs.


5 years ago

#11 @garrett-eclipse
5 years ago

  • Keywords needs-design-feedback 2nd-opinion dev-feedback added; needs-patch removed

Hello All,

I just wanted to update the thread with some discussion posted via make/core that is applicable to the discussion here and provides potential forward direction. I hope those mentioned in the quoted text don't mind the ping, I mostly want to direct further discussion via this ticket in order to have it be a central point for this request. As well I didn't want to quote without indicating such to the original commenters.

The following thread comes from comments on these office hours minutes;
https://make.wordpress.org/core/2019/02/20/core-privacy-office-hours-minutes-13-february-2019/

Tom J Nowell (@tjnowell) 5:14 am on February 20, 2019

a proof of concept patch to add a customizer option to disable Google Fonts for the older (pre-Twenty Nineteen) default themes.

Can we not just bundle the fonts? That way no user opt in or out is necessary. Additionally, there’s no mention of this in either of the quoted tickets

Also, are we disregarding the precedent set when the Google Font was removed from MP6 in WP Admin? /cc @pento It seems to me that the TLDR is that a mistake was made adding Noto Serif via Google Fonts probably due to a lack of awareness. Rather than admit it, it’s been swept under the rug so that the decision can be made a second time to save face, or at least that’s how it looks. Honestly it isn’t such a big deal, I suggest sticking to the original decision

###REPLY###
Gary Pendergast (@pento) 8:24 pm on February 20, 2019

Open Sans was removed from wp-admin in WordPress 4.6, several years after the MP6 redesign released in WordPress 3.8. It was removed because system fonts had reached a point where the admin could be rendered in a consistent manner with good fonts across all platforms (#36753).

I’d like to see more research into the font bundling option, as some translations will use the localised version of the relevant font (for example, Japanese uses Noto Serif JP), while others will choose to use system fonts where no good equivalent font exists (eg, Hebrew). This can vary by font, too, as Hebrew does use Inconsolata loaded from Google Fonts in Twenty Fifteen.

There’s some relevant discussion on GB2798: this was a deliberate decision to use Noto Serif over system fonts, the folks making that decision were clearly aware of the tradeoffs.

There are multiple criteria that need to be met for changes to happen here:

A font change would need to be approved by a Gutenberg design lead. @karmatosed, @joen, or @mapk could provide more insight here.
Bundling fonts would need to not increase the wordpress.zip file size significantly.
Bundled fonts would need to be released under a GPL2-compatible license.
Bundling fonts would also need the appropriate infrastructure in place to switch font by language.
I’m fine with this change happening, but folks working on it should be aware that there are significant hurdles to overcome in order to avoid it creating a poor user experience.

###REPLY###
Joen Asmussen (@joen) 11:41 pm on February 20, 2019

We certainly don’t want lumpy rugs!

I can speak to this, as I was part of the decision-making process. As is hopefully clear from the discussion on the ticket, no malice was intended in linking to Google’s servers, and for the past two years of the font being in place the major concerns around the font choice were related to localization challenges and performance.

As you bring it up, I do recall Open Sans being removed for privacy reasons. It’s unfortunate we forgot to bring that up during the discussion, it should have been. I recall the time of discussion being an incredibly challenging phase of the project, for everyone involved, and although in hindsight this should’ve had more thought, so many gears were in motion at the time, that it was perhaps easy for us to forget, or postpone and then forget. I hope that as we try to attract a new generation of WordPress contributors, we can surface past decisions like these in a way that empowers and encourages, as opposed to discourage.

In addition to what is mentioned on the thread, one of the reasons Noto Serif stayed in place (aside from being more pleasant to read than Georgia) was to imply that the block editor is more than just a generic post editor; that via editor styles you can make it look the same as the theme itself. Georgia, and to a vastly greater extent Times New Roman, imply “generic” in a way that if you see Times New Roman on a website you might think the CSS is still loading. Simply put, the goal of Noto was to suggest that there are styling opportunities here.

This hasn’t worked as well as we intended. In actual practice, and by virtue of being a theme that actually styles the editor, Twenty Nineteen has done far more to get that point across than the vanilla editor style ever could.

In that vein, and as is also hopefully clear from that discussion linked, removing Noto Serif from the editor, or bundling it, is likely not going to meet blocking pushback, even if we might want to pour one out for it.

I greatly appreciate this has been discussed in detail, thank you.

As Gary mentioned, "I’m fine with this change happening, but folks working on it should be aware that there are significant hurdles to overcome in order to avoid it creating a poor user experience.", I've added a needs-design-feedback to start the discussion on this as the decision to utilize bundled fonts or system fonts is more ux/ui related since both options satisfy the privacy/performance concerns.

I'm also adding the 2nd-opinion and dev-feedback to expand the discussion but wanted to requote Gary as 'A font change would need to be approved by a Gutenberg design lead.'

Once a final decision is made this can be introduced to Gutenberg first as that would then end up in the corresponding Core release slightly later.
There's an existing ticket on Gutenberg that can be reopened to continue the initial work there;
https://github.com/WordPress/gutenberg/issues/11648

P.S. For now I've dropped the needs-patch as a decision is first required for the patch forward, and the actual implementation would more likely be done via GutenGitHub.

#12 @karmatosed
5 years ago

  • Keywords needs-design removed

This ticket was mentioned in Slack in #core-privacy by audrasjb. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-themes by garrett-eclipse. View the logs.


5 years ago

This ticket was mentioned in Slack in #core-privacy by carike. View the logs.


4 years ago

Gurkenschreck commented on PR #71:


4 years ago
#17

Please make this happen!

#18 @aristath
4 years ago

Google Fonts and privacy has been a recurring issue...
A solution to this would be to locally host the Google Fonts. An implementation for themes has already been suggested in https://make.wordpress.org/themes/2020/09/25/new-package-to-allow-locally-hosting-webfonts/ and if that code (or a variation of it) finds its way in WP-Core, we can keep Noto Serif (or any other font-family we choose in the future) without any GDPR concerns.

This ticket was mentioned in Slack in #forums by aristath. View the logs.


4 years ago

#20 @aristath
4 years ago

#51402 was marked as a duplicate.

#21 @hedgefield
3 years ago

  • Keywords needs-design-feedback removed

As discussed, there are pros and cons to this. Privacy and speed are two great arguments for removing the Gfonts. Beautiful design and especially consistency (as the default WordPress system font stack will of course render a different font for MacOS as it does for Windows etc) can be mentioned as reasons against removing it. But I agree that I think the arguments against it are largely moot as the editor should render the font that the theme provides. We could still keep Noto in the way that Ari suggests, or we just rely on system fonts, which, while a bit plain, are fast and well-legible. As a fallback for themes to override, that seems fine.

@Joen
3 years ago

First stab at removing Noto Serif

#22 @Joen
3 years ago

Hi, I just attached a first stab at a patch to remove Noto Serif as the default editor font.

I would appreciate a sanity check on the code to see that I've tweaked the right files.

This patch needs to be paired with a Gutenberg PR, which I have here: https://github.com/WordPress/gutenberg/pull/26822

#23 @aristath
3 years ago

Tested this one along with the PR from https://github.com/WordPress/gutenberg/pull/26822, seems to be working as expected. It is a simple change and has already been approved by design.

Just one note, this PR leaves in src/wp-includes/script-loader.php this part which doesn't do anything and can/should be removed:

<?php
$fonts_url = '';
$styles->add( 'wp-editor-font', $fonts_url );
Last edited 3 years ago by aristath (previous) (diff)

#24 @garrett-eclipse
3 years ago

  • Keywords needs-refresh has-patch needs-testing added; 2nd-opinion dev-feedback removed
  • Milestone changed from Awaiting Review to 5.7
  • Owner set to garrett-eclipse
  • Status changed from new to reviewing

Thanks @Joen that's awesome, let's get this into 5.7.

I don't think the .env changes should be in the diff. You can specify the src folder when generating your patch;
svn diff src > XXXX.diff
Or if it includes tests;
svn diff src tests > XXXX.X.diff

I also learned something new after questioning that default font stack, very cool and the right choice I believe.

@Joen
3 years ago

#25 @Joen
3 years ago

Ack, apologies for including the .env file 🤦‍♂️ and thanks for catching it.

Ari catched another one in https://core.trac.wordpress.org/ticket/46169#comment:23, that the editor font which is no longer needed, was still registered as a dependency. So I removed that dependency, and did the same in the plugin in https://github.com/WordPress/gutenberg/pull/26822/commits/e26ccaab21126e1b0310b62d16b2ac12dc96da40. Props @aristath.

I believe the patch should be good now, but I'd appreciate sanity checks.

#26 @garrett-eclipse
3 years ago

  • Focuses ui css administration added
  • Keywords dev-feedback added; needs-refresh removed

That's awesome @Joen the patch is looking great.

As to the full removal of the font, I do wonder if plugins/themes used it and as such we should deprecate instead of remove. See the Open Sans deprecation;
$styles->add( 'open-sans', $open_sans_font_url ); // No longer used in core as of 4.6.

I'll let someone with more knowledge on that chime in before we do anything further.

This ticket was mentioned in Slack in #core-css by kirstyburgoine. View the logs.


3 years ago

#28 @sabernhardt
3 years ago

For anyone who has Noto Serif installed, could that still be in the font stack as a preferred system font (before Segoe UI)?

#29 @noisysocks
3 years ago

  • Keywords dev-feedback removed

Yes we'll need to keep the $style->add() for backwards compatibility. Copying what we do for open-sans should do the trick.

Thinking about the logistics of merging this for a moment: we will need to merge GB26822, then wait until packages in trunk are updated, and then commit this. Otherwise there'll be styling issues in trunk.

This ticket was mentioned in Slack in #core by lukecarbis. View the logs.


3 years ago

#31 @Joen
3 years ago

I've refreshed the GB PR and it remains ready to merge and is approved: https://github.com/WordPress/gutenberg/pull/26822

Would be nice land this one for 5.7.

#32 @hellofromTonya
3 years ago

  • Keywords needs-testing-info added

Steps for this ticket per @noisysocks

  1. Merge GB26822 ✅Has been merged. Done.
  2. Then wait until packages in trunk are updated
  3. Then commit this

@noisysocks @garrett-eclipse Is it okay to mark this ticket for commit?

In preparation for Testing Scrub scheduling, please provide more
information for the testers:

  • What are the testing steps?
  • Are there any testing dependencies such as a plugin or script?
  • What is the expected behavior after applying the patch?

#33 @noisysocks
3 years ago

Let's wait for packages to be updated (see #52334) before committing. Aiming to get that done next week.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


3 years ago

#35 @hellofromTonya
3 years ago

Per @ noisysocks, the packages are now updated and is no longer a blocker for this ticket.

Work remaining for this ticket is to restore the $style->add() code, per @noisysocks' feedback:

Yes we'll need to keep the $style->add() for backwards compatibility. Copying what we do for open-sans should do the trick.

@hellofromTonya
3 years ago

Restores $style->add() code for backwards compatibility, but adds notes for no longer used (similar to Open Sans).

#36 @hellofromTonya
3 years ago

@noisysocks Can you take do a code review of the last patch where I restored $style->add()?

Version 0, edited 3 years ago by hellofromTonya (next)

#37 @SergeyBiryukov
3 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 50155:

Block Editor: Drop Noto Serif in favor of system fonts.

This aims to improve privacy and performance of the editor.

Follow-up to [37361].

Props Joen, hellofromTonya, garrett-eclipse, aristath, noisysocks, hedgefield, pento, sabernhardt, joyously, yannkozon.
Fixes #46169.

#39 @rolfsiebers
3 years ago

  • Keywords needs-testing removed

I've tested this with @jeroenrotty and it seems to work correctly.

This ticket was mentioned in Slack in #core by francina. View the logs.


3 years ago

#41 @hellofromTonya
3 years ago

  • Keywords needs-testing-info removed

#43 @dd32
3 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Following up on this, I'm seeing a broken font-stack in trunk, PR #994 above is a sync of https://github.com/WordPress/gutenberg/pull/28960 to core.

#44 @SergeyBiryukov
3 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 50295:

Block Editor: Remove extra quotes from the default font stack for editor styles.

Follow-up to [50155].

Props dd32, Joen, kafleg.
Fixes #46169.

This ticket was mentioned in Slack in #core by monikarao. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.