Skip to content

Commit

Permalink
fix(mail): Fix big logos in mail templates for Outlook
Browse files Browse the repository at this point in the history
Signed-off-by: Joas Schilling <coding@schilljs.com>
  • Loading branch information
nickvergessen committed Jul 17, 2024
1 parent 2f9fcc2 commit 37154e2
Show file tree
Hide file tree
Showing 12 changed files with 149 additions and 5 deletions.
2 changes: 2 additions & 0 deletions apps/settings/tests/Mailer/NewUserMailHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ protected function setUp(): void {
$this->defaults,
$this->urlGenerator,
$this->l10nFactory,
null,
null,
'test.TestTemplate',
[]
);
Expand Down
23 changes: 23 additions & 0 deletions apps/theming/lib/ImageManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,10 @@ public function delete(string $key): void {
} catch (NotFoundException $e) {
} catch (NotPermittedException $e) {
}

if ($key === 'logo') {
$this->config->deleteAppValue('theming', 'logoDimensions');

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::deleteAppValue has been marked as deprecated
}
}

public function updateImage(string $key, string $tmpFile): string {
Expand Down Expand Up @@ -266,6 +270,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));

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::setAppValue has been marked as deprecated
} 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]);

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::setAppValue has been marked as deprecated
} else {
$this->logger->warning('Could not read logo image dimensions to optimize for mail header');
$this->config->deleteAppValue('theming', 'logoDimensions');

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::deleteAppValue has been marked as deprecated
}
} else {
$this->logger->warning('Could not read logo image dimensions to optimize for mail header');
$this->config->deleteAppValue('theming', 'logoDimensions');

Check notice

Code scanning / Psalm

DeprecatedMethod Note

The method OCP\IConfig::deleteAppValue has been marked as deprecated
}
}

return $detectedMimeType;
}

Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -1757,6 +1757,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',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -1790,6 +1790,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',
Expand Down
12 changes: 10 additions & 2 deletions lib/private/Mail/EMailTemplate.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,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>
Expand Down Expand Up @@ -308,6 +308,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,
) {
Expand All @@ -330,8 +332,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]);
}

/**
Expand Down
33 changes: 33 additions & 0 deletions lib/private/Mail/Mailer.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,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(
Expand Down Expand Up @@ -109,10 +115,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
);
Expand Down
71 changes: 71 additions & 0 deletions lib/private/Repair/RepairLogoDimension.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
2 changes: 1 addition & 1 deletion tests/data/emails/new-account-email-custom.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion tests/data/emails/new-account-email-single-button.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion tests/data/emails/new-account-email.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 2 additions & 0 deletions tests/lib/Mail/EMailTemplateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ protected function setUp(): void {
$this->defaults,
$this->urlGenerator,
$this->l10n,
252,
120,
'test.TestTemplate',
[]
);
Expand Down
3 changes: 3 additions & 0 deletions tests/lib/Mail/MailerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,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')));
}
Expand Down

0 comments on commit 37154e2

Please sign in to comment.