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

[OJS 3.1.1.0] TinyMCE calls for google font even when enable_cdn is disabled #3759

Closed
MarHerUMR opened this issue Jun 1, 2018 · 6 comments
Closed
Assignees
Labels
Bug:1:Low A bug that does not have a severe consequence or affects a small number of users.

Comments

@MarHerUMR
Copy link
Contributor

In to share as little data as possible we disabled the enable_cdn option in the config. But it turns out that the backend still makes calls to //fonts.googleapis.com/css?family=Noto+Sans:400,400italic,700,700italic.

As far as I can see the call is made because the plugins/generic/tinymce/styles/content.css has the import:

@import url('//fonts.googleapis.com/css?family=Noto+Sans:400,400italic,700,700italic');

I would have assumed that the google fonts would be disabled in the front- and backend when disabling enable_cdn (especially because of the advise to disable it because of GDPR). But I'm not quite shure how this can be disabled here.

@asmecher
Copy link
Member

asmecher commented Jun 1, 2018

@MarHerUMR, at a glance it looks like there's a separate Noto Sans import (in lib/pkp/classes/template/PKPTemplateManager.inc.php) that is dependent on the CDN setting being enabled -- you should be able to remove the reference in plugins/generic/tinymce/styles/content.css without any negative effects. Could you try this and confirm?

@NateWr
Copy link
Contributor

NateWr commented Jun 4, 2018

plugins/generic/tinymce/styles/content.css loads styles that are only applied in the iframe of the tinymce editor. That means that the noto font isn't loaded separately, and removing it from content.css will cause the content inside the editor to not be styled properly.

However, @MarHerUMR, if you are already disabling the CDN, then you won't have this font anyway, so you're probably safe removing that line in content.css for the time being.

We'll probably need to figure out a solution that takes enable_cdn into account when loading the TinyMCE content CSS file. I'll take this into account with the TinyMCE work I'm doing in #3594.

@NateWr NateWr self-assigned this Jun 4, 2018
@NateWr NateWr added the Bug:1:Low A bug that does not have a severe consequence or affects a small number of users. label Jun 4, 2018
@NateWr
Copy link
Contributor

NateWr commented Jun 4, 2018

Ooh, it supports multiple stylesheets. 🙏 https://www.tinymce.com/docs-3x/reference/configuration/Configuration3x@content_css/ Ok, let me address this real quick.

NateWr added a commit to NateWr/pkp-lib that referenced this issue Jun 4, 2018
@NateWr
Copy link
Contributor

NateWr commented Jun 4, 2018

PR:
#3767

Tests:
pkp/ojs#1999

@NateWr
Copy link
Contributor

NateWr commented Jun 5, 2018

This needs a commit to the tinymce plugin to add the new/modified CSS.

@NateWr
Copy link
Contributor

NateWr commented Apr 11, 2019

This was addressed as part of #3594

@NateWr NateWr closed this as completed Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug:1:Low A bug that does not have a severe consequence or affects a small number of users.
Projects
None yet
Development

No branches or pull requests

3 participants