-
-
Notifications
You must be signed in to change notification settings - Fork 23
Description
Describe the bug
When using Local fonts, the current code does a lot of unnesscarry, cacheable work. In particular does it try to validate the local fonts used on each template rendering by opening ALL font files it can find in the various directories. This leads to increased system pressure as the library used for those actions uses an temporary file everytime a fontfile is asked for it's name.
To Reproduce
Steps to reproduce the behavior:
- Install some fonts in your templates media/font-folders. Make absolutely sure to include eof, ttf, woff AND woff2 files!
- Edit the Typography settings of a Astroid managed style to use local fonts only.
- Visit the public facing site a bunch of times (or use an pressure tool like apache jmeter to simulate a good amount of requests)
- Monitor your php's tmp folder and see many (mostly broken) woff files.
Expected behavior
That available fonts are only queried in the administator / style editor and then cached.
I would suggest cacheing the fonts an template has either in an seperate cache-file or inside the database (i.e. Astroid\Template::getFonts should cache more strongly; explanation see in additional context). Cleaning would work by clearing it everytime the typography settings are updated.
Screenshots
System Informations (please complete the following information):
- Joomla Version: Joomla 5
- PHP Version PHP 8.4
- Astroid Version 3.3.0
- Device: n/a
- OS: n/a
- Browser n/a
Additional context
I've digged into it a bit and the code-flow is the following:
-
Astroid\Helper\Font::loadLocalFont is called to generate the
@font-facecss declaration for an locally stored font.
astroid-framework/framework/library/astroid/Helper/Font.php
Lines 301 to 327 in 932a160
public static function loadLocalFont($value) { $template = Framework::getTemplate(); $document = Factory::getApplication()->getDocument(); $wa = $document->getWebAssetManager(); $uploaded_fonts = $template->getFonts(); $template_media_fonts_path = JPATH_SITE . "/media/templates/site/{$template->template}/fonts"; $template_custom_fonts_path = JPATH_SITE . "/images/{$template->template}/fonts"; $font_custom_path = Uri::root() . "images/{$template->template}/fonts/"; if (file_exists($template_media_fonts_path)) { $font_path = Uri::root() . "media/templates/site/{$template->template}/fonts/"; } else { $font_path = Uri::root() . "templates/{$template->template}/fonts/"; } if (isset($uploaded_fonts[$value])) { $files = $uploaded_fonts[$value]['files']; $value = $uploaded_fonts[$value]['name']; foreach ($files as $file) { if (file_exists($template_custom_fonts_path . '/' . $file)) { $wa->addInlineStyle('@font-face { font-family: "' . $value . '"; src: url("' . $font_custom_path . $file . '");}'); } else { $wa->addInlineStyle('@font-face { font-family: "' . $value . '"; src: url("' . $font_path . $file . '");}'); } } } return $value; } -
Astroid\Template::getFonts get's called to get the fonts of the template. While this uses an class-level property to cache SOME data, this isn't persisted across requests. IMO would this be a good location to add some code to persist the cache in an file or the database.
astroid-framework/framework/library/astroid/Template.php
Lines 356 to 362 in 932a160
public function getFonts() { if (self::$fonts === null) { self::$fonts = Helper\Font::getUploadedFonts($this->template); } return self::$fonts; } -
Astroid\Helper\Font::getUploadedFonts then starts to gather directories where font-files could be found:
astroid-framework/framework/library/astroid/Helper/Font.php
Lines 123 to 147 in 932a160
public static function getUploadedFonts($template) { Framework::getDebugger()->start('local-fonts'); if (empty($template)) { return []; } $template_fonts_path = JPATH_SITE . "/templates/{$template}/fonts"; $template_media_fonts_path = JPATH_SITE . "/media/templates/site/{$template}/fonts"; $template_custom_fonts_path = JPATH_SITE . "/images/{$template}/fonts"; if (!file_exists($template_fonts_path) && !file_exists($template_media_fonts_path) && !file_exists($template_custom_fonts_path)) { return []; } $fonts = []; if (file_exists($template_media_fonts_path) || file_exists($template_fonts_path)) { if (file_exists($template_media_fonts_path)) $template_fonts_path = $template_media_fonts_path; $fonts = self::getLocalFonts($template_fonts_path); } if (file_exists($template_custom_fonts_path)) { $fonts = array_merge($fonts, self::getLocalFonts($template_custom_fonts_path)); } Framework::getDebugger()->stop('local-fonts'); return $fonts; } -
Astroid\Helper\Font::getLocalFonts iterates over EVERY FILE in the supplied directory and opens ALL of them in an afford to find the font's name:
astroid-framework/framework/library/astroid/Helper/Font.php
Lines 149 to 171 in 932a160
public static function getLocalFonts($template_fonts_path) { $fonts = []; $font_extensions = ['otf', 'ttf', 'woff']; foreach (scandir($template_fonts_path) as $font_path) { if (is_file($template_fonts_path . '/' . $font_path)) { $pathinfo = pathinfo($template_fonts_path . '/' . $font_path); if (in_array($pathinfo['extension'], $font_extensions)) { $font = \FontLib\Font::load($template_fonts_path . '/' . $font_path); $font->parse(); $fontname = $font->getFontFullName(); $fontid = 'library-font-' . Helper::slugify($fontname); if (!isset($fonts[$fontid])) { $fonts[$fontid] = []; $fonts[$fontid]['id'] = $fontid; $fonts[$fontid]['name'] = $fontname; $fonts[$fontid]['files'] = []; } $fonts[$fontid]['files'][] = $font_path; } } } return $fonts; } -
FontLib\Font::load is really just a delegator to the font's format handler:
astroid-framework/framework/library/vendor/phenx/php-font-lib/src/FontLib/Font.php
Line 46 in 932a160
$class = "WOFF\\File"; -
FontLib\WOFF\File::load does the actual loading for WOFF in this case. It opens an tempfile (
false) here means it uses/tmp/mktempinstead of an in-memory stream.
astroid-framework/framework/library/vendor/phenx/php-font-lib/src/FontLib/WOFF/File.php
Line 36 in 932a160
$fw = $this->getTempFile(false);
In additon it seems that this issue is a long-standing one that never got REALLY fixed: #239 #539 (and propably more).