From ee6d74eb63908d7dc665af6f2201d39a97ca586e Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 10 Jul 2024 15:29:45 +0200 Subject: [PATCH] fix(mail): Fix big logos in mail templates for Outlook Signed-off-by: Joas Schilling --- .../tests/Mailer/NewUserMailHelperTest.php | 2 + apps/theming/lib/ImageManager.php | 23 ++++++ lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Mail/EMailTemplate.php | 12 +++- lib/private/Mail/Mailer.php | 33 +++++++++ lib/private/Repair.php | 2 + lib/private/Repair/RepairLogoDimension.php | 71 +++++++++++++++++++ .../data/emails/new-account-email-custom.html | 2 +- .../new-account-email-single-button.html | 2 +- tests/data/emails/new-account-email.html | 2 +- tests/lib/Mail/EMailTemplateTest.php | 2 + tests/lib/Mail/MailerTest.php | 3 + 13 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 lib/private/Repair/RepairLogoDimension.php diff --git a/apps/settings/tests/Mailer/NewUserMailHelperTest.php b/apps/settings/tests/Mailer/NewUserMailHelperTest.php index d4ab5c1069256..0ea11d6c0e135 100644 --- 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 index f536bae0421de..4fcb0aab24d7f 100644 --- 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 index 2cb008efda0dc..6defb0f12fbe7 100644 --- 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 index ea059ec21ea3f..db8baeb6341d6 100644 --- 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 index bb60f7614506d..54a16e99ba9db 100644 --- a/lib/private/Mail/EMailTemplate.php +++ b/lib/private/Mail/EMailTemplate.php @@ -106,7 +106,7 @@ class EMailTemplate implements IEMailTemplate {
- +
@@ -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 index 750a5ee80e57f..77ba12c4852ee 100644 --- 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 index c9dd91be3f823..96a3feee2c278 100644 --- 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 new file mode 100644 index 0000000000000..122da205986a0 --- /dev/null +++ b/lib/private/Repair/RepairLogoDimension.php @@ -0,0 +1,71 @@ +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 index e1c9fd4fb4f81..370a2f749d74e 100644 --- a/tests/data/emails/new-account-email-custom.html +++ b/tests/data/emails/new-account-email-custom.html @@ -23,7 +23,7 @@
- +
diff --git a/tests/data/emails/new-account-email-single-button.html b/tests/data/emails/new-account-email-single-button.html index 35ad9b14a1c0f..51ed5b16f66a6 100644 --- a/tests/data/emails/new-account-email-single-button.html +++ b/tests/data/emails/new-account-email-single-button.html @@ -23,7 +23,7 @@
- +
diff --git a/tests/data/emails/new-account-email.html b/tests/data/emails/new-account-email.html index e879f44120676..e45edba74eea0 100644 --- a/tests/data/emails/new-account-email.html +++ b/tests/data/emails/new-account-email.html @@ -23,7 +23,7 @@
- +
diff --git a/tests/lib/Mail/EMailTemplateTest.php b/tests/lib/Mail/EMailTemplateTest.php index 284c53e73b860..d7e7a3370249d 100644 --- 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 index 5626abb085af6..e3995fd17be12 100644 --- 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'))); }