DSpace / dspace-angular

DSpace User Interface built on Angular.io
https://wiki.lyrasis.org/display/DSDOC8x/
BSD 3-Clause "New" or "Revised" License
131 stars 430 forks source link

Currently not GDPR compliant with standard settings - reason external fonts #2077

Closed fapsi closed 1 year ago

fapsi commented 1 year ago

Describe the bug It would be good if all external assets are part of the application and dspace does not link to a CDN version with the default settings.

To Reproduce Steps to reproduce the behavior:

  1. Go to https://demo7.dspace.org
  2. See loaded external assets in browsers dev-mode->network-analysis with cache deactivated:

Expected behavior No requests to external assets. At a minimum a theme-able behavior.

Code to Change https://github.com/DSpace/dspace-angular/blob/430b43581a5b8dfe29d8d8a5f0d94b0a62006c81/src/main.browser.ts#L24

tdonohue commented 1 year ago

@fapsi : Thanks for noting this. It appears (to me) that the external font linked to on that line is unused. I think this is old code that can be safely removed.

I'll pull this over to the 7.6 board to see if I can find a volunteer. It should be simple to remove, just needs someone to create a PR.

artlowel commented 1 year ago

@tdonohue While you're right that droid sans is longer being used and can be removed, we still import a google font from css: https://github.com/DSpace/dspace-angular/blob/95b8dc3c111f82c321b2a467f74866ac5851a5ee/src/themes/dspace/styles/_theme_sass_variable_overrides.scss#L6

That droid sans line in main.browser.ts was something added at the very start to load those fonts more quickly, so you won't see it flash, but subsequently forgotten about 😅

To address this ticket we'll need something like fontsource to add these fonts as dependencies in package.json and bundle them with the rest of the code

tdonohue commented 1 year ago

@artlowel : Since that @import of a font is easily replaced in a custom theme (and is only in the "dspace" theme), I'm less concerned about it. It's not something that truly blocks GDPR compliancy in my opinion since anyone can swap the font easily.

However, the hardcoded font in main.browser.ts is a bigger issue as it is "inherited" to every theme anyone creates.

So, while I'm not against adding fontsource or similar, I don't think it's required to solve this bug ticket.