fix(mail): Fix big logos in mail templates for Outlook

Signed-off-by: Joas Schilling <coding@schilljs.com>
nextcloud · Jul 19, 2024 · ee6d74e · ee6d74e
1 parent d63621b
commit ee6d74e
Show file tree

Hide file tree

Showing 13 changed files with 151 additions and 5 deletions.
diff --git a/apps/settings/tests/Mailer/NewUserMailHelperTest.php b/apps/settings/tests/Mailer/NewUserMailHelperTest.php
@@ -85,6 +85,8 @@ protected function setUp(): void {
 			$this->defaults,
 			$this->urlGenerator,
 			$this->l10nFactory,
+			null,
+			null,
 			'test.TestTemplate',
 			[]
 		);

diff --git a/apps/theming/lib/ImageManager.php b/apps/theming/lib/ImageManager.php
@@ -209,6 +209,10 @@ public function delete(string $key): void {
 		} catch (NotFoundException $e) {
 		} catch (NotPermittedException $e) {
 		}
+
+		if ($key === 'logo') {
+			$this->config->deleteAppValue('theming', 'logoDimensions');
+		}
 	}
 
 	public function updateImage(string $key, string $tmpFile): string {
@@ -272,6 +276,25 @@ public function updateImage(string $key, string $tmpFile): string {
 
 		$target->putContent(file_get_contents($tmpFile));
 
+		if ($key === 'logo') {
+			$content = file_get_contents($tmpFile);
+			$newImage = @imagecreatefromstring($content);
+			if ($newImage !== false) {
+				$this->config->setAppValue('theming', 'logoDimensions', imagesx($newImage) . 'x' . imagesy($newImage));
+			} elseif (str_starts_with($detectedMimeType, 'image/svg')) {
+				$matched = preg_match('/viewbox=["\']\d* \d* (\d*\.?\d*) (\d*\.?\d*)["\']/i', $content, $matches);
+				if ($matched) {
+					$this->config->setAppValue('theming', 'logoDimensions', $matches[1] . 'x' . $matches[2]);
+				} else {
+					$this->logger->warning('Could not read logo image dimensions to optimize for mail header');
+					$this->config->deleteAppValue('theming', 'logoDimensions');
+				}
+			} else {
+				$this->logger->warning('Could not read logo image dimensions to optimize for mail header');
+				$this->config->deleteAppValue('theming', 'logoDimensions');
+			}
+		}
+
 		return $detectedMimeType;
 	}
 

diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
@@ -1700,6 +1700,7 @@
     'OC\\Repair\\RemoveLinkShares' => $baseDir . '/lib/private/Repair/RemoveLinkShares.php',
     'OC\\Repair\\RepairDavShares' => $baseDir . '/lib/private/Repair/RepairDavShares.php',
     'OC\\Repair\\RepairInvalidShares' => $baseDir . '/lib/private/Repair/RepairInvalidShares.php',
+    'OC\\Repair\\RepairLogoDimension' => $baseDir . '/lib/private/Repair/RepairLogoDimension.php',
     'OC\\Repair\\RepairMimeTypes' => $baseDir . '/lib/private/Repair/RepairMimeTypes.php',
     'OC\\RichObjectStrings\\Validator' => $baseDir . '/lib/private/RichObjectStrings/Validator.php',
     'OC\\Route\\CachingRouter' => $baseDir . '/lib/private/Route/CachingRouter.php',

diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
@@ -1733,6 +1733,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Repair\\RemoveLinkShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RemoveLinkShares.php',
         'OC\\Repair\\RepairDavShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairDavShares.php',
         'OC\\Repair\\RepairInvalidShares' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairInvalidShares.php',
+        'OC\\Repair\\RepairLogoDimension' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairLogoDimension.php',
         'OC\\Repair\\RepairMimeTypes' => __DIR__ . '/../../..' . '/lib/private/Repair/RepairMimeTypes.php',
         'OC\\RichObjectStrings\\Validator' => __DIR__ . '/../../..' . '/lib/private/RichObjectStrings/Validator.php',
         'OC\\Route\\CachingRouter' => __DIR__ . '/../../..' . '/lib/private/Route/CachingRouter.php',

diff --git a/lib/private/Mail/EMailTemplate.php b/lib/private/Mail/EMailTemplate.php
@@ -106,7 +106,7 @@ class EMailTemplate implements IEMailTemplate {
 							<tbody>
 							<tr style="padding:0;text-align:left;vertical-align:top">
 								<center data-parsed="" style="background-color:%s;min-width:175px;max-height:175px; padding:35px 0px;border-radius:200px">
-									<img class="logo float-center" src="%s" alt="%s" align="center" style="-ms-interpolation-mode:bicubic;clear:both;display:block;float:none;margin:0 auto;outline:0;text-align:center;text-decoration:none;max-height:105px;max-width:105px;width:auto;height:auto">
+									<img class="logo float-center" src="%s" alt="%s" align="center" style="-ms-interpolation-mode:bicubic;clear:both;display:block;float:none;margin:0 auto;outline:0;text-align:center;text-decoration:none;max-height:105px;max-width:105px;width:auto;height:auto"%s>
 								</center>
 							</tr>
 							</tbody>
@@ -338,6 +338,8 @@ public function __construct(
 		protected Defaults $themingDefaults,
 		protected IURLGenerator $urlGenerator,
 		protected IFactory $l10nFactory,
+		protected ?int $logoWidth,
+		protected ?int $logoHeight,
 		protected string $emailId,
 		protected array $data,
 	) {
@@ -360,8 +362,14 @@ public function addHeader(): void {
 		}
 		$this->headerAdded = true;
 
+		$logoSizeDimensions = '';
+		if ($this->logoWidth && $this->logoHeight) {
+			// Provide a logo size when we have the dimensions so that it displays nicely in Outlook
+			$logoSizeDimensions = ' width="' . $this->logoWidth . '" height="' . $this->logoHeight . '"';
+		}
+
 		$logoUrl = $this->urlGenerator->getAbsoluteURL($this->themingDefaults->getLogo(false));
-		$this->htmlBody .= vsprintf($this->header, [$this->themingDefaults->getDefaultColorPrimary(), $logoUrl, $this->themingDefaults->getName()]);
+		$this->htmlBody .= vsprintf($this->header, [$this->themingDefaults->getDefaultColorPrimary(), $logoUrl, $this->themingDefaults->getName(), $logoSizeDimensions]);
 	}
 
 	/**

diff --git a/lib/private/Mail/Mailer.php b/lib/private/Mail/Mailer.php
@@ -79,6 +79,12 @@
  * @package OC\Mail
  */
 class Mailer implements IMailer {
+	// Do not move this block or change it's content without contacting the release crew
+	public const DEFAULT_DIMENSIONS = '252x120';
+	// Do not move this block or change it's content without contacting the release crew
+
+	public const MAX_LOGO_SIZE = 105;
+
 	private ?MailerInterface $instance = null;
 
 	public function __construct(
@@ -136,10 +142,37 @@ public function createEMailTemplate(string $emailId, array $data = []): IEMailTe
 			);
 		}
 
+		$logoDimensions = $this->config->getAppValue('theming', 'logoDimensions', self::DEFAULT_DIMENSIONS);
+		if (str_contains($logoDimensions, 'x')) {
+			[$width, $height] = explode('x', $logoDimensions);
+			$width = (int) $width;
+			$height = (int) $height;
+
+			if ($width > self::MAX_LOGO_SIZE || $height > self::MAX_LOGO_SIZE) {
+				if ($width === $height) {
+					$logoWidth = self::MAX_LOGO_SIZE;
+					$logoHeight = self::MAX_LOGO_SIZE;
+				} elseif ($width > $height) {
+					$logoWidth = self::MAX_LOGO_SIZE;
+					$logoHeight = (int) (($height / $width) * self::MAX_LOGO_SIZE);
+				} else {
+					$logoWidth = (int) (($width / $height) * self::MAX_LOGO_SIZE);
+					$logoHeight = self::MAX_LOGO_SIZE;
+				}
+			} else {
+				$logoWidth = $width;
+				$logoHeight = $height;
+			}
+		} else {
+			$logoWidth = $logoHeight = null;
+		}
+
 		return new EMailTemplate(
 			$this->defaults,
 			$this->urlGenerator,
 			$this->l10nFactory,
+			$logoWidth,
+			$logoHeight,
 			$emailId,
 			$data
 		);

diff --git a/lib/private/Repair.php b/lib/private/Repair.php
@@ -79,6 +79,7 @@
 use OC\Repair\RemoveLinkShares;
 use OC\Repair\RepairDavShares;
 use OC\Repair\RepairInvalidShares;
+use OC\Repair\RepairLogoDimension;
 use OC\Repair\RepairMimeTypes;
 use OC\Template\JSCombiner;
 use OCA\DAV\Migration\DeleteSchedulingObjects;
@@ -211,6 +212,7 @@ public static function getRepairSteps(): array {
 			\OCP\Server::get(AddRemoveOldTasksBackgroundJob::class),
 			\OCP\Server::get(AddMetadataGenerationJob::class),
 			\OCP\Server::get(AddAppConfigLazyMigration::class),
+			\OCP\Server::get(RepairLogoDimension::class),
 		];
 	}
 

diff --git a/lib/private/Repair/RepairLogoDimension.php b/lib/private/Repair/RepairLogoDimension.php
@@ -0,0 +1,71 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+namespace OC\Repair;
+
+use OCA\Theming\ImageManager;
+use OCP\IConfig;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+use OCP\Server;
+
+class RepairLogoDimension implements IRepairStep {
+	public function __construct(
+		protected IConfig $config,
+	) {
+	}
+
+	public function getName(): string {
+		return 'Cache logo dimension to fix size in emails on Outlook';
+	}
+
+	public function run(IOutput $output): void {
+		$logoDimensions = $this->config->getAppValue('theming', 'logoDimensions');
+		if (preg_match('/^\d+x\d+$/', $logoDimensions)) {
+			$output->info('Logo dimensions are already known');
+			return;
+		}
+
+		try {
+			/** @var ImageManager $imageManager */
+			$imageManager = Server::get(ImageManager::class);
+		} catch (\Throwable) {
+			$output->info('Theming is disabled');
+			return;
+		}
+
+		if (!$imageManager->hasImage('logo')) {
+			$output->info('Theming is not used to provide a logo');
+			return;
+		}
+
+		$simpleFile = $imageManager->getImage('logo', false);
+
+		$image = @imagecreatefromstring($simpleFile->getContent());
+
+		$dimensions = '';
+		if ($image !== false) {
+			$dimensions = imagesx($image) . 'x' . imagesy($image);
+		} elseif (str_starts_with($simpleFile->getMimeType(), 'image/svg')) {
+			$matched = preg_match('/viewbox=["\']\d* \d* (\d*\.?\d*) (\d*\.?\d*)["\']/i', $simpleFile->getContent(), $matches);
+			if ($matched) {
+				$dimensions = $matches[1] . 'x' . $matches[2];
+			}
+		}
+
+		if (!$dimensions) {
+			$output->warning('Failed to read dimensions from logo');
+			$this->config->deleteAppValue('theming', 'logoDimensions');
+			return;
+		}
+
+		$dimensions = imagesx($image) . 'x' . imagesy($image);
+		$this->config->setAppValue('theming', 'logoDimensions', $dimensions);
+		$output->info('Updated logo dimensions: ' . $dimensions);
+	}
+}
diff --git a/tests/data/emails/new-account-email-custom.html b/tests/data/emails/new-account-email-custom.html
@@ -23,7 +23,7 @@
 							<tbody>
 							<tr style="padding:0;text-align:left;vertical-align:top">
 								<center data-parsed="" style="background-color:#0082c9;min-width:175px;max-height:175px; padding:35px 0px;border-radius:200px">
-									<img class="logo float-center" src="https://example.org/img/logo-mail-header.png" alt="TestCloud" align="center" style="-ms-interpolation-mode:bicubic;clear:both;display:block;float:none;margin:0 auto;outline:0;text-align:center;text-decoration:none;max-height:105px;max-width:105px;width:auto;height:auto">
+									<img class="logo float-center" src="https://example.org/img/logo-mail-header.png" alt="TestCloud" align="center" style="-ms-interpolation-mode:bicubic;clear:both;display:block;float:none;margin:0 auto;outline:0;text-align:center;text-decoration:none;max-height:105px;max-width:105px;width:auto;height:auto" width="252" height="120">
 								</center>
 							</tr>
 							</tbody>

diff --git a/tests/data/emails/new-account-email-single-button.html b/tests/data/emails/new-account-email-single-button.html
@@ -23,7 +23,7 @@
 							<tbody>
 							<tr style="padding:0;text-align:left;vertical-align:top">
 								<center data-parsed="" style="background-color:#0082c9;min-width:175px;max-height:175px; padding:35px 0px;border-radius:200px">
-									<img class="logo float-center" src="https://example.org/img/logo-mail-header.png" alt="TestCloud" align="center" style="-ms-interpolation-mode:bicubic;clear:both;display:block;float:none;margin:0 auto;outline:0;text-align:center;text-decoration:none;max-height:105px;max-width:105px;width:auto;height:auto">
+									<img class="logo float-center" src="https://example.org/img/logo-mail-header.png" alt="TestCloud" align="center" style="-ms-interpolation-mode:bicubic;clear:both;display:block;float:none;margin:0 auto;outline:0;text-align:center;text-decoration:none;max-height:105px;max-width:105px;width:auto;height:auto" width="252" height="120">
 								</center>
 							</tr>
 							</tbody>

diff --git a/tests/data/emails/new-account-email.html b/tests/data/emails/new-account-email.html
@@ -23,7 +23,7 @@
 							<tbody>
 							<tr style="padding:0;text-align:left;vertical-align:top">
 								<center data-parsed="" style="background-color:#0082c9;min-width:175px;max-height:175px; padding:35px 0px;border-radius:200px">
-									<img class="logo float-center" src="https://example.org/img/logo-mail-header.png" alt="TestCloud" align="center" style="-ms-interpolation-mode:bicubic;clear:both;display:block;float:none;margin:0 auto;outline:0;text-align:center;text-decoration:none;max-height:105px;max-width:105px;width:auto;height:auto">
+									<img class="logo float-center" src="https://example.org/img/logo-mail-header.png" alt="TestCloud" align="center" style="-ms-interpolation-mode:bicubic;clear:both;display:block;float:none;margin:0 auto;outline:0;text-align:center;text-decoration:none;max-height:105px;max-width:105px;width:auto;height:auto" width="252" height="120">
 								</center>
 							</tr>
 							</tbody>

diff --git a/tests/lib/Mail/EMailTemplateTest.php b/tests/lib/Mail/EMailTemplateTest.php
@@ -55,6 +55,8 @@ protected function setUp(): void {
 			$this->defaults,
 			$this->urlGenerator,
 			$this->l10n,
+			252,
+			120,
 			'test.TestTemplate',
 			[]
 		);

diff --git a/tests/lib/Mail/MailerTest.php b/tests/lib/Mail/MailerTest.php
@@ -240,6 +240,9 @@ public function testCreateEMailTemplate() {
 		$this->config->method('getSystemValueString')
 			->with('mail_template_class', '')
 			->willReturnArgument(1);
+		$this->config->method('getAppValue')
+			->with('theming', 'logoDimensions', Mailer::DEFAULT_DIMENSIONS)
+			->willReturn(Mailer::DEFAULT_DIMENSIONS);
 
 		$this->assertSame(EMailTemplate::class, get_class($this->mailer->createEMailTemplate('tests.MailerTest')));
 	}