diff --git a/.horde.yml b/.horde.yml index 5f4d28bb..c0c95a16 100644 --- a/.horde.yml +++ b/.horde.yml @@ -55,7 +55,7 @@ dependencies: horde/compress: ^3 horde/compress_fast: ^2 horde/controller: ^3 - horde/cssminify: ^2 + horde/cssminify: ^2.0.0-beta2 horde/data: ^3 horde/date: ^3 horde/exception: ^3 diff --git a/lib/Horde/Themes/Css.php b/lib/Horde/Themes/Css.php index ddc9c563..e0d9aa4a 100644 --- a/lib/Horde/Themes/Css.php +++ b/lib/Horde/Themes/Css.php @@ -14,6 +14,9 @@ * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 * @package Core */ + +use Horde\Log\Logger; + class Horde_Themes_Css { /** @@ -253,8 +256,19 @@ public function getBaseStylesheetList() */ public function loadCssFiles($files) { + global $injector; + $compress = new Horde_Themes_Css_Compress(); - return $compress->compress($files); + + // Try to get PSR-3 logger for error reporting + $logger = null; + try { + $logger = $injector->get(Logger::class); + } catch (Exception $e) { + // Logger not available, continue without logging + } + + return $compress->compress($files, $logger); } } diff --git a/lib/Horde/Themes/Css/Cache/File.php b/lib/Horde/Themes/Css/Cache/File.php index 03e96d41..cbcac422 100644 --- a/lib/Horde/Themes/Css/Cache/File.php +++ b/lib/Horde/Themes/Css/Cache/File.php @@ -12,6 +12,8 @@ * @package Core */ +use Horde\Log\Logger; + /** * Filesystem backend for the CSS caching library. * @@ -28,7 +30,7 @@ class Horde_Themes_Css_Cache_File extends Horde_Themes_Css_Cache */ public function process($css, $cacheid) { - global $registry; + global $registry, $injector; if (!empty($this->_params['filemtime'])) { foreach ($css as &$val) { @@ -49,8 +51,17 @@ public function process($css, $cacheid) if (!file_exists($path)) { $compress = new Horde_Themes_Css_Compress(); + + // Try to get PSR-3 logger for error reporting + $logger = null; + try { + $logger = $injector->get(Logger::class); + } catch (Exception $e) { + // Logger not available, continue without logging + } + $temp = Horde_Util::getTempFile('staticcss', true, $js_fs); - if (!file_put_contents($temp, $compress->compress($css), LOCK_EX) || + if (!file_put_contents($temp, $compress->compress($css, $logger), LOCK_EX) || !chmod($temp, 0o777 & ~umask()) || !rename($temp, $path)) { Horde::log('Could not write cached CSS file to disk.', 'EMERG'); diff --git a/lib/Horde/Themes/Css/Cache/HordeCache.php b/lib/Horde/Themes/Css/Cache/HordeCache.php index 920535c2..3c832ccd 100644 --- a/lib/Horde/Themes/Css/Cache/HordeCache.php +++ b/lib/Horde/Themes/Css/Cache/HordeCache.php @@ -12,6 +12,8 @@ * @package Core */ +use Horde\Log\Logger; + /** * Horde_Cache backend for the CSS caching library. * @@ -48,7 +50,16 @@ public function process($css, $cacheid) // Do lifetime checking here, not on cache display page. if (!$cache->exists($sig, empty($this->_params['lifetime']) ? 0 : $this->_params['lifetime'])) { $compress = new Horde_Themes_Css_Compress(); - $cache->set($sig, $compress->compress($css)); + + // Try to get PSR-3 logger for error reporting + $logger = null; + try { + $logger = $injector->get(Logger::class); + } catch (Exception $e) { + // Logger not available, continue without logging + } + + $cache->set($sig, $compress->compress($css, $logger)); } return [ diff --git a/lib/Horde/Themes/Css/Compress.php b/lib/Horde/Themes/Css/Compress.php index 08328f23..6b81a94a 100644 --- a/lib/Horde/Themes/Css/Compress.php +++ b/lib/Horde/Themes/Css/Compress.php @@ -1,23 +1,32 @@ * @category Horde - * @copyright 2014-2017 Horde LLC + * @copyright 2014-2026 Horde LLC * @license http://www.horde.org/licenses/lgpl21 LGPL 2.1 * @package Core * @since 2.12.0 @@ -29,25 +38,62 @@ class Horde_Themes_Css_Compress * to a string. * * @param array $css See Horde_Themes_Css#getStylesheets(). + * @param LoggerInterface|null $logger Optional PSR-3 logger for error reporting. * * @return string CSS data. */ - public function compress($css) + public function compress($css, ?LoggerInterface $logger = null) { global $browser, $conf, $injector; $files = []; foreach ($css as $val) { - $files[$val['uri']] = $val['fs']; + if (!isset($val['uri']) || !isset($val['fs'])) { + continue; + } + try { + $files[] = new CssFile((string) $val['uri'], (string) $val['fs']); + } catch (InvalidArgumentException $e) { + // Skip unreadable files + if ($logger !== null) { + $logger->warning( + 'Skipping unreadable CSS file: {file}', + ['file' => $val['fs'] ?? 'unknown', 'exception' => $e->getMessage()] + ); + } + continue; + } + } + + if (empty($files)) { + return ''; + } + + $dataUrlCallback = null; + if (empty($conf['nobase64_img']) && $browser->hasFeature('dataurl')) { + $dataUrlCallback = new UrlCallback([$this, 'dataurlCallback']); + } + + // Use provided logger or attempt to get from injector + if ($logger === null) { + try { + $logger = $injector->get(Logger::class); + } catch (Exception $e) { + // Logger not available, continue without logging + $logger = null; + } } - $parser = new Horde_CssMinify_CssParser($files, [ - 'dataurl' => (empty($conf['nobase64_img']) && $browser->hasFeature('dataurl')) ? [$this, 'dataurlCallback'] : null, - 'import' => [$this, 'importCallback'], - 'logger' => $injector->getInstance('Horde_Log_Logger'), - ]); + $minifier = new CssParserMinifier( + new FileCollectionInput(...$files), + new Settings( + dataUrlCallback: $dataUrlCallback, + importCallback: new ImportCallback([$this, 'importCallback']), + logger: $logger + ) + ); - return $parser->minify(); + return $minifier->minify(); } /** diff --git a/test/Horde_Themes_Css_CompressTest.php b/test/Horde_Themes_Css_CompressTest.php new file mode 100644 index 00000000..b979383d --- /dev/null +++ b/test/Horde_Themes_Css_CompressTest.php @@ -0,0 +1,145 @@ +fixturesPath = __DIR__ . '/fixtures/'; + + // Create fixtures directory if it doesn't exist + if (!is_dir($this->fixturesPath)) { + mkdir($this->fixturesPath, 0o777, true); + } + + // Create test CSS file + file_put_contents( + $this->fixturesPath . 'test.css', + 'body { color: red; margin: 10px; }' + ); + + file_put_contents( + $this->fixturesPath . 'with-import.css', + '@import "shared.css"; body { background: white; }' + ); + + file_put_contents( + $this->fixturesPath . 'shared.css', + '.header { padding: 20px; }' + ); + + file_put_contents( + $this->fixturesPath . 'with-url.css', + 'body { background-image: url(../images/bg.png); }' + ); + } + + protected function tearDown(): void + { + // Cleanup fixtures + $files = glob($this->fixturesPath . '*.css'); + foreach ($files as $file) { + unlink($file); + } + if (is_dir($this->fixturesPath)) { + rmdir($this->fixturesPath); + } + } + + public function testCompressBasicCss(): void + { + $compress = new Horde_Themes_Css_Compress(); + + $css = [ + ['uri' => 'test.css', 'fs' => $this->fixturesPath . 'test.css'], + ]; + + $result = $compress->compress($css); + + $this->assertStringContainsString('color:red', $result); + $this->assertStringContainsString('margin:10px', $result); + } + + public function testCompressMultipleFiles(): void + { + $compress = new Horde_Themes_Css_Compress(); + + $css = [ + ['uri' => 'test.css', 'fs' => $this->fixturesPath . 'test.css'], + ['uri' => 'shared.css', 'fs' => $this->fixturesPath . 'shared.css'], + ]; + + $result = $compress->compress($css); + + $this->assertStringContainsString('color:red', $result); + $this->assertStringContainsString('.header', $result); + } + + public function testCompressHandlesInvalidFiles(): void + { + $compress = new Horde_Themes_Css_Compress(); + + $css = [ + ['uri' => 'missing.css', 'fs' => $this->fixturesPath . 'nonexistent.css'], + ]; + + // Should not throw, returns empty string + $result = $compress->compress($css); + + $this->assertSame('', $result); + } + + public function testCompressHandlesMissingKeys(): void + { + $compress = new Horde_Themes_Css_Compress(); + + $css = [ + ['uri' => 'test.css'], // Missing 'fs' + ['fs' => $this->fixturesPath . 'test.css'], // Missing 'uri' + [], // Missing both + ]; + + // Should not throw, skips invalid entries + $result = $compress->compress($css); + + $this->assertSame('', $result); + } + + public function testCompressHandlesEmptyInput(): void + { + $compress = new Horde_Themes_Css_Compress(); + + $result = $compress->compress([]); + + $this->assertSame('', $result); + } + + public function testDataurlCallback(): void + { + $compress = new Horde_Themes_Css_Compress(); + + // Test callback is callable + $result = $compress->dataurlCallback('/path/to/image.png'); + + // Should return something (actual implementation depends on Horde_Themes_Image) + $this->assertIsString($result); + } + + public function testImportCallback(): void + { + $compress = new Horde_Themes_Css_Compress(); + + // Test callback returns array with uri and fs + $result = $compress->importCallback('test.css'); + + $this->assertIsArray($result); + $this->assertCount(2, $result); + } +} diff --git a/test/Horde_Themes_Css_Compress_IntegrationTest.php b/test/Horde_Themes_Css_Compress_IntegrationTest.php new file mode 100644 index 00000000..09dacf10 --- /dev/null +++ b/test/Horde_Themes_Css_Compress_IntegrationTest.php @@ -0,0 +1,90 @@ +assertTrue(class_exists('Horde\CssMinify\CssParserMinifier')); + $this->assertTrue(class_exists('Horde\CssMinify\Input\CssFile')); + $this->assertTrue(class_exists('Horde\CssMinify\Input\FileCollectionInput')); + $this->assertTrue(class_exists('Horde\CssMinify\Settings')); + $this->assertTrue(class_exists('Horde\CssMinify\UrlCallback')); + $this->assertTrue(class_exists('Horde\CssMinify\ImportCallback')); + } + + public function testCssFileValidation(): void + { + $tempFile = tempnam(sys_get_temp_dir(), 'css'); + file_put_contents($tempFile, 'body { color: red; }'); + + // Modern API validates at construction + $file = new \Horde\CssMinify\Input\CssFile('test.css', $tempFile); + + $this->assertSame('test.css', $file->uri); + $this->assertSame($tempFile, $file->filepath); + + unlink($tempFile); + } + + public function testCssFileThrowsOnInvalid(): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('File not readable'); + + new \Horde\CssMinify\Input\CssFile('missing.css', '/nonexistent/path.css'); + } + + public function testModernApiTypeSafety(): void + { + $tempFile = tempnam(sys_get_temp_dir(), 'css'); + file_put_contents($tempFile, 'body { color: red; }'); + + $file = new \Horde\CssMinify\Input\CssFile('test.css', $tempFile); + $collection = new \Horde\CssMinify\Input\FileCollectionInput($file); + $minifier = new \Horde\CssMinify\CssParserMinifier($collection); + + $result = $minifier->minify(); + + $this->assertIsString($result); + $this->assertStringContainsString('color:red', $result); + + unlink($tempFile); + } + + public function testSettingsImmutability(): void + { + $urlCallback = new \Horde\CssMinify\UrlCallback(fn ($p) => $p); + $settings = new \Horde\CssMinify\Settings(dataUrlCallback: $urlCallback); + + // Properties are readonly + $this->assertSame($urlCallback, $settings->dataUrlCallback); + } + + public function testCompressCodePath(): void + { + // This verifies the updated Compress.php code paths compile + $reflection = new \ReflectionClass('Horde_Themes_Css_Compress'); + + // Verify compress method exists + $this->assertTrue($reflection->hasMethod('compress')); + + // Verify it uses modern API classes (check use statements in file) + $filename = $reflection->getFileName(); + $content = file_get_contents($filename); + + $this->assertStringContainsString('use Horde\CssMinify\CssParserMinifier', $content); + $this->assertStringContainsString('use Horde\CssMinify\Input\CssFile', $content); + $this->assertStringContainsString('use Horde\CssMinify\Input\FileCollectionInput', $content); + $this->assertStringContainsString('use Horde\CssMinify\Settings', $content); + } +}