From 31b9fc9eaca2e16d7baef3ef7e4dfc12ba0ea45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Tue, 12 Sep 2017 12:05:03 +0200 Subject: [PATCH 1/3] Theming: Generate favicon sizes to avoid resizing issues done by the browser fixes #5193 Signed-off-by: Julius Haertl --- apps/theming/lib/IconBuilder.php | 25 +++++++++++++++++++++++-- apps/theming/tests/IconBuilderTest.php | 3 +++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index 8325834198..9669768737 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -56,13 +56,34 @@ class IconBuilder { */ public function getFavicon($app) { try { - $icon = $this->renderAppIcon($app, 32); + $favicon = new Imagick(); + $favicon->setFormat("ico"); + $icon = $this->renderAppIcon($app, 128); if ($icon === false) { return false; } $icon->setImageFormat("png32"); - $data = $icon->getImageBlob(); + + $clone = clone $icon; + $clone->scaleImage(16,0); + $favicon->addImage($clone); + + $clone = clone $icon; + $clone->scaleImage(32,0); + $favicon->addImage($clone); + + $clone = clone $icon; + $clone->scaleImage(64,0); + $favicon->addImage($clone); + + $clone = clone $icon; + $clone->scaleImage(128,0); + $favicon->addImage($clone); + + $data = $favicon->getImagesBlob(); + $favicon->destroy(); $icon->destroy(); + $clone->destroy(); return $data; } catch (\ImagickException $e) { return false; diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php index f25f76a818..b301fa0620 100644 --- a/apps/theming/tests/IconBuilderTest.php +++ b/apps/theming/tests/IconBuilderTest.php @@ -71,6 +71,9 @@ class IconBuilderTest extends TestCase { if (count($checkImagick->queryFormats('SVG')) < 1) { $this->markTestSkipped('No SVG provider present.'); } + if (count($checkImagick->queryFormats('PNG')) < 1) { + $this->markTestSkipped('No PNG provider present.'); + } } public function dataRenderAppIcon() { From d70e9059a5d08c1606d716e0c1b0e0662733b5cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Sat, 23 Sep 2017 11:35:45 +0200 Subject: [PATCH 2/3] Theming: Fix tests for favicon containing multiple sizes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/theming/lib/IconBuilder.php | 3 +++ apps/theming/tests/IconBuilderTest.php | 18 +++++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/apps/theming/lib/IconBuilder.php b/apps/theming/lib/IconBuilder.php index 9669768737..ad44dd7ed6 100644 --- a/apps/theming/lib/IconBuilder.php +++ b/apps/theming/lib/IconBuilder.php @@ -55,6 +55,9 @@ class IconBuilder { * @return string|false image blob */ public function getFavicon($app) { + if (!$this->themingDefaults->shouldReplaceIcons()) { + return false; + } try { $favicon = new Imagick(); $favicon->setFormat("ico"); diff --git a/apps/theming/tests/IconBuilderTest.php b/apps/theming/tests/IconBuilderTest.php index b301fa0620..4f5078d4c5 100644 --- a/apps/theming/tests/IconBuilderTest.php +++ b/apps/theming/tests/IconBuilderTest.php @@ -149,17 +149,23 @@ class IconBuilderTest extends TestCase { */ public function testGetFavicon($app, $color, $file) { $this->checkImagick(); + $this->themingDefaults->expects($this->once()) + ->method('shouldReplaceIcons') + ->willReturn(true); $this->themingDefaults->expects($this->once()) ->method('getColorPrimary') ->willReturn($color); $expectedIcon = new \Imagick(realpath(dirname(__FILE__)). "/data/" . $file); + $actualIcon = $this->iconBuilder->getFavicon($app); + $icon = new \Imagick(); - $icon->readImageBlob($this->iconBuilder->getFavicon($app)); + $icon->setFormat('ico'); + $icon->readImageBlob($actualIcon); $this->assertEquals(true, $icon->valid()); - $this->assertEquals(32, $icon->getImageWidth()); - $this->assertEquals(32, $icon->getImageHeight()); + $this->assertEquals(128, $icon->getImageWidth()); + $this->assertEquals(128, $icon->getImageHeight()); $icon->destroy(); $expectedIcon->destroy(); // FIXME: We may need some comparison of the generated and the test images @@ -170,8 +176,12 @@ class IconBuilderTest extends TestCase { * @expectedException \PHPUnit_Framework_Error_Warning */ public function testGetFaviconNotFound() { + $this->checkImagick(); $util = $this->getMockBuilder(Util::class)->disableOriginalConstructor()->getMock(); $iconBuilder = new IconBuilder($this->themingDefaults, $util); + $this->themingDefaults->expects($this->once()) + ->method('shouldReplaceIcons') + ->willReturn(true); $util->expects($this->once()) ->method('getAppIcon') ->willReturn('notexistingfile'); @@ -182,6 +192,7 @@ class IconBuilderTest extends TestCase { * @expectedException \PHPUnit_Framework_Error_Warning */ public function testGetTouchIconNotFound() { + $this->checkImagick(); $util = $this->getMockBuilder(Util::class)->disableOriginalConstructor()->getMock(); $iconBuilder = new IconBuilder($this->themingDefaults, $util); $util->expects($this->once()) @@ -194,6 +205,7 @@ class IconBuilderTest extends TestCase { * @expectedException \PHPUnit_Framework_Error_Warning */ public function testColorSvgNotFound() { + $this->checkImagick(); $util = $this->getMockBuilder(Util::class)->disableOriginalConstructor()->getMock(); $iconBuilder = new IconBuilder($this->themingDefaults, $util); $util->expects($this->once()) From 7a812aa0e13704b2cc3a209cd9fc4ba564d86613 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julius=20H=C3=A4rtl?= Date: Sun, 12 Nov 2017 15:12:28 +0100 Subject: [PATCH 3/3] Theming: bump version to retrigger icon generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/theming/appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/theming/appinfo/info.xml b/apps/theming/appinfo/info.xml index ee3126c078..883dfbbb2e 100644 --- a/apps/theming/appinfo/info.xml +++ b/apps/theming/appinfo/info.xml @@ -5,7 +5,7 @@ Adjust the Nextcloud theme AGPL Nextcloud - 1.4.0 + 1.4.1 Theming other