Skip to content
Snippets Groups Projects
Commit fa7bd9f9 authored by Kevin Appelt's avatar Kevin Appelt Committed by Benjamin Franzke
Browse files

[BUGFIX] Make sure social media images gets processed if needed 

The check if the image has to be processed has been moved to
dedicated methods to enhance readability and testability

The condition still checks if the image is wider than the defined
maximum width. The check if the entire image is used should now be
easier to understand. No offset and 100% of the width and height
of the image correspond to the full image and negated in its entirety.

Resolves: #98118
Releases: main, 11.5
Change-Id: I8283e2f74310654455fb247e84c9e4204f3e1dd4
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/75531


Tested-by: default avatarOliver Hader <oliver.hader@typo3.org>
Tested-by: default avatarcore-ci <typo3@b13.com>
Tested-by: default avatarBenjamin Franzke <bfr@qbus.de>
Reviewed-by: default avatarOliver Hader <oliver.hader@typo3.org>
Reviewed-by: default avatarBenjamin Franzke <bfr@qbus.de>
parent b03d039e
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
typo3/sysext/seo/Classes/MetaTag/MetaTagGenerator.php
+ 36
− 28
View file @ fa7bd9f9
......@@ -19,6 +19,7 @@ namespace TYPO3\CMS\Seo\MetaTag;
use TYPO3\CMS\Core\Imaging\ImageManipulation\CropVariantCollection;
use TYPO3\CMS\Core\MetaTag\MetaTagManagerRegistry;
use TYPO3\CMS\Core\Resource\FileInterface;
use TYPO3\CMS\Core\Resource\FileReference;
use TYPO3\CMS\Core\Resource\ProcessedFile;
use TYPO3\CMS\Core\Utility\GeneralUtility;
......@@ -124,7 +125,7 @@ class MetaTagGenerator
}
/**
* @param array $fileReferences
* @param list<FileReference> $fileReferences
* @return array
*/
protected function generateSocialImages(array $fileReferences): array
......@@ -133,38 +134,45 @@ class MetaTagGenerator
$socialImages = [];
/** @var FileReference $file */
foreach ($fileReferences as $file) {
$arguments = $file->getProperties();
$cropVariantCollection = CropVariantCollection::create((string)$arguments['crop']);
$cropVariant = ($arguments['cropVariant'] ?? false) ?: 'social';
$cropArea = $cropVariantCollection->getCropArea($cropVariant);
$crop = $cropArea->makeAbsoluteBasedOnFile($file);
$processingConfiguration = [
'crop' => $crop,
'maxWidth' => 2000,
];
if ($file->getProperty('width') > $processingConfiguration['maxWidth'] || ($cropArea->getHeight() !== 1.0 && $cropArea->getWidth() !== 1.0)) {
$image = $file->getOriginalFile()->process(
ProcessedFile::CONTEXT_IMAGECROPSCALEMASK,
$processingConfiguration
);
} else {
$image = $file->getOriginalFile();
}
$imageUri = $imageService->getImageUri($image, true);
foreach ($fileReferences as $fileReference) {
$arguments = $fileReference->getProperties();
$image = $this->processSocialImage($fileReference);
$socialImages[] = [
'url' => $imageUri,
'width' => floor($image->getProperty('width')),
'height' => floor($image->getProperty('height')),
'url' => $imageService->getImageUri($image, true),
'width' => floor((float)$image->getProperty('width')),
'height' => floor((float)$image->getProperty('height')),
'alternative' => $arguments['alternative'],
];
}
return $socialImages;
}
protected function processSocialImage(FileReference $fileReference): FileInterface
{
$arguments = $fileReference->getProperties();
$cropVariantCollection = CropVariantCollection::create((string)($arguments['crop'] ?? ''));
$cropVariantName = ($arguments['cropVariant'] ?? false) ?: 'social';
$cropArea = $cropVariantCollection->getCropArea($cropVariantName);
$crop = $cropArea->makeAbsoluteBasedOnFile($fileReference);
$processingConfiguration = [
'crop' => $crop,
'maxWidth' => 2000,
];
// The image needs to be processed if:
// - the image width is greater than the defined maximum width, or
// - there is a cropping other than the full image (starts at 0,0 and has a width and height of 100%) defined
$needsProcessing = $fileReference->getProperty('width') > $processingConfiguration['maxWidth']
|| !$cropArea->isEmpty();
if (!$needsProcessing) {
return $fileReference->getOriginalFile();
}
return $fileReference->getOriginalFile()->process(
ProcessedFile::CONTEXT_IMAGECROPSCALEMASK,
$processingConfiguration
);
}
}
typo3/sysext/seo/Tests/Functional/MetaTag/MetaTagGeneratorTest.php 0 → 100644
+ 230
− 0
View file @ fa7bd9f9
<?php
declare(strict_types=1);
/*
* This file is part of the TYPO3 CMS project.
*
* It is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, either version 2
* of the License, or any later version.
*
* For the full copyright and license information, please read the
* LICENSE.txt file that was distributed with this source code.
*
* The TYPO3 project - inspiring people to share!
*/
namespace TYPO3\CMS\Seo\Tests\Functional\MetaTag;
use org\bovigo\vfs\vfsStream;
use org\bovigo\vfs\vfsStreamDirectory;
use TYPO3\CMS\Core\Imaging\ImageManipulation\Area;
use TYPO3\CMS\Core\Imaging\ImageManipulation\CropVariantCollection;
use TYPO3\CMS\Core\Resource\File;
use TYPO3\CMS\Core\Resource\FileInterface;
use TYPO3\CMS\Core\Resource\ProcessedFile;
use TYPO3\CMS\Core\Resource\ResourceFactory;
use TYPO3\CMS\Core\Resource\ResourceStorage;
use TYPO3\CMS\Core\Resource\StorageRepository;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Install\Configuration\Image\GraphicsMagickPreset;
use TYPO3\CMS\Seo\MetaTag\MetaTagGenerator;
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
class MetaTagGeneratorTest extends FunctionalTestCase
{
private MetaTagGenerator $subject;
private ResourceStorage $defaultStorage;
private vfsStreamDirectory $temporaryFileSystem;
protected $coreExtensionsToLoad = ['seo'];
protected function setUp(): void
{
parent::setUp();
// functional tests use GraphicMagick per default, resolve the corresponding path in current OS
$GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_path'] = $this->determineGraphicMagickBinaryPath();
$this->subject = GeneralUtility::makeInstance(MetaTagGenerator::class);
$this->defaultStorage = GeneralUtility::makeInstance(StorageRepository::class)->getDefaultStorage();
$this->temporaryFileSystem = vfsStream::setup();
}
public static function socialImageIsProcessedDataProvider(): \Generator
{
// having a valid `crop` definition, images are only process if there's a necessity
yield 'social: 600x600 enforced ratio' => [
true,
['width' => 600, 'height' => 600],
['width' => 600, 'height' => 315],
ProcessedFile::class,
];
yield 'social: 600x315 kept as is' => [
true,
['width' => 600, 'height' => 315],
['width' => 600, 'height' => 315],
File::class,
];
yield 'social: 1200x630 kept as is' => [
true,
['width' => 1200, 'height' => 630],
['width' => 1200, 'height' => 630],
File::class,
];
yield 'social: 2400x1260 limited to maxWidth, kept ratio' => [
true,
['width' => 2400, 'height' => 1260],
['width' => 2000, 'height' => 1050],
ProcessedFile::class,
];
yield 'social: 3000x3000 limited to maxWidth, enforced ratio' => [
true,
['width' => 3000, 'height' => 3000],
['width' => 2000, 'height' => 1050],
ProcessedFile::class,
];
yield 'social: 600x300 enforced ratio (no up-scaling)' => [
true,
['width' => 600, 'height' => 3000],
['width' => 600, 'height' => 315],
ProcessedFile::class,
];
yield 'social: 3000x600 enforced ratio (no up-scaling)' => [
true,
['width' => 3000, 'height' => 600],
['width' => 1142, 'height' => 600],
ProcessedFile::class,
];
// in case `crop` is not defined, no target ratio is defined for these images
// (data created prior to https://review.typo3.org/c/Packages/TYPO3.CMS/+/58774/ in v9.5.1 behaves like this)
yield 'empty crop: 600x600 kept as is' => [
false,
['width' => 600, 'height' => 600],
['width' => 600, 'height' => 600],
File::class,
];
yield 'empty crop: 600x315 kept as is' => [
false,
['width' => 600, 'height' => 315],
['width' => 600, 'height' => 315],
File::class,
];
yield 'empty crop: 1200x630 kept as is' => [
false,
['width' => 1200, 'height' => 630],
['width' => 1200, 'height' => 630],
File::class,
];
yield 'empty crop: 2400x1260 limited to maxWidth' => [
false,
['width' => 2400, 'height' => 1260],
['width' => 2000, 'height' => 1050],
ProcessedFile::class,
];
yield 'empty crop: 3000x3000 limited to maxWidth' => [
false,
['width' => 3000, 'height' => 3000],
['width' => 2000, 'height' => 2000],
ProcessedFile::class,
];
yield 'empty crop: 600x300 kept as is' => [
false,
['width' => 600, 'height' => 3000],
['width' => 600, 'height' => 3000],
File::class,
];
yield 'empty crop: 3000x600 limited to maxWidth' => [
false,
['width' => 3000, 'height' => 600],
['width' => 2000, 'height' => 400],
ProcessedFile::class,
];
}
/**
* @param array{width: int, height: int} $imageDimension
* @param array{width: int, height: int} $expectedDimension
* @test
* @dataProvider socialImageIsProcessedDataProvider
*/
public function socialImageIsProcessed(bool $hasCrop, array $imageDimension, array $expectedDimension, string $expectedClassName): void
{
$fileName = sprintf('test_%dx%d.png', $imageDimension['width'], $imageDimension['height']);
$folder = $this->defaultStorage->getFolder('/');
// drop file if it exists
$file = $folder->getFile($fileName);
if ($file !== null) {
$file->delete();
}
// create new file, fill it dummy PNG data for given dimension
/** @var File $file */
$file = $this->defaultStorage->createFile($fileName, $folder);
$file->setContents($this->createImagePngContent($imageDimension['width'], $imageDimension['height']));
// temporary file reference to an actual existing file
$fileReferenceProperties = [
'uid_local' => $file->getUid(),
'uid_foreign' => 0,
'uid' => 0,
'crop' => '',
];
if ($hasCrop) {
$cropVariantCollection = CropVariantCollection::create('', $this->resolveCropVariantsConfiguration());
$cropVariantCollection = $cropVariantCollection->applyRatioRestrictionToSelectedCropArea($file);
$fileReferenceProperties['crop'] = (string)$cropVariantCollection;
}
$fileReference = GeneralUtility::makeInstance(ResourceFactory::class)
->createFileReferenceObject($fileReferenceProperties);
// invoke processing of social image
$reflectionSubject = new \ReflectionObject($this->subject);
$reflectionMethod = $reflectionSubject->getMethod('processSocialImage');
$reflectionMethod->setAccessible(true); // no-op in PHP 8.1
/** @var FileInterface $processedSocialImage */
$processedSocialImage = $reflectionMethod->invoke($this->subject, $fileReference);
self::assertSame($expectedDimension, [
'width' => (int)$processedSocialImage->getProperty('width'),
'height' => (int)$processedSocialImage->getProperty('height'),
]);
self::assertInstanceOf($expectedClassName, $processedSocialImage);
}
private function createImagePngContent(int $width, int $height): string
{
$filePath = $this->temporaryFileSystem->url() . '/image.png';
$gdImage = imagecreatetruecolor($width, $height);
imagepng($gdImage, $filePath);
return file_get_contents($filePath);
}
private function determineGraphicMagickBinaryPath(): string
{
$values = GeneralUtility::makeInstance(GraphicsMagickPreset::class)->getConfigurationValues();
return $values['GFX/processor_path'] ?? $GLOBALS['TYPO3_CONF_VARS']['GFX']['processor_path'];
}
/**
* A little bit like `\TYPO3\CMS\Backend\Form\Element\ImageManipulationElement::populateConfiguration`...
*/
private function resolveCropVariantsConfiguration(): array
{
$config = $GLOBALS['TCA']['pages']['columns']['og_image']['config']['overrideChildTca']['columns']['crop']['config'];
$cropVariants = [];
foreach ($config['cropVariants'] as $id => $cropVariant) {
// Filter allowed aspect ratios
$cropVariant['allowedAspectRatios'] = array_filter(
$cropVariant['allowedAspectRatios'] ?? [],
static fn ($aspectRatio) => empty($aspectRatio['disabled'])
);
// Ignore disabled crop variants
if (!empty($cropVariant['disabled'])) {
continue;
}
// Enforce a crop area (default is full image)
if (empty($cropVariant['cropArea'])) {
$cropVariant['cropArea'] = Area::createEmpty()->asArray();
}
$cropVariants[$id] = $cropVariant;
}
return $cropVariants;
}
}
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment