Include fonts in the Umami demo theme

Scope One is licensed under SIL Open Font Licence 1.1 https://scripts.sil.org/cms/scripts/page.php?site_id=nrsi&id=OFL_web. This is the same license as used for Font Awesome as listed at https://www.drupal.org/packaging-whitelist?title=font, therefore it looks like we can include it in the theme.

Open Sans uses Apache License 2.0 .This is the same license as used for which Facebook PHP SDK, Google Code Prettify, which are also on our packaging whitelist https://www.drupal.org/packaging-whitelist?page=1 so looks like we can also use it in the theme.

So this looks good to proceed from a licensing PoV. Also would be good for people installing without network connectivity etc. So let's do it!

Working with @eli-t and @mradcliffe at DrupalEurope. We can confirm that we have all just checked the licensing as described in #3. Should be good for including the font in the theme subject to relevant product manager review/approval.

I gave it a shot, downloading the fonts from google and embedded them in the base.css

Good work @parijke. The patch applied cleanly and all appears to work properly out-of-the-box.

Thank you for the compliment... it's my first ever contribution to core

Instead of downloading the fonts from google I recommend using https://google-webfonts-helper.herokuapp.com/fonts/

Also be aware that the line
+ src: local('Open Sans Regular'), local('OpenSans-Regular'),

Will mean that the font is loaded from the users own computer (local) before the font is loaded from the web site

Normally this won't be a problem BUT if the user has a different font with the same name OR they have only installed one weight of the font locally this will cause issues

It is the preferred way recommended by Google. I think because it makes sense to load from locsl if it exists there to optimize performance.

Can't foresee if this would cause any problems.

It adds an extra ~400kb to the packaged Drupal core archive, so I've added a conargument to the issue summary. Currently the TGZ is 16.24mb.

Setting back to needs review, and tagging, for performance. @alexpott's comment in the related issue sounds like a tacit approval, but I'd like to hear a performance opinion.

IMO the privacy and reliability pro arguments have a lot of weight. I think we can live with the extra size.

I made some mess in the summary. Cleaning it up.

We could reduce somewhat if we decide to support only Modern Browsers. That would skip the need of packaging the .eot .svg and .ttf files

We surely don't need the .eot file at least, as we don't support IE8 and below anyway? IE9 and higher will use the .woff if I understand correctly.

I believe so too.... Anyone who can approve this before I make a new patch? @kjay?

Looks like #1475972: Drupal.org distribution packaging requirements, specifically https://www.drupal.org/node/1475972#gplv2-compatible-licenses contradicts my reasoning in #3, as pointed out by @laurii on today's initiative call ☹️

Can this https://www.drupal.org/project/drupal/issues/2936841 be done with the fonts as well? That would address the con mentioned in #13

@Eliot is there a difference in licensing using the fonts online or in the distribution? If not, the licensing discussion shouldn't affect this issue but goes for using the fonts in general, right?

Looks like #1475972: Drupal.org distribution packaging requirements, specifically https://www.drupal.org/node/1475972#gplv2-compatible-licenses contradicts my reasoning in #3, as pointed out by @laurii on today's initiative call ☹️

Discussed in this week's OOTB call and @smaz noted that #1475972: Drupal.org distribution packaging requirements states:

Drupal also permits GPL-incompatible non-code assets (e.g. fonts, icons, etc.) to be packaged and/or distributed as long as the maintainer has the right to distribute them (i.e. they must have a free/libre license that permits them to be distributed “in aggregate” with GPL code). Such licenses are whitelisted as “GPLv-friendly”.

Despite Open Sans being Apache 2.0 and is not white-listed as GPLv2 compatible, being a font, it seems reasonable to conclude that we are able to bundle it so long as Apache 2.0 permits us to do so. Checking the Apache 2.0 license is therefore probably the next task.

tagging for badcamp 2018

A well-maintained Unicode font will be updated frequently across various character sets. New emoji are added regularly and there are improvements for various languages.

This might not be a problem since the demo content is relatively static. Regular updates to the packaged fonts, and some discouragement from using the font in production, should be planned on.

Tagging for upcoming contribution days.

Hi All,

Just checking in to see if someone could summarize what we need to have this marked as RTBC. It looks like we have a working patch and general agreement about including the fonts.

The main issue that seems to be outstanding is @drumm's thought about fonts being updated, which is mitigated by our content being static and not relying on emojis and things like that.

With that being the case, I _think_ we could move this to RTBC. Anyone against that?

Adding some tags for visibility.

I am removing the Novice tag from this issue because there isn't a task for a Novice to do. This might change. I queued the patch in #5 for 9.2.x. It may make sense to mark this RTBC or try to re-upload the patch for 9.2.x.

Since no one has disagreed with #31, I'm going to mark this as RTBC so we can get a frontend framework manager to review it.

Bumping this to major, we need to give this serious attention very quickly given the court rulings in the EU about Google Fonts.

By including the font in Drupal core (#37), we eliminate all the potential legal issues?

I tested #37 using DrupalPod.

Fonts were loaded locally, without Google's domain.

Webpagetest.org report: https://www.webpagetest.org/result/220308_BiDc0H_ASF/

Screenshot from that report:

Scope One is a compatible license meaning those assets could be packaged in Drupal. 👍

Unfortunately, Open Sans is licensed under Apache 2.0 which is not compatible with our licensing, meaning we can't include it in the package. 😓 We will likely have to try to find an alternative font which would be licensed under a compatible license.

I have installed with and without the patch in #49 on Drupal 9.3.9 on gitpod so we can compare Umami with Open Sans and Public Sans.

Here is the home page with Open Sans (left, unpatched) and Public Sans (right patched). Note the privacy badger warning for calling Google fonts does not show up when patched.

Full homepage with Open Sans

Full homepage with Public Sans

Article page with Open Sans

Article page with Public Sans

Recipe page with Open Sans

Recipe page with Public Sans

Individual recipe with Open Sans

Individual recipe with Public Sans

Tag page with Open Sans

Tag page with Public Sans

Individual article with Open Sans

Individual article with Public Sans

Those screenshots with Public Sans look great to me. I'm not a designer so will defer to @kjay to verify, but this looks like a really good solution.

@lauriii has suggested a couple of other alternative fonts with licences compatible with Drupal:

• https://fonts.google.com/specimen/Inter
• https://fonts.google.com/specimen/Source+Sans+Pro

Considering these two alongside Public Sans, it looks like Inter is actually the closest font to what we're already proposing so I'll provide another patch to trial that instead and generate some more screenshots.

Now I'm not too sure if Inter is the closest suggestion so here's the home page in the original font, the Public Sans font used in the @ressa patch and the two suggested by @lauriii for design feedback/discussion.

Open Sans (original)

Public Sans (as suggested in #45)

Inter

Source Sans Pro

On closer consideration, I think as far as the character shapes are concerned, Source Sans Pro is the closest match and best works with the design, but some small amount of CSS work would be needed to adjust the size and spacing to more closely match the current font.

Agreed with @Eli-T about the shape: Source Sans Pro is closer to Open Sans, so the style would match better.

But be aware Source Sans Pro font x-size is smaller, meaning using this one would probably need a follow-up to generally increase font sizes. The design tendency is to use bigger fonts than the ones resulting with this changes (even the same font size is set).

Maybe the second closer font on a style perspective would be Inter.

I'm not sure why this issue says #53 failed, since it's green.

Are we good to put this to RTBC?

We still need to address #51.

+++ b/core/profiles/demo_umami/themes/umami/css/base.css
@@ -3,6 +3,67 @@
+    url("../fonts/source-sans-pro-v21-latin-regular.woff") format("woff"); /* Chrome 6+, Firefox 3.6+, IE 9+, Safari 5.1+ */

Let's create a Drupal 10 version of the patch without the "woff" fonts because all browsers supported by Drupal 10 support woff2.

I have rerolled the patch against 10.1x.
also addressed #60 and applied the changes.

Thank you for the patch! Changing status to Needs review to trigger a CI run.

Thanks @Anchal_gupta for the patch! There are a couple issues:

1. The font-face declarations are all broken, I believe this is due to the fact that the src property is not terminated with a semicolon. Note the url definition is trailed by a comma when it should be a semicolon.

   /* source-sans-pro-regular - latin */
   @font-face {
     font-family: "Source Sans Pro";
     src:
       local("Source Sans Pro"),
       url("../fonts/source-sans-pro-v21-latin-regular.woff2") format("woff2"), /* Chrome 26+, Opera 23+, Firefox 39+ */
   
     font-weight: 400;
     font-style: normal;
   }
       

2. Since we're no longer using the .woff files, we shouldn't include them in the patch. Please remove these and re-upload.

please ignore this patch. I have again reuploaded the patch in the same thread

Thanks for your suggestions @andy-blum
I have addressed point 1. and 2. on comment #65
Here, I have Attached a patch again with fixed the error

Patch in #67 still contains .woff files.

Additionally, to aid in the review of your fixes please do the following:

• name your patches in the format <issue number>-<comment number> so that the patch number and comment numbers line up instead of incrementing the numbers of the patch.
• provide an interdiff between incremental patches to help identify the changes from patch to patch.

If you're at drupalcon right now and would like some assistance in any of this, you can reach out to one of the mentors in mentored contribution or find me in the orange shirt at the front-end table in general contribution.

Thanks, @andy-blum for consistently reviewing my fixes and noticing my mistake, and assisting me. Really I grateful to you and next time I will remember that and follow this step
.Woff Files have been removed
I have attach the patch and interdiff also.

Okay, Thanks @ressa, I changed the status to trigger test.

Patch #70 works - the only change I'd request is to remove the comments around the font-face declarations. They don't add anything particularly valuable to core, so let's remove those and then we should be in good shape.

New patch & interdiff

New patch to satisfy the linter

Even though this isn't changing the font-size, I think the overall result looks OK. So moving this to RTBC.

This was reviewed by @ckrina and I have reviewed this as well. Also discussed this with @justafish and @xjm and we agreed to backport this down until 9.5.x because Umami has no BC policy.

Committed and pushed 1f1b26680c to 10.1.x and 141f888d82 to 10.0.x and ff1f52eb41 to 9.5.x. Thanks!

As per @lauriii and @xjm backported to 9.5.x

Committed 9925f1d and pushed to 10.1x. Thanks!
Committed 141f888 and pushed to 10.0.x. Thanks!
Committed ff1f52e and pushed to 9.5.x. Thanks!