diff --git a/apps/theming/lib/Controller/IconController.php b/apps/theming/lib/Controller/IconController.php index e82faf78a798a..ed4026944bd28 100644 --- a/apps/theming/lib/Controller/IconController.php +++ b/apps/theming/lib/Controller/IconController.php @@ -21,6 +21,7 @@ use OCP\AppFramework\Http\NotFoundResponse; use OCP\AppFramework\Http\Response; use OCP\Files\NotFoundException; +use OCP\IConfig; use OCP\IRequest; class IconController extends Controller { @@ -30,6 +31,7 @@ class IconController extends Controller { public function __construct( $appName, IRequest $request, + private IConfig $config, private ThemingDefaults $themingDefaults, private IconBuilder $iconBuilder, private ImageManager $imageManager, @@ -79,7 +81,7 @@ public function getThemedIcon(string $app, string $image): Response { * Return a 32x32 favicon as png * * @param string $app ID of the app - * @return DataDisplayResponse|FileDisplayResponse|NotFoundResponse + * @return DataDisplayResponse|FileDisplayResponse|NotFoundResponse * @throws \Exception * * 200: Favicon returned @@ -95,12 +97,14 @@ public function getFavicon(string $app = 'core'): Response { $response = null; $iconFile = null; + // retrieve instance favicon try { $iconFile = $this->imageManager->getImage('favicon', false); $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); } catch (NotFoundException $e) { } - if ($iconFile === null && $this->imageManager->shouldReplaceIcons()) { + // retrieve or generate app specific favicon + if (($this->imageManager->canConvert('PNG') || $this->imageManager->canConvert('SVG')) && $this->imageManager->canConvert('ICO')) { $color = $this->themingDefaults->getColorPrimary(); try { $iconFile = $this->imageManager->getCachedImage('favIcon-' . $app . $color); @@ -113,9 +117,10 @@ public function getFavicon(string $app = 'core'): Response { } $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); } + // fallback to core favicon if ($response === null) { $fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon.png'; - $response = new DataDisplayResponse($this->fileAccessHelper->file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $response = new DataDisplayResponse($this->fileAccessHelper->file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/png']); } $response->cacheFor(86400); return $response; @@ -125,7 +130,7 @@ public function getFavicon(string $app = 'core'): Response { * Return a 512x512 icon for touch devices * * @param string $app ID of the app - * @return DataDisplayResponse|FileDisplayResponse|NotFoundResponse + * @return DataDisplayResponse|FileDisplayResponse|NotFoundResponse * @throws \Exception * * 200: Touch icon returned @@ -140,12 +145,14 @@ public function getTouchIcon(string $app = 'core'): Response { } $response = null; + // retrieve instance favicon try { $iconFile = $this->imageManager->getImage('favicon'); - $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => $iconFile->getMimeType()]); } catch (NotFoundException $e) { } - if ($this->imageManager->shouldReplaceIcons()) { + // retrieve or generate app specific touch icon + if ($this->imageManager->canConvert('PNG')) { $color = $this->themingDefaults->getColorPrimary(); try { $iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app . $color); @@ -158,6 +165,7 @@ public function getTouchIcon(string $app = 'core'): Response { } $response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']); } + // fallback to core touch icon if ($response === null) { $fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon-touch.png'; $response = new DataDisplayResponse($this->fileAccessHelper->file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/png']); diff --git a/apps/theming/lib/Controller/ThemingController.php b/apps/theming/lib/Controller/ThemingController.php index 8bb9841ae5596..3b83e755b65da 100644 --- a/apps/theming/lib/Controller/ThemingController.php +++ b/apps/theming/lib/Controller/ThemingController.php @@ -345,6 +345,7 @@ public function undoAll(): DataResponse { #[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)] public function getImage(string $key, bool $useSvg = true) { try { + $useSvg = $useSvg && $this->imageManager->canConvert('SVG'); $file = $this->imageManager->getImage($key, $useSvg); } catch (NotFoundException $e) { return new NotFoundResponse(); @@ -355,13 +356,8 @@ public function getImage(string $key, bool $useSvg = true) { $csp->allowInlineStyle(); $response->setContentSecurityPolicy($csp); $response->cacheFor(3600); - $response->addHeader('Content-Type', $this->config->getAppValue($this->appName, $key . 'Mime', '')); + $response->addHeader('Content-Type', $file->getMimeType()); $response->addHeader('Content-Disposition', 'attachment; filename="' . $key . '"'); - if (!$useSvg) { - $response->addHeader('Content-Type', 'image/png'); - } else { - $response->addHeader('Content-Type', $this->config->getAppValue($this->appName, $key . 'Mime', '')); - } return $response; } diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index 63f4559970dfe..9fb57ffbc027a 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -7,6 +7,7 @@ namespace OCA\Theming; use Imagick; +use ImagickDraw; use ImagickPixel; use OCP\Files\SimpleFS\ISimpleFile; @@ -30,17 +31,18 @@ public function __construct( * @return string|false image blob */ public function getFavicon($app) { - if (!$this->imageManager->shouldReplaceIcons()) { + if (!$this->imageManager->canConvert('PNG')) { return false; } try { - $favicon = new Imagick(); - $favicon->setFormat('ico'); $icon = $this->renderAppIcon($app, 128); if ($icon === false) { return false; } - $icon->setImageFormat('png32'); + $icon->setImageFormat('PNG32'); + + $favicon = new Imagick(); + $favicon->setFormat('ICO'); $clone = clone $icon; $clone->scaleImage(16, 0); @@ -96,7 +98,9 @@ public function getTouchIcon($app) { * @return Imagick|false */ public function renderAppIcon($app, $size) { - $appIcon = $this->util->getAppIcon($app); + $supportSvg = $this->imageManager->canConvert('SVG'); + // retrieve app icon + $appIcon = $this->util->getAppIcon($app, $supportSvg); if ($appIcon instanceof ISimpleFile) { $appIconContent = $appIcon->getContent(); $mime = $appIcon->getMimeType(); @@ -110,80 +114,101 @@ public function renderAppIcon($app, $size) { if ($appIconContent === false || $appIconContent === '') { return false; } + + $appIconIsSvg = ($mime === 'image/svg+xml' || str_starts_with($appIconContent, 'themingDefaults->getColorPrimary(); - - // generate background image with rounded corners - $cornerRadius = 0.2 * $size; - $background = '' - . '' - . '' - . ''; - // resize svg magic as this seems broken in Imagemagick - if ($mime === 'image/svg+xml' || substr($appIconContent, 0, 4) === 'setBackgroundColor(new ImagickPixel('transparent')); - $tmp->setResolution(72, 72); - $tmp->readImageBlob($svg); - $x = $tmp->getImageWidth(); - $y = $tmp->getImageHeight(); - $tmp->destroy(); - - // convert svg to resized image + // construct original image object + try { $appIconFile = new Imagick(); - $resX = (int)(72 * $size / $x); - $resY = (int)(72 * $size / $y); - $appIconFile->setResolution($resX, $resY); $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); - $appIconFile->readImageBlob($svg); - - /** - * invert app icons for bright primary colors - * the default nextcloud logo will not be inverted to black - */ - if ($this->util->isBrightColor($color) - && !$appIcon instanceof ISimpleFile - && $app !== 'core' - ) { - $appIconFile->negateImage(false); + + if ($appIconIsSvg) { + // handle SVG images + // ensure proper XML declaration + if (str_starts_with($appIconContent, '' . $appIconContent; + } else { + $svg = $appIconContent; + } + // get dimensions for resolution calculation + $tmp = new Imagick(); + $tmp->setBackgroundColor(new ImagickPixel('transparent')); + $tmp->setResolution(72, 72); + $tmp->readImageBlob($svg); + $x = $tmp->getImageWidth(); + $y = $tmp->getImageHeight(); + $tmp->destroy(); + // set resolution for proper scaling + $resX = (int)(72 * $size / $x); + $resY = (int)(72 * $size / $y); + $appIconFile->setResolution($resX, $resY); + $appIconFile->readImageBlob($svg); + } else { + // handle non-SVG images + $appIconFile->readImageBlob($appIconContent); } - } else { - $appIconFile = new Imagick(); - $appIconFile->setBackgroundColor(new ImagickPixel('transparent')); - $appIconFile->readImageBlob($appIconContent); + } catch (\ImagickException $e) { + return false; } - // offset for icon positioning - $padding = 0.15; - $border_w = (int)($appIconFile->getImageWidth() * $padding); - $border_h = (int)($appIconFile->getImageHeight() * $padding); - $innerWidth = ($appIconFile->getImageWidth() - $border_w * 2); - $innerHeight = ($appIconFile->getImageHeight() - $border_h * 2); - $appIconFile->adaptiveResizeImage($innerWidth, $innerHeight); - // center icon - $offset_w = (int)($size / 2 - $innerWidth / 2); - $offset_h = (int)($size / 2 - $innerHeight / 2); - - $finalIconFile = new Imagick(); - $finalIconFile->setBackgroundColor(new ImagickPixel('transparent')); - $finalIconFile->readImageBlob($background); - $finalIconFile->setImageVirtualPixelMethod(Imagick::VIRTUALPIXELMETHOD_TRANSPARENT); - $finalIconFile->setImageArtifact('compose:args', '1,0,-0.5,0.5'); - $finalIconFile->compositeImage($appIconFile, Imagick::COMPOSITE_ATOP, $offset_w, $offset_h); - $finalIconFile->setImageFormat('png24'); - if (defined('Imagick::INTERPOLATE_BICUBIC') === true) { - $filter = Imagick::INTERPOLATE_BICUBIC; - } else { - $filter = Imagick::FILTER_LANCZOS; + // calculate final image size and position + $padding = 0.85; + $original_w = $appIconFile->getImageWidth(); + $original_h = $appIconFile->getImageHeight(); + $contentSize = (int)floor($size * $padding); + $scale = min($contentSize / $original_w, $contentSize / $original_h); + $new_w = max(1, (int)floor($original_w * $scale)); + $new_h = max(1, (int)floor($original_h * $scale)); + $offset_w = (int)floor(($size - $new_w) / 2); + $offset_h = (int)floor(($size - $new_h) / 2); + $cornerRadius = 0.2 * $size; + $color = $this->themingDefaults->getColorPrimary(); + // resize original image + $appIconFile->resizeImage($new_w, $new_h, Imagick::FILTER_LANCZOS, 1); + /** + * invert app icons for bright primary colors + * the default nextcloud logo will not be inverted to black + */ + if ($this->util->isBrightColor($color) + && !$appIcon instanceof ISimpleFile + && $app !== 'core' + ) { + $appIconFile->negateImage(false); + } + // construct final image object + try { + // image background + $finalIconFile = new Imagick(); + $finalIconFile->setBackgroundColor(new ImagickPixel('transparent')); + // icon background + $finalIconFile->newImage($size, $size, new ImagickPixel('transparent')); + $draw = new ImagickDraw(); + $draw->setFillColor($color); + $draw->roundRectangle(0, 0, $size - 1, $size - 1, $cornerRadius, $cornerRadius); + $finalIconFile->drawImage($draw); + $draw->destroy(); + // overlay icon + $finalIconFile->setImageVirtualPixelMethod(Imagick::VIRTUALPIXELMETHOD_TRANSPARENT); + $finalIconFile->setImageArtifact('compose:args', '1,0,-0.5,0.5'); + $finalIconFile->compositeImage($appIconFile, Imagick::COMPOSITE_ATOP, $offset_w, $offset_h); + $finalIconFile->setImageFormat('PNG32'); + if (defined('Imagick::INTERPOLATE_BICUBIC') === true) { + $filter = Imagick::INTERPOLATE_BICUBIC; + } else { + $filter = Imagick::FILTER_LANCZOS; + } + $finalIconFile->resizeImage($size, $size, $filter, 1, false); + + return $finalIconFile; + } finally { + unset($appIconFile); } - $finalIconFile->resizeImage($size, $size, $filter, 1, false); - $appIconFile->destroy(); - return $finalIconFile; + return false; } /** diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php index 309bf192bc3f0..c25cf5bdade04 100644 --- a/apps/theming/lib/ImageManager.php +++ b/apps/theming/lib/ImageManager.php @@ -85,18 +85,37 @@ public function getImageUrlAbsolute(string $key): string { public function getImage(string $key, bool $useSvg = true): ISimpleFile { $mime = $this->config->getAppValue('theming', $key . 'Mime', ''); $folder = $this->getRootFolder()->getFolder('images'); + $useSvg = $useSvg && $this->canConvert('SVG'); if ($mime === '' || !$folder->fileExists($key)) { throw new NotFoundException(); } - - if (!$useSvg && $this->shouldReplaceIcons()) { + // if SVG was requested and is supported + if ($useSvg) { + if (!$folder->fileExists($key . '.svg')) { + try { + $finalIconFile = new \Imagick(); + $finalIconFile->setBackgroundColor('none'); + $finalIconFile->readImageBlob($folder->getFile($key)->getContent()); + $finalIconFile->setImageFormat('SVG'); + $svgFile = $folder->newFile($key . '.svg'); + $svgFile->putContent($finalIconFile->getImageBlob()); + return $svgFile; + } catch (\ImagickException $e) { + $this->logger->info('The image was requested to be no SVG file, but converting it to SVG failed: ' . $e->getMessage()); + } + } else { + return $folder->getFile($key . '.svg'); + } + } + // if SVG was not requested, but PNG is supported + if (!$useSvg && $this->canConvert('PNG')) { if (!$folder->fileExists($key . '.png')) { try { $finalIconFile = new \Imagick(); $finalIconFile->setBackgroundColor('none'); $finalIconFile->readImageBlob($folder->getFile($key)->getContent()); - $finalIconFile->setImageFormat('png32'); + $finalIconFile->setImageFormat('PNG32'); $pngFile = $folder->newFile($key . '.png'); $pngFile->putContent($finalIconFile->getImageBlob()); return $pngFile; @@ -107,7 +126,7 @@ public function getImage(string $key, bool $useSvg = true): ISimpleFile { return $folder->getFile($key . '.png'); } } - + // fallback to the original file return $folder->getFile($key); } @@ -328,7 +347,7 @@ private function shouldOptimizeBackgroundImage(string $mimeType, int $contentSiz public function getSupportedUploadImageFormats(string $key): array { $supportedFormats = ['image/jpeg', 'image/png', 'image/gif', 'image/webp']; - if ($key !== 'favicon' || $this->shouldReplaceIcons() === true) { + if ($key !== 'favicon' || $this->canConvert('SVG') === true) { $supportedFormats[] = 'image/svg+xml'; $supportedFormats[] = 'image/svg'; } @@ -364,17 +383,26 @@ public function cleanup() { * @return bool */ public function shouldReplaceIcons() { + return $this->canConvert('SVG'); + } + + /** + * Check if Imagemagick is enabled and if format is supported + * + * @return bool + */ + public function canConvert(string $format): bool { $cache = $this->cacheFactory->createDistributed('theming-' . $this->urlGenerator->getBaseUrl()); - if ($value = $cache->get('shouldReplaceIcons')) { + if ($value = $cache->get('convert-' . $format)) { return (bool)$value; } $value = false; if (extension_loaded('imagick')) { - if (count(\Imagick::queryFormats('SVG')) >= 1) { + if (count(\Imagick::queryFormats($format)) >= 1) { $value = true; } } - $cache->set('shouldReplaceIcons', $value); + $cache->set('convert-' . $format, $value); return $value; } diff --git a/apps/theming/lib/ThemingDefaults.php b/apps/theming/lib/ThemingDefaults.php index da92f31903b23..22628adcb247c 100644 --- a/apps/theming/lib/ThemingDefaults.php +++ b/apps/theming/lib/ThemingDefaults.php @@ -379,10 +379,10 @@ public function replaceImagePath($app, $image) { } $route = false; - if ($image === 'favicon.ico' && ($this->imageManager->shouldReplaceIcons() || $this->getCustomFavicon() !== null)) { + if ($image === 'favicon.ico' && ($this->imageManager->canConvert('ICO') || $this->getCustomFavicon() !== null)) { $route = $this->urlGenerator->linkToRoute('theming.Icon.getFavicon', ['app' => $app]); } - if (($image === 'favicon-touch.png' || $image === 'favicon-fb.png') && ($this->imageManager->shouldReplaceIcons() || $this->getCustomFavicon() !== null)) { + if (($image === 'favicon-touch.png' || $image === 'favicon-fb.png') && ($this->imageManager->canConvert('PNG') || $this->getCustomFavicon() !== null)) { $route = $this->urlGenerator->linkToRoute('theming.Icon.getTouchIcon', ['app' => $app]); } if ($image === 'manifest.json') { diff --git a/apps/theming/lib/Util.php b/apps/theming/lib/Util.php index 385fc14fac43a..70e5d2a0dd759 100644 --- a/apps/theming/lib/Util.php +++ b/apps/theming/lib/Util.php @@ -206,30 +206,38 @@ public function generateRadioButton($color) { * @param string $app app name * @return string|ISimpleFile path to app icon / file of logo */ - public function getAppIcon($app) { + public function getAppIcon($app, $useSvg = true) { $app = $this->appManager->cleanAppId($app); try { + // find app specific icon $appPath = $this->appManager->getAppPath($app); - $icon = $appPath . '/img/' . $app . '.svg'; + $extension = ($useSvg ? '.svg' : '.png'); + + $icon = $appPath . '/img/' . $app . $extension; if (file_exists($icon)) { return $icon; } - $icon = $appPath . '/img/app.svg'; + + $icon = $appPath . '/img/app' . $extension; if (file_exists($icon)) { return $icon; } } catch (AppPathNotFoundException $e) { } - + // fallback to custom instance logo if ($this->config->getAppValue('theming', 'logoMime', '') !== '') { - $logoFile = null; try { $folder = $this->appData->getFolder('global/images'); return $folder->getFile('logo'); } catch (NotFoundException $e) { } } - return \OC::$SERVERROOT . '/core/img/logo/logo.svg'; + // fallback to core logo + if ($useSvg) { + return \OC::$SERVERROOT . '/core/img/logo/logo.svg'; + } else { + return \OC::$SERVERROOT . '/core/img/logo/logo.png'; + } } /** diff --git a/apps/theming/openapi.json b/apps/theming/openapi.json index 33f9c54cc274b..0ab33e2cd8135 100644 --- a/apps/theming/openapi.json +++ b/apps/theming/openapi.json @@ -437,6 +437,12 @@ "200": { "description": "Favicon returned", "content": { + "image/png": { + "schema": { + "type": "string", + "format": "binary" + } + }, "image/x-icon": { "schema": { "type": "string", @@ -506,7 +512,7 @@ "format": "binary" } }, - "image/x-icon": { + "*/*": { "schema": { "type": "string", "format": "binary" diff --git a/apps/theming/tests/Controller/IconControllerTest.php b/apps/theming/tests/Controller/IconControllerTest.php index c5034600e0364..a0a39e206af93 100644 --- a/apps/theming/tests/Controller/IconControllerTest.php +++ b/apps/theming/tests/Controller/IconControllerTest.php @@ -19,6 +19,7 @@ use OCP\AppFramework\Http\FileDisplayResponse; use OCP\AppFramework\Utility\ITimeFactory; use OCP\Files\NotFoundException; +use OCP\IConfig; use OCP\IRequest; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -32,6 +33,7 @@ class IconControllerTest extends TestCase { private IAppManager&MockObject $appManager; private ImageManager&MockObject $imageManager; private IconController $iconController; + private IConfig&MockObject $config; protected function setUp(): void { $this->request = $this->createMock(IRequest::class); @@ -40,6 +42,7 @@ protected function setUp(): void { $this->imageManager = $this->createMock(ImageManager::class); $this->fileAccessHelper = $this->createMock(FileAccessHelper::class); $this->appManager = $this->createMock(IAppManager::class); + $this->config = $this->createMock(IConfig::class); $this->timeFactory = $this->createMock(ITimeFactory::class); $this->timeFactory->expects($this->any()) @@ -51,6 +54,7 @@ protected function setUp(): void { $this->iconController = new IconController( 'theming', $this->request, + $this->config, $this->themingDefaults, $this->iconBuilder, $this->imageManager, @@ -97,7 +101,8 @@ public function testGetFaviconDefault(): void { ->with('favicon') ->willThrowException(new NotFoundException()); $this->imageManager->expects($this->any()) - ->method('shouldReplaceIcons') + ->method('canConvert') + ->with('PNG') ->willReturn(true); $this->imageManager->expects($this->once()) ->method('getCachedImage') @@ -110,7 +115,7 @@ public function testGetFaviconDefault(): void { ->method('setCachedImage') ->willReturn($file); - $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $expected = new FileDisplayResponse($file, Http::STATUS_OK, ['Content-Type' => 'image/png']); $expected->cacheFor(86400); $this->assertEquals($expected, $this->iconController->getFavicon()); } @@ -121,14 +126,18 @@ public function testGetFaviconFail(): void { ->with('favicon', false) ->willThrowException(new NotFoundException()); $this->imageManager->expects($this->any()) - ->method('shouldReplaceIcons') - ->willReturn(false); + ->method('canConvert') + ->willReturnMap([ + ['SVG', false], + ['PNG', false], + ['ICO', false], + ]); $fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon.png'; $this->fileAccessHelper->expects($this->once()) ->method('file_get_contents') ->with($fallbackLogo) ->willReturn(file_get_contents($fallbackLogo)); - $expected = new DataDisplayResponse(file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/x-icon']); + $expected = new DataDisplayResponse(file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/png']); $expected->cacheFor(86400); $this->assertEquals($expected, $this->iconController->getFavicon()); } @@ -146,7 +155,8 @@ public function testGetTouchIconDefault(): void { ->method('getImage') ->willThrowException(new NotFoundException()); $this->imageManager->expects($this->any()) - ->method('shouldReplaceIcons') + ->method('canConvert') + ->with('PNG') ->willReturn(true); $this->iconBuilder->expects($this->once()) ->method('getTouchIcon') @@ -171,7 +181,8 @@ public function testGetTouchIconFail(): void { ->with('favicon') ->willThrowException(new NotFoundException()); $this->imageManager->expects($this->any()) - ->method('shouldReplaceIcons') + ->method('canConvert') + ->with('PNG') ->willReturn(false); $fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon-touch.png'; $this->fileAccessHelper->expects($this->once()) diff --git a/apps/theming/tests/Controller/ThemingControllerTest.php b/apps/theming/tests/Controller/ThemingControllerTest.php index fb461f03a289e..24b53b53d5190 100644 --- a/apps/theming/tests/Controller/ThemingControllerTest.php +++ b/apps/theming/tests/Controller/ThemingControllerTest.php @@ -645,6 +645,7 @@ public function testGetLogo(): void { $file = $this->createMock(ISimpleFile::class); $file->method('getName')->willReturn('logo.svg'); $file->method('getMTime')->willReturn(42); + $file->method('getMimeType')->willReturn('text/svg'); $this->imageManager->expects($this->once()) ->method('getImage') ->willReturn($file); @@ -661,7 +662,7 @@ public function testGetLogo(): void { $csp = new ContentSecurityPolicy(); $csp->allowInlineStyle(); $expected->setContentSecurityPolicy($csp); - @$this->assertEquals($expected, $this->themingController->getImage('logo')); + @$this->assertEquals($expected, $this->themingController->getImage('logo', true)); } @@ -677,6 +678,7 @@ public function testGetLoginBackground(): void { $file = $this->createMock(ISimpleFile::class); $file->method('getName')->willReturn('background.png'); $file->method('getMTime')->willReturn(42); + $file->method('getMimeType')->willReturn('image/png'); $this->imageManager->expects($this->once()) ->method('getImage') ->willReturn($file); diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php index d881e4eb75cc3..2e51de3e59c6e 100644 --- a/apps/theming/tests/IconBuilderTest.php +++ b/apps/theming/tests/IconBuilderTest.php @@ -13,9 +13,7 @@ use OCA\Theming\ThemingDefaults; use OCA\Theming\Util; use OCP\App\IAppManager; -use OCP\Files\NotFoundException; use OCP\IConfig; -use OCP\ServerVersion; use PHPUnit\Framework\MockObject\MockObject; use Test\TestCase; @@ -25,7 +23,7 @@ class IconBuilderTest extends TestCase { protected ThemingDefaults&MockObject $themingDefaults; protected ImageManager&MockObject $imageManager; protected IAppManager&MockObject $appManager; - protected Util $util; + protected Util&MockObject $util; protected IconBuilder $iconBuilder; protected function setUp(): void { @@ -36,123 +34,228 @@ protected function setUp(): void { $this->themingDefaults = $this->createMock(ThemingDefaults::class); $this->appManager = $this->createMock(IAppManager::class); $this->imageManager = $this->createMock(ImageManager::class); - $this->util = new Util($this->createMock(ServerVersion::class), $this->config, $this->appManager, $this->appData, $this->imageManager); + $this->util = $this->createMock(Util::class); $this->iconBuilder = new IconBuilder($this->themingDefaults, $this->util, $this->imageManager); } - private function checkImagick() { + /** + * Checks if Imagick and the required format are available. + * If provider is null, only checks for Imagick extension. + */ + private function checkImagick(?string $provider = null) { if (!extension_loaded('imagick')) { $this->markTestSkipped('Imagemagick is required for dynamic icon generation.'); } - $checkImagick = new \Imagick(); - if (count($checkImagick->queryFormats('SVG')) < 1) { - $this->markTestSkipped('No SVG provider present.'); - } - if (count($checkImagick->queryFormats('PNG')) < 1) { - $this->markTestSkipped('No PNG provider present.'); + if ($provider !== null) { + $checkImagick = new \Imagick(); + if (count($checkImagick->queryFormats($provider)) < 1) { + $this->markTestSkipped('Imagemagick ' . $provider . ' support is required for this icon generation test.'); + } } } - public static function dataRenderAppIcon(): array { + /** + * Data provider for app icon rendering tests (SVG only). + */ + public static function dataRenderAppIconSvg(): array { return [ - ['core', '#0082c9', 'touch-original.png'], - ['core', '#FF0000', 'touch-core-red.png'], - ['testing', '#FF0000', 'touch-testing-red.png'], - ['comments', '#0082c9', 'touch-comments.png'], - ['core', '#0082c9', 'touch-original-png.png'], + ['logo', '#0082c9', 'logo.svg'], + ['settings', '#FF0000', 'settings.svg'], ]; } - #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIcon')] - public function testRenderAppIcon(string $app, string $color, string $file): void { - $this->checkImagick(); - $this->themingDefaults->expects($this->once()) + /** + * Data provider for app icon rendering tests (PNG only). + */ + public static function dataRenderAppIconPng(): array { + return [ + ['logo', '#0082c9', 'logo.png'], + ['settings', '#FF0000', 'settings.png'], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIconSvg')] + public function testRenderAppIconSvg(string $app, string $color, string $file): void { + $this->checkImagick('SVG'); + // mock required methods + $this->imageManager->expects($this->any()) + ->method('canConvert') + ->willReturnMap([ + ['SVG', true], + ['PNG', true] + ]); + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with($app, true) + ->willReturn(__DIR__ . '/data/' . $file); + $this->themingDefaults->expects($this->any()) ->method('getColorPrimary') ->willReturn($color); - $this->appData->expects($this->once()) - ->method('getFolder') - ->with('global/images') - ->willThrowException(new NotFoundException()); - - $expectedIcon = new \Imagick(realpath(__DIR__) . '/data/' . $file); + // generate expected output from source file + $expectedIcon = $this->generateTestIcon($file, 'SVG', 512, $color); + // run test $icon = $this->iconBuilder->renderAppIcon($app, 512); - $this->assertEquals(true, $icon->valid()); $this->assertEquals(512, $icon->getImageWidth()); $this->assertEquals(512, $icon->getImageHeight()); - $this->assertEquals($icon, $expectedIcon); + $icon->setImageFormat('SVG'); + $expectedIcon->setImageFormat('SVG'); + $this->assertEquals($expectedIcon->getImageBlob(), $icon->getImageBlob(), 'Generated icon differs from expected'); $icon->destroy(); $expectedIcon->destroy(); - // FIXME: We may need some comparison of the generated and the test images - // cloud be something like $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]) } - #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIcon')] - public function testGetTouchIcon(string $app, string $color, string $file): void { - $this->checkImagick(); - $this->themingDefaults->expects($this->once()) + #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIconPng')] + public function testRenderAppIconPng(string $app, string $color, string $file): void { + $this->checkImagick('PNG'); + // mock required methods + $this->imageManager->expects($this->any()) + ->method('canConvert') + ->willReturnMap([ + ['SVG', false], + ['PNG', true] + ]); + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with($app, false) + ->willReturn(__DIR__ . '/data/' . $file); + $this->themingDefaults->expects($this->any()) ->method('getColorPrimary') ->willReturn($color); - $this->appData->expects($this->once()) - ->method('getFolder') - ->with('global/images') - ->willThrowException(new NotFoundException()); - - $expectedIcon = new \Imagick(realpath(__DIR__) . '/data/' . $file); - $icon = new \Imagick(); - $icon->readImageBlob($this->iconBuilder->getTouchIcon($app)); - + // generate expected output from source file + $expectedIcon = $this->generateTestIcon($file, 'PNG', 512, $color); + // run test + $icon = $this->iconBuilder->renderAppIcon($app, 512); $this->assertEquals(true, $icon->valid()); $this->assertEquals(512, $icon->getImageWidth()); $this->assertEquals(512, $icon->getImageHeight()); - $this->assertEquals($icon, $expectedIcon); + $icon->setImageFormat('PNG'); + $expectedIcon->setImageFormat('PNG'); + $this->assertEquals($expectedIcon->getImageBlob(), $icon->getImageBlob(), 'Generated icon differs from expected'); $icon->destroy(); $expectedIcon->destroy(); - // FIXME: We may need some comparison of the generated and the test images - // cloud be something like $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]) } - #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIcon')] - public function testGetFavicon(string $app, string $color, string $file): void { - $this->checkImagick(); - $this->imageManager->expects($this->once()) - ->method('shouldReplaceIcons') - ->willReturn(true); - $this->themingDefaults->expects($this->once()) + #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIconSvg')] + public function testGetTouchIconSvg(string $app, string $color, string $file): void { + $this->checkImagick('SVG'); + // mock required methods + $this->imageManager->expects($this->any()) + ->method('canConvert') + ->willReturnMap([ + ['SVG', false], + ['PNG', true] + ]); + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with($app, true) + ->willReturn(__DIR__ . '/data/' . $file); + $this->themingDefaults->expects($this->any()) ->method('getColorPrimary') ->willReturn($color); - $this->appData->expects($this->once()) - ->method('getFolder') - ->with('global/images') - ->willThrowException(new NotFoundException()); + // generate expected output from source file + $expectedIcon = $this->generateTestIcon($file, 'SVG', 512, $color); + $expectedIcon->setImageFormat('PNG32'); + // run test + $result = $this->iconBuilder->getTouchIcon($app); + $this->assertIsString($result, 'Touch icon generation should return a PNG blob'); + $this->assertEquals($expectedIcon->getImageBlob(), $result, 'Generated touch icon differs from expected'); + $expectedIcon->destroy(); + } - $expectedIcon = new \Imagick(realpath(__DIR__) . '/data/' . $file); - $actualIcon = $this->iconBuilder->getFavicon($app); + #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIconPng')] + public function testGetTouchIconPng(string $app, string $color, string $file): void { + $this->checkImagick('PNG'); + // mock required methods + $this->imageManager->expects($this->any()) + ->method('canConvert') + ->willReturnMap([ + ['SVG', false], + ['PNG', true] + ]); + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with($app, false) + ->willReturn(__DIR__ . '/data/' . $file); + $this->themingDefaults->expects($this->any()) + ->method('getColorPrimary') + ->willReturn($color); + // generate expected output from source file + $expectedIcon = $this->generateTestIcon($file, 'PNG', 512, $color); + $expectedIcon->setImageFormat('PNG32'); + // run test + $result = $this->iconBuilder->getTouchIcon($app); + $this->assertIsString($result, 'Touch icon generation should return a PNG blob'); + $this->assertEquals($expectedIcon->getImageBlob(), $result, 'Generated touch icon differs from expected'); + $expectedIcon->destroy(); + } - $icon = new \Imagick(); - $icon->setFormat('ico'); - $icon->readImageBlob($actualIcon); + #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIconSvg')] + public function testGetFavIconSvg(string $app, string $color, string $file): void { + $this->checkImagick('SVG'); + // mock required methods + $this->imageManager->expects($this->any()) + ->method('canConvert') + ->willReturnMap([ + ['ICO', true], + ['SVG', true], + ['PNG', true] + ]); + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with($app, true) + ->willReturn(__DIR__ . '/data/' . $file); + $this->themingDefaults->expects($this->any()) + ->method('getColorPrimary') + ->willReturn($color); + // generate expected output from source file + $expectedIcon = $this->generateTestFavIcon($file, 'SVG', $color); + // run test + $result = $this->iconBuilder->getFavicon($app); + $this->assertIsString($result, 'Favicon generation should return a ICO blob'); + $this->assertEquals($expectedIcon->getImageBlob(), $result, 'Generated favicon differs from expected'); + $expectedIcon->destroy(); + } - $this->assertEquals(true, $icon->valid()); - $this->assertEquals(128, $icon->getImageWidth()); - $this->assertEquals(128, $icon->getImageHeight()); - $icon->destroy(); + #[\PHPUnit\Framework\Attributes\DataProvider('dataRenderAppIconPng')] + public function testGetFaviconPng(string $app, string $color, string $file): void { + $this->checkImagick('PNG'); + // mock required methods + $this->imageManager->expects($this->any()) + ->method('canConvert') + ->willReturnMap([ + ['ICO', true], + ['SVG', false], + ['PNG', true] + ]); + $this->util->expects($this->once()) + ->method('getAppIcon') + ->with($app, false) + ->willReturn(__DIR__ . '/data/' . $file); + $this->themingDefaults->expects($this->any()) + ->method('getColorPrimary') + ->willReturn($color); + // generate expected output from source file + $expectedIcon = $this->generateTestFavIcon($file, 'PNG', $color); + // run test + $result = $this->iconBuilder->getFavicon($app); + $this->assertIsString($result, 'Favicon generation should return a PNG blob'); + $this->assertEquals($expectedIcon->getImagesBlob(), $result, 'Generated favicon differs from expected'); $expectedIcon->destroy(); - // FIXME: We may need some comparison of the generated and the test images - // cloud be something like $expectedIcon->compareImages($icon, Imagick::METRIC_MEANABSOLUTEERROR)[1]) } public function testGetFaviconNotFound(): void { - $this->checkImagick(); + $this->checkImagick('ICO'); $util = $this->createMock(Util::class); $iconBuilder = new IconBuilder($this->themingDefaults, $util, $this->imageManager); - $this->imageManager->expects($this->once()) - ->method('shouldReplaceIcons') + $this->imageManager->expects($this->any()) + ->method('canConvert') ->willReturn(true); $util->expects($this->once()) ->method('getAppIcon') ->willReturn('notexistingfile'); - $this->assertFalse($iconBuilder->getFavicon('noapp')); + $result = $iconBuilder->getFavicon('noapp'); + $this->assertFalse($result, 'Favicon generation should fail for missing file'); } public function testGetTouchIconNotFound(): void { @@ -174,4 +277,73 @@ public function testColorSvgNotFound(): void { ->willReturn('notexistingfile'); $this->assertFalse($iconBuilder->colorSvg('noapp', 'noimage')); } + + /** + * Helper to generate expected icon from source file for tests. + */ + private function generateTestIcon(string $file, string $format, int $size, string $color): \Imagick { + $filePath = realpath(__DIR__ . '/data/' . $file); + $appIconFile = new \Imagick(); + if ($format === 'SVG') { + $svgContent = file_get_contents($filePath); + if (substr($svgContent, 0, 5) !== '' . $svgContent; + } + $appIconFile->setResolution($size, $size); + $appIconFile->readImageBlob($svgContent); + } else { + $appIconFile->readImage($filePath); + } + $padding = 0.85; + $original_w = $appIconFile->getImageWidth(); + $original_h = $appIconFile->getImageHeight(); + $contentSize = (int)floor($size * $padding); + $scale = min($contentSize / $original_w, $contentSize / $original_h); + $new_w = max(1, (int)floor($original_w * $scale)); + $new_h = max(1, (int)floor($original_h * $scale)); + $offset_w = (int)floor(($size - $new_w) / 2); + $offset_h = (int)floor(($size - $new_h) / 2); + $cornerRadius = 0.2 * $size; + $appIconFile->resizeImage($new_w, $new_h, \Imagick::FILTER_LANCZOS, 1); + $finalIconFile = new \Imagick(); + $finalIconFile->setBackgroundColor(new \ImagickPixel('transparent')); + $finalIconFile->newImage($size, $size, new \ImagickPixel('transparent')); + $draw = new \ImagickDraw(); + $draw->setFillColor($color); + $draw->roundRectangle(0, 0, $size - 1, $size - 1, $cornerRadius, $cornerRadius); + $finalIconFile->drawImage($draw); + $draw->destroy(); + $finalIconFile->setImageVirtualPixelMethod(\Imagick::VIRTUALPIXELMETHOD_TRANSPARENT); + $finalIconFile->setImageArtifact('compose:args', '1,0,-0.5,0.5'); + $finalIconFile->compositeImage($appIconFile, \Imagick::COMPOSITE_ATOP, $offset_w, $offset_h); + $finalIconFile->setImageFormat('PNG32'); + if (defined('Imagick::INTERPOLATE_BICUBIC') === true) { + $filter = \Imagick::INTERPOLATE_BICUBIC; + } else { + $filter = \Imagick::FILTER_LANCZOS; + } + $finalIconFile->resizeImage($size, $size, $filter, 1, false); + $finalIconFile->setImageFormat('png'); + $appIconFile->destroy(); + return $finalIconFile; + } + + /** + * Helper to generate expected favicon from source file for tests. + */ + private function generateTestFavIcon(string $file, string $format, string $color): \Imagick { + $baseIcon = $this->generateTestIcon($file, $format, 128, $color); + $baseIcon->setImageFormat('PNG32'); + + $testIcon = new \Imagick(); + $testIcon->setFormat('ICO'); + foreach ([16, 32, 64, 128] as $size) { + $clone = clone $baseIcon; + $clone->scaleImage($size, 0); + $testIcon->addImage($clone); + $clone->destroy(); + } + $baseIcon->destroy(); + return $testIcon; + } } diff --git a/apps/theming/tests/data/favicon-original.ico b/apps/theming/tests/data/favicon-original.ico deleted file mode 100644 index fab2f7f0231e6..0000000000000 Binary files a/apps/theming/tests/data/favicon-original.ico and /dev/null differ diff --git a/apps/theming/tests/data/logo.png b/apps/theming/tests/data/logo.png new file mode 100644 index 0000000000000..df32e1c7eab44 Binary files /dev/null and b/apps/theming/tests/data/logo.png differ diff --git a/apps/theming/tests/data/logo.png.license b/apps/theming/tests/data/logo.png.license new file mode 100644 index 0000000000000..14e094ccb3f8c --- /dev/null +++ b/apps/theming/tests/data/logo.png.license @@ -0,0 +1,2 @@ +SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later \ No newline at end of file diff --git a/apps/theming/tests/data/logo.svg b/apps/theming/tests/data/logo.svg new file mode 100644 index 0000000000000..d68aca1fb46cd --- /dev/null +++ b/apps/theming/tests/data/logo.svg @@ -0,0 +1 @@ + diff --git a/apps/theming/tests/data/logo.svg.license b/apps/theming/tests/data/logo.svg.license new file mode 100644 index 0000000000000..14e094ccb3f8c --- /dev/null +++ b/apps/theming/tests/data/logo.svg.license @@ -0,0 +1,2 @@ +SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later \ No newline at end of file diff --git a/apps/theming/tests/data/settings.png b/apps/theming/tests/data/settings.png new file mode 100644 index 0000000000000..b9af15c7e98c0 Binary files /dev/null and b/apps/theming/tests/data/settings.png differ diff --git a/apps/theming/tests/data/settings.png.license b/apps/theming/tests/data/settings.png.license new file mode 100644 index 0000000000000..14e094ccb3f8c --- /dev/null +++ b/apps/theming/tests/data/settings.png.license @@ -0,0 +1,2 @@ +SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later \ No newline at end of file diff --git a/apps/theming/tests/data/settings.svg b/apps/theming/tests/data/settings.svg new file mode 100644 index 0000000000000..61f78599121b6 --- /dev/null +++ b/apps/theming/tests/data/settings.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/apps/theming/tests/data/settings.svg.license b/apps/theming/tests/data/settings.svg.license new file mode 100644 index 0000000000000..14e094ccb3f8c --- /dev/null +++ b/apps/theming/tests/data/settings.svg.license @@ -0,0 +1,2 @@ +SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors +SPDX-License-Identifier: AGPL-3.0-or-later \ No newline at end of file diff --git a/apps/theming/tests/data/touch-comments.png b/apps/theming/tests/data/touch-comments.png deleted file mode 100644 index c4dc68d8238db..0000000000000 Binary files a/apps/theming/tests/data/touch-comments.png and /dev/null differ diff --git a/apps/theming/tests/data/touch-core-red.png b/apps/theming/tests/data/touch-core-red.png deleted file mode 100644 index 95e9a292e6e70..0000000000000 Binary files a/apps/theming/tests/data/touch-core-red.png and /dev/null differ diff --git a/apps/theming/tests/data/touch-original-png.png b/apps/theming/tests/data/touch-original-png.png deleted file mode 100644 index a619accae0f53..0000000000000 Binary files a/apps/theming/tests/data/touch-original-png.png and /dev/null differ diff --git a/apps/theming/tests/data/touch-original.png b/apps/theming/tests/data/touch-original.png deleted file mode 100644 index 079e555d39d2d..0000000000000 Binary files a/apps/theming/tests/data/touch-original.png and /dev/null differ diff --git a/apps/theming/tests/data/touch-testing-red.png b/apps/theming/tests/data/touch-testing-red.png deleted file mode 100644 index 352b74fba24de..0000000000000 Binary files a/apps/theming/tests/data/touch-testing-red.png and /dev/null differ diff --git a/openapi.json b/openapi.json index 17a613cc4b484..c73b21761d382 100644 --- a/openapi.json +++ b/openapi.json @@ -33610,6 +33610,12 @@ "200": { "description": "Favicon returned", "content": { + "image/png": { + "schema": { + "type": "string", + "format": "binary" + } + }, "image/x-icon": { "schema": { "type": "string", @@ -33679,7 +33685,7 @@ "format": "binary" } }, - "image/x-icon": { + "*/*": { "schema": { "type": "string", "format": "binary"