Skip to content

Commit 929e165

Browse files
SebastianKrupinskiSebastianKrupinski
authored
Merge pull request #55132 from nextcloud/fix/favourite-icon-without-imagick-svg-support
fix: generate favourite icon without imagick svg support
2 parents 92d9ca6 + e2c4db1 commit 929e165

25 files changed

+471
-183
lines changed

‎apps/theming/lib/Controller/IconController.php‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use OCP\AppFramework\Http\NotFoundResponse;
2222
use OCP\AppFramework\Http\Response;
2323
use OCP\Files\NotFoundException;
24+
use OCP\IConfig;
2425
use OCP\IRequest;
2526

2627
class IconController extends Controller {
@@ -30,6 +31,7 @@ class IconController extends Controller {
3031
public function __construct(
3132
$appName,
3233
IRequest $request,
34+
private IConfig $config,
3335
private ThemingDefaults $themingDefaults,
3436
private IconBuilder $iconBuilder,
3537
private ImageManager $imageManager,
@@ -79,7 +81,7 @@ public function getThemedIcon(string $app, string $image): Response {
7981
* Return a 32x32 favicon as png
8082
*
8183
* @param string $app ID of the app
82-
* @return DataDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/x-icon'}>|FileDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/x-icon'}>|NotFoundResponse<Http::STATUS_NOT_FOUND, array{}>
84+
* @return DataDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/png'}>|FileDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/x-icon'}>|NotFoundResponse<Http::STATUS_NOT_FOUND, array{}>
8385
* @throws \Exception
8486
*
8587
* 200: Favicon returned
@@ -95,12 +97,14 @@ public function getFavicon(string $app = 'core'): Response {
9597

9698
$response = null;
9799
$iconFile = null;
100+
// retrieve instance favicon
98101
try {
99102
$iconFile = $this->imageManager->getImage('favicon', false);
100103
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
101104
} catch (NotFoundException $e) {
102105
}
103-
if ($iconFile === null && $this->imageManager->shouldReplaceIcons()) {
106+
// retrieve or generate app specific favicon
107+
if (($this->imageManager->canConvert('PNG') || $this->imageManager->canConvert('SVG')) && $this->imageManager->canConvert('ICO')) {
104108
$color = $this->themingDefaults->getColorPrimary();
105109
try {
106110
$iconFile = $this->imageManager->getCachedImage('favIcon-' . $app . $color);
@@ -113,9 +117,10 @@ public function getFavicon(string $app = 'core'): Response {
113117
}
114118
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
115119
}
120+
// fallback to core favicon
116121
if ($response === null) {
117122
$fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon.png';
118-
$response = new DataDisplayResponse($this->fileAccessHelper->file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
123+
$response = new DataDisplayResponse($this->fileAccessHelper->file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/png']);
119124
}
120125
$response->cacheFor(86400);
121126
return $response;
@@ -125,7 +130,7 @@ public function getFavicon(string $app = 'core'): Response {
125130
* Return a 512x512 icon for touch devices
126131
*
127132
* @param string $app ID of the app
128-
* @return DataDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/png'}>|FileDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/x-icon'|'image/png'}>|NotFoundResponse<Http::STATUS_NOT_FOUND, array{}>
133+
* @return DataDisplayResponse<Http::STATUS_OK, array{Content-Type: 'image/png'}>|FileDisplayResponse<Http::STATUS_OK, array{Content-Type: string}>|NotFoundResponse<Http::STATUS_NOT_FOUND, array{}>
129134
* @throws \Exception
130135
*
131136
* 200: Touch icon returned
@@ -140,12 +145,14 @@ public function getTouchIcon(string $app = 'core'): Response {
140145
}
141146

142147
$response = null;
148+
// retrieve instance favicon
143149
try {
144150
$iconFile = $this->imageManager->getImage('favicon');
145-
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/x-icon']);
151+
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => $iconFile->getMimeType()]);
146152
} catch (NotFoundException $e) {
147153
}
148-
if ($this->imageManager->shouldReplaceIcons()) {
154+
// retrieve or generate app specific touch icon
155+
if ($this->imageManager->canConvert('PNG')) {
149156
$color = $this->themingDefaults->getColorPrimary();
150157
try {
151158
$iconFile = $this->imageManager->getCachedImage('touchIcon-' . $app . $color);
@@ -158,6 +165,7 @@ public function getTouchIcon(string $app = 'core'): Response {
158165
}
159166
$response = new FileDisplayResponse($iconFile, Http::STATUS_OK, ['Content-Type' => 'image/png']);
160167
}
168+
// fallback to core touch icon
161169
if ($response === null) {
162170
$fallbackLogo = \OC::$SERVERROOT . '/core/img/favicon-touch.png';
163171
$response = new DataDisplayResponse($this->fileAccessHelper->file_get_contents($fallbackLogo), Http::STATUS_OK, ['Content-Type' => 'image/png']);

‎apps/theming/lib/Controller/ThemingController.php‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,7 @@ public function undoAll(): DataResponse {
366366
#[OpenAPI(scope: OpenAPI::SCOPE_DEFAULT)]
367367
public function getImage(string $key, bool $useSvg = true) {
368368
try {
369+
$useSvg = $useSvg && $this->imageManager->canConvert('SVG');
369370
$file = $this->imageManager->getImage($key, $useSvg);
370371
} catch (NotFoundException $e) {
371372
return new NotFoundResponse();
@@ -376,13 +377,8 @@ public function getImage(string $key, bool $useSvg = true) {
376377
$csp->allowInlineStyle();
377378
$response->setContentSecurityPolicy($csp);
378379
$response->cacheFor(3600);
379-
$response->addHeader('Content-Type', $this->config->getAppValue($this->appName, $key . 'Mime', ''));
380+
$response->addHeader('Content-Type', $file->getMimeType());
380381
$response->addHeader('Content-Disposition', 'attachment; filename="' . $key . '"');
381-
if (!$useSvg) {
382-
$response->addHeader('Content-Type', 'image/png');
383-
} else {
384-
$response->addHeader('Content-Type', $this->config->getAppValue($this->appName, $key . 'Mime', ''));
385-
}
386382
return $response;
387383
}
388384

‎apps/theming/lib/IconBuilder.php‎

Lines changed: 96 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
namespace OCA\Theming;
88

99
use Imagick;
10+
use ImagickDraw;
1011
use ImagickPixel;
1112
use OCP\Files\SimpleFS\ISimpleFile;
1213

@@ -30,17 +31,18 @@ public function __construct(
3031
* @return string|false image blob
3132
*/
3233
public function getFavicon($app) {
33-
if (!$this->imageManager->shouldReplaceIcons()) {
34+
if (!$this->imageManager->canConvert('PNG')) {
3435
return false;
3536
}
3637
try {
37-
$favicon = new Imagick();
38-
$favicon->setFormat('ico');
3938
$icon = $this->renderAppIcon($app, 128);
4039
if ($icon === false) {
4140
return false;
4241
}
43-
$icon->setImageFormat('png32');
42+
$icon->setImageFormat('PNG32');
43+
44+
$favicon = new Imagick();
45+
$favicon->setFormat('ICO');
4446

4547
$clone = clone $icon;
4648
$clone->scaleImage(16, 0);
@@ -96,7 +98,9 @@ public function getTouchIcon($app) {
9698
* @return Imagick|false
9799
*/
98100
public function renderAppIcon($app, $size) {
99-
$appIcon = $this->util->getAppIcon($app);
101+
$supportSvg = $this->imageManager->canConvert('SVG');
102+
// retrieve app icon
103+
$appIcon = $this->util->getAppIcon($app, $supportSvg);
100104
if ($appIcon instanceof ISimpleFile) {
101105
$appIconContent = $appIcon->getContent();
102106
$mime = $appIcon->getMimeType();
@@ -111,78 +115,100 @@ public function renderAppIcon($app, $size) {
111115
return false;
112116
}
113117

114-
$color = $this->themingDefaults->getColorPrimary();
118+
$appIconIsSvg = ($mime === 'image/svg+xml' || str_starts_with($appIconContent, '<svg') || str_starts_with($appIconContent, '<?xml'));
119+
// if source image is svg but svg not supported, abort.
120+
// source images are both user and developer set, and there is guarantees that mime and extension match actual contents type
121+
if ($appIconIsSvg && !$supportSvg) {
122+
return false;
123+
}
115124

116-
// generate background image with rounded corners
117-
$cornerRadius = 0.2 * $size;
118-
$background = '<?xml version="1.0" encoding="UTF-8"?>'
119-
. '<svg xmlns="http://www.w3.org/2000/svg" version="1.1" xmlns:cc="http://creativecommons.org/ns#" width="' . $size . '" height="' . $size . '" xmlns:xlink="http://www.w3.org/1999/xlink">'
120-
. '<rect x="0" y="0" rx="' . $cornerRadius . '" ry="' . $cornerRadius . '" width="' . $size . '" height="' . $size . '" style="fill:' . $color . ';" />'
121-
. '</svg>';
122-
// resize svg magic as this seems broken in Imagemagick
123-
if ($mime === 'image/svg+xml' || substr($appIconContent, 0, 4) === '<svg') {
124-
if (substr($appIconContent, 0, 5) !== '<?xml') {
125-
$svg = '<?xml version="1.0"?>' . $appIconContent;
126-
} else {
127-
$svg = $appIconContent;
128-
}
129-
$tmp = new Imagick();
130-
$tmp->setBackgroundColor(new ImagickPixel('transparent'));
131-
$tmp->setResolution(72, 72);
132-
$tmp->readImageBlob($svg);
133-
$x = $tmp->getImageWidth();
134-
$y = $tmp->getImageHeight();
135-
$tmp->destroy();
136-
137-
// convert svg to resized image
125+
// construct original image object
126+
try {
138127
$appIconFile = new Imagick();
139-
$res = (int)(72 * $size / max($x, $y));
140-
$appIconFile->setResolution($res, $res);
141128
$appIconFile->setBackgroundColor(new ImagickPixel('transparent'));
142-
$appIconFile->readImageBlob($svg);
143-
144-
/**
145-
* invert app icons for bright primary colors
146-
* the default nextcloud logo will not be inverted to black
147-
*/
148-
if ($this->util->isBrightColor($color)
149-
&& !$appIcon instanceof ISimpleFile
150-
&& $app !== 'core'
151-
) {
152-
$appIconFile->negateImage(false);
129+
130+
if ($appIconIsSvg) {
131+
// handle SVG images
132+
// ensure proper XML declaration
133+
if (!str_starts_with($appIconContent, '<?xml')) {
134+
$svg = '<?xml version="1.0"?>' . $appIconContent;
135+
} else {
136+
$svg = $appIconContent;
137+
}
138+
// get dimensions for resolution calculation
139+
$tmp = new Imagick();
140+
$tmp->setBackgroundColor(new ImagickPixel('transparent'));
141+
$tmp->setResolution(72, 72);
142+
$tmp->readImageBlob($svg);
143+
$x = $tmp->getImageWidth();
144+
$y = $tmp->getImageHeight();
145+
$tmp->destroy();
146+
// set resolution for proper scaling
147+
$resX = (int)(72 * $size / $x);
148+
$resY = (int)(72 * $size / $y);
149+
$appIconFile->setResolution($resX, $resY);
150+
$appIconFile->readImageBlob($svg);
151+
} else {
152+
// handle non-SVG images
153+
$appIconFile->readImageBlob($appIconContent);
153154
}
154-
} else {
155-
$appIconFile = new Imagick();
156-
$appIconFile->setBackgroundColor(new ImagickPixel('transparent'));
157-
$appIconFile->readImageBlob($appIconContent);
155+
} catch (\ImagickException $e) {
156+
return false;
158157
}
159-
// offset for icon positioning
160-
$padding = 0.15;
161-
$border_w = (int)($appIconFile->getImageWidth() * $padding);
162-
$border_h = (int)($appIconFile->getImageHeight() * $padding);
163-
$innerWidth = ($appIconFile->getImageWidth() - $border_w * 2);
164-
$innerHeight = ($appIconFile->getImageHeight() - $border_h * 2);
165-
$appIconFile->adaptiveResizeImage($innerWidth, $innerHeight);
166-
// center icon
167-
$offset_w = (int)($size / 2 - $innerWidth / 2);
168-
$offset_h = (int)($size / 2 - $innerHeight / 2);
169-
170-
$finalIconFile = new Imagick();
171-
$finalIconFile->setBackgroundColor(new ImagickPixel('transparent'));
172-
$finalIconFile->readImageBlob($background);
173-
$finalIconFile->setImageVirtualPixelMethod(Imagick::VIRTUALPIXELMETHOD_TRANSPARENT);
174-
$finalIconFile->setImageArtifact('compose:args', '1,0,-0.5,0.5');
175-
$finalIconFile->compositeImage($appIconFile, Imagick::COMPOSITE_ATOP, $offset_w, $offset_h);
176-
$finalIconFile->setImageFormat('png24');
177-
if (defined('Imagick::INTERPOLATE_BICUBIC') === true) {
178-
$filter = Imagick::INTERPOLATE_BICUBIC;
179-
} else {
180-
$filter = Imagick::FILTER_LANCZOS;
158+
// calculate final image size and position
159+
$padding = 0.85;
160+
$original_w = $appIconFile->getImageWidth();
161+
$original_h = $appIconFile->getImageHeight();
162+
$contentSize = (int)floor($size * $padding);
163+
$scale = min($contentSize / $original_w, $contentSize / $original_h);
164+
$new_w = max(1, (int)floor($original_w * $scale));
165+
$new_h = max(1, (int)floor($original_h * $scale));
166+
$offset_w = (int)floor(($size - $new_w) / 2);
167+
$offset_h = (int)floor(($size - $new_h) / 2);
168+
$cornerRadius = 0.2 * $size;
169+
$color = $this->themingDefaults->getColorPrimary();
170+
// resize original image
171+
$appIconFile->resizeImage($new_w, $new_h, Imagick::FILTER_LANCZOS, 1);
172+
/**
173+
* invert app icons for bright primary colors
174+
* the default nextcloud logo will not be inverted to black
175+
*/
176+
if ($this->util->isBrightColor($color)
177+
&& !$appIcon instanceof ISimpleFile
178+
&& $app !== 'core'
179+
) {
180+
$appIconFile->negateImage(false);
181+
}
182+
// construct final image object
183+
try {
184+
// image background
185+
$finalIconFile = new Imagick();
186+
$finalIconFile->setBackgroundColor(new ImagickPixel('transparent'));
187+
// icon background
188+
$finalIconFile->newImage($size, $size, new ImagickPixel('transparent'));
189+
$draw = new ImagickDraw();
190+
$draw->setFillColor($color);
191+
$draw->roundRectangle(0, 0, $size - 1, $size - 1, $cornerRadius, $cornerRadius);
192+
$finalIconFile->drawImage($draw);
193+
$draw->destroy();
194+
// overlay icon
195+
$finalIconFile->setImageVirtualPixelMethod(Imagick::VIRTUALPIXELMETHOD_TRANSPARENT);
196+
$finalIconFile->setImageArtifact('compose:args', '1,0,-0.5,0.5');
197+
$finalIconFile->compositeImage($appIconFile, Imagick::COMPOSITE_ATOP, $offset_w, $offset_h);
198+
$finalIconFile->setImageFormat('PNG32');
199+
if (defined('Imagick::INTERPOLATE_BICUBIC') === true) {
200+
$filter = Imagick::INTERPOLATE_BICUBIC;
201+
} else {
202+
$filter = Imagick::FILTER_LANCZOS;
203+
}
204+
$finalIconFile->resizeImage($size, $size, $filter, 1, false);
205+
206+
return $finalIconFile;
207+
} finally {
208+
unset($appIconFile);
181209
}
182-
$finalIconFile->resizeImage($size, $size, $filter, 1, false);
183210

184-
$appIconFile->destroy();
185-
return $finalIconFile;
211+
return false;
186212
}
187213

188214
/**

0 commit comments

Comments
 (0)